From 8545e6a545cffcbfb5b13a07296e67a858e39a12 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 30 Oct 2026 20:57:38 -0400 Subject: [PATCH 01/18] release: expand an empty test argument list safely under macOS bash 3.2 The release-surface step ran go test "${test_args[@]}" under set -u. With no known-red ledger the array is empty, and /bin/bash 3.2 on macOS treats that as an unbound variable, so the prologue test failed on a Mac. The conditional expansion behaves the same on bash 5 and is safe on 3.2. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/release.yml | 3 ++- internal/release/workflow_test.go | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index f53df8e447..e34285fed4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -189,7 +189,8 @@ jobs: fi test_args=() [ -z "$skip" ] || test_args=(-skip "^(${skip})$") - go test "${test_args[@]}" \ + # Bash 3.2 treats an empty array as unbound under nounset, so expand it conditionally. + go test ${test_args[@]+"${test_args[@]}"} \ ./cmd/codeaf \ ./internal/config \ ./internal/exec/... \ diff --git a/internal/release/workflow_test.go b/internal/release/workflow_test.go index 2cfea32078..765f90ae49 100644 --- a/internal/release/workflow_test.go +++ b/internal/release/workflow_test.go @@ -106,7 +106,7 @@ func TestReleaseWorkflowKeepsTheChannelContract(t *testing.T) { `if [ -s .github/known-red.txt ]; then`, `skip="$(awk '!/^#/ && NF {print}' .github/known-red.txt | paste -sd'|' -)"`, `test_args=()`, - `go test "${test_args[@]}"`, + `go test ${test_args[@]+"${test_args[@]}"}`, } { if !strings.Contains(workflow, source) { t.Errorf("the release-surface test is not safe for an empty or absent known-red ledger: missing %q", source) @@ -183,7 +183,7 @@ func releaseSurfacePrologue(t *testing.T) string { continue } if strings.HasPrefix(trimmed, "go test ") { - script = append(script, `printf 'ARG:%s\n' "${test_args[@]}"`) + script = append(script, `printf 'ARG:%s\n' ${test_args[@]+"${test_args[@]}"}`) return strings.Join(script, "\n") + "\n" } script = append(script, trimmed) From 7accc18bc34556e12546a10f86b2d6ec1e453572 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 30 Oct 2026 20:57:38 -0400 Subject: [PATCH 02/18] test(syncsetup): give hand-built Sync fixtures their own home Two fixtures built Sync{} with an empty Home. A refusal retires the send hold, which joined the empty Home and wrote v3/sync/holds/.lock into the package's source directory on every run. They now use t.TempDir(). Co-Authored-By: Claude Opus 5.5 --- internal/syncsetup/drive_refusal_safety_test.go | 3 ++- internal/syncsetup/take_phases_test.go | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/syncsetup/drive_refusal_safety_test.go b/internal/syncsetup/drive_refusal_safety_test.go index 3d3f57a1b4..869a8eeb59 100644 --- a/internal/syncsetup/drive_refusal_safety_test.go +++ b/internal/syncsetup/drive_refusal_safety_test.go @@ -9,7 +9,8 @@ import ( ) func TestAStaleDriveRefusalImmediatelyMakesTheWindowAViewer(t *testing.T) { - d := &Drive{sync: &Sync{}} + // Refusal retires a saved send hold, so even this minimal drive needs an isolated home. + d := &Drive{sync: &Sync{Home: t.TempDir()}} d.refusal(directory.ErrFenceStale) if d.Gate() == nil { t.Fatal("a refused drive still allows new work") diff --git a/internal/syncsetup/take_phases_test.go b/internal/syncsetup/take_phases_test.go index 020d9b62b5..b5a5e30cf4 100644 --- a/internal/syncsetup/take_phases_test.go +++ b/internal/syncsetup/take_phases_test.go @@ -106,7 +106,8 @@ func TestStagedWorkspaceNeverTargetsTheBorrowedFolder(t *testing.T) { t.Fatal(err) } target := &admissionTarget{} - sync := &Sync{} + // Take helpers derive local sync paths, so this fixture must own its home. + sync := &Sync{Home: t.TempDir()} continuer := sync.Continuer(cellstore.Engine{DataRoot: t.TempDir(), Transport: target}, TakeOptions{RootFor: func(string) string { return root }}) take := continuer.taker(t.Context(), &Scope{Store: blobstore.NewMemory()}) if err := take.Fetch.Fetch(t.Context(), cell.Cell{ID: "chat", Root: stage}, strings.Repeat("a", 64)); err != nil { From b82f185ea97f09b16bc741f4c60a1e43c3e65e20 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 30 Oct 2026 20:57:44 -0400 Subject: [PATCH 03/18] test: engine fixtures record a real conversation and wait for async branches These tests need a furrow engine. GitHub's full tests stage none, so they skip there, and they failed on every machine that has one (a Mac after make build, or CODEAF_FURROW on Linux): - the scripted drive chat wrote files but never a reply, and a chat with no conversation is deliberately not published, so its turns never became durable; each scripted turn now writes a reply to the transcript - the large-home startup test drained a warm capture that has no turn, so nothing was sealed; it now completes one recorded call first - a refused drive branches asynchronously, so the counts and two-home tests wait for the branch row instead of reading it once - the superseded-drive test keeps A offline until its next publish, as the scenario states, and waits for the kept-work notice too Co-Authored-By: Claude Opus 5.5 --- internal/cellstore/homecapture_test.go | 6 ++++++ internal/syncsetup/counts_test.go | 8 +++++--- internal/syncsetup/drive_test.go | 15 ++++++++++----- internal/syncsetup/twohomes_test.go | 8 +++++--- 4 files changed, 26 insertions(+), 11 deletions(-) diff --git a/internal/cellstore/homecapture_test.go b/internal/cellstore/homecapture_test.go index 7e3686ab58..8720f19f1e 100644 --- a/internal/cellstore/homecapture_test.go +++ b/internal/cellstore/homecapture_test.go @@ -181,6 +181,12 @@ func TestStartupInALargeHomeWalksNoHome(t *testing.T) { if err != nil { t.Fatal(err) } + // Warm capture only prepares an unused chat, so a completed call must give Drain a turn to save. + if err := seat.Around(t.Context(), executor.Call{Tool: "read", Changed: []string{}}, func() ([]byte, bool) { + return []byte("fixture"), false + }); err != nil { + t.Fatal(err) + } if err := executor.Drain(t.Context(), seat); err != nil { t.Fatal(err) } diff --git a/internal/syncsetup/counts_test.go b/internal/syncsetup/counts_test.go index daa6b1810b..4a8856fd84 100644 --- a/internal/syncsetup/counts_test.go +++ b/internal/syncsetup/counts_test.go @@ -81,9 +81,11 @@ func TestScopesCountsEveryRelayRequest(t *testing.T) { } a.mustSay("orphaned") // A does not know it lost the chat; its publish is refused and becomes a branch waitFor(t, "A to learn it was superseded", func() bool { _, viewer := a.drive.Viewer(); return viewer }) - if _, ok := branchRowOf(t, h.a, h.cell.ID); !ok { - t.Fatal("A's refused turns did not become a branch") - } + // The drive refuses new work before its asynchronous branch upload finishes. + waitFor(t, "A's refused turns to become a branch", func() bool { + _, ok := branchRowOf(t, h.a, h.cell.ID) + return ok + }) if err := a.drive.Close(ctx); err != nil { t.Fatal(err) } diff --git a/internal/syncsetup/drive_test.go b/internal/syncsetup/drive_test.go index da3f0dab19..f6b8dba318 100644 --- a/internal/syncsetup/drive_test.go +++ b/internal/syncsetup/drive_test.go @@ -225,10 +225,12 @@ func (r *driveRig) startChat(notices *noticeLog) *chat { return &chat{r: r, drive: drive, seat: executor.Gated(seat, drive.Gate)} } -// say is one scripted model turn: it writes a file and the seat seals the call. +// say is one scripted model turn: it records a reply, writes a file and seals the call. func (c *chat) say() error { c.calls++ name := filepath.Join(c.r.work, "note"+string(rune('a'+c.calls))+".txt") + // Publication ignores unused chats, so each scripted reply must reach the transcript. + appendTo(c.r.t, transcriptOf(c.r.cell), "{\"type\":\"message\",\"role\":\"assistant\",\"content\":\"fixture turn\"}\n") call := executor.Call{Tool: "bash", Args: []byte(`{"command":"echo"}`)} return c.seat.Around(context.Background(), call, func() ([]byte, bool) { return nil, os.WriteFile(name, []byte("turn "+name), 0o600) != nil @@ -335,6 +337,8 @@ func countFiles(t *testing.T, dir string) (n int) { // C5 also requires the kept work to be announced rather than silently forked. func TestDriveSideSupersededStopsTools(t *testing.T) { r := newDriveRig(t) + // Suppress watches and heartbeats so A can seal its next call before discovering the takeover. + r.a.Dir = offlineDir{Client: r.a.Dir, down: func() bool { return true }} ctx := context.Background() var notices noticeLog c := r.startChat(¬ices) @@ -357,13 +361,14 @@ func TestDriveSideSupersededStopsTools(t *testing.T) { c.mustSay() // A does not know yet; this turn is unpublished want := chatlist.Superseded("laptop") waitFor(t, "A to learn it was superseded", func() bool { _, viewer := c.drive.Viewer(); return viewer }) + // The early viewer notice precedes branching, so wait for the kept-work notice too. + kept := chatlist.KeptEdits(1, "tower") + waitFor(t, "the superseded and kept-work lines to be shown", func() bool { + return notices.last() == want && notices.has(kept) + }) if line, _ := c.drive.Viewer(); line != want { t.Fatalf("line = %q, want %q", line, want) } - waitFor(t, "the superseded line to be shown", func() bool { return notices.last() == want }) - if kept := chatlist.KeptEdits(1, "tower"); !notices.has(kept) { - t.Fatalf("the displaced window silently kept a branch; notices = %v, want %q", notices.all(), kept) - } v, err := r.a.Dir.Cell(ctx, r.cell.ID) if err != nil || v.Cell.Head != durable { diff --git a/internal/syncsetup/twohomes_test.go b/internal/syncsetup/twohomes_test.go index e3e04af372..e7c33fa0a3 100644 --- a/internal/syncsetup/twohomes_test.go +++ b/internal/syncsetup/twohomes_test.go @@ -675,9 +675,11 @@ func TestTwoHomesTakeARunningChatAtOnce(t *testing.T) { } a.mustSay("orphaned") // A does not know it lost the chat; its publish is refused and becomes a branch waitFor(t, "A to learn it was superseded", func() bool { _, viewer := a.drive.Viewer(); return viewer }) - if _, ok := branchRowOf(t, h.a, h.cell.ID); !ok { - t.Fatal("A's refused turns did not become a branch") - } + // The drive refuses new work before its asynchronous branch upload finishes. + waitFor(t, "A's refused turns to become a branch", func() bool { + _, ok := branchRowOf(t, h.a, h.cell.ID) + return ok + }) if err := a.drive.Close(ctx); err != nil { t.Fatal(err) } From a8ddb0dd0091c21884f5c49340cc92e74253775e Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 30 Oct 2026 22:39:02 -0400 Subject: [PATCH 04/18] Never write a new identity into a codeaf home that was deleted A codeaf engine that was still saving after it was asked to stop could find its home deleted and treat that as a first run: identity.EnsureSolo made the folder again and wrote a new identity.json, device.json and identity-solo, and the vault capture then wrote the sync ledgers beside them. The Mac tester saw exactly that 8-22 s after removing a test home. Each codeaf process now records the home it starts in (home.Witness). The identity writers, the vault open, the stats recorder and the send-hold writer refuse with "codeaf home was removed or replaced" once that folder is gone or is a different folder, and they never re-create a recorded root. A first run, where no home existed at start, creates it exactly as before. Co-Authored-By: Claude Opus 5.5 --- cmd/codeaf/main.go | 1 + internal/cellstats/home_removed_test.go | 43 +++++++ internal/cellstats/recorder.go | 13 +- internal/home/witness.go | 135 ++++++++++++++++++++ internal/home/witness_test.go | 69 +++++++++++ internal/identity/device.go | 3 + internal/identity/home_removed_test.go | 158 ++++++++++++++++++++++++ internal/identity/store.go | 48 ++++++- internal/syncsetup/home_removed_test.go | 107 ++++++++++++++++ internal/syncsetup/sendlease.go | 5 +- internal/syncsetup/take.go | 7 ++ 11 files changed, 577 insertions(+), 12 deletions(-) create mode 100644 internal/cellstats/home_removed_test.go create mode 100644 internal/home/witness.go create mode 100644 internal/home/witness_test.go create mode 100644 internal/identity/home_removed_test.go create mode 100644 internal/syncsetup/home_removed_test.go diff --git a/cmd/codeaf/main.go b/cmd/codeaf/main.go index 8ed13bc9c6..ae116ac90c 100644 --- a/cmd/codeaf/main.go +++ b/cmd/codeaf/main.go @@ -53,6 +53,7 @@ func main() { // timers installed by the feature that automations replaced. automation.RemoveOldTimers() } + home.Witness(home.Dir()) registerRunningCLI() os.Exit(execute()) } diff --git a/internal/cellstats/home_removed_test.go b/internal/cellstats/home_removed_test.go new file mode 100644 index 0000000000..fc53743601 --- /dev/null +++ b/internal/cellstats/home_removed_test.go @@ -0,0 +1,43 @@ +package cellstats + +import ( + "os" + "path/filepath" + "testing" + + "github.com/Agent-Field/codeaf/internal/home" +) + +// SPEC-3 item 2: late settlement cannot recreate a removed or replaced home. +func TestFixRound2StatsSettleRefusesRemovedHome(t *testing.T) { + for _, replaced := range []bool{false, true} { + t.Run(map[bool]string{false: "missing", true: "replaced"}[replaced], func(t *testing.T) { + h := filepath.Join(t.TempDir(), "home") + if err := os.Mkdir(h, 0700); err != nil { + t.Fatal(err) + } + home.Witness(h) + r := NewRecorder(h, "chat", MeterFunc(func() Counts { return Counts{Puts: 1} })) + if err := os.Rename(h, h+".old"); err != nil { + t.Fatal(err) + } + if replaced { + if err := os.Mkdir(h, 0700); err != nil { + t.Fatal(err) + } + } + r.Settle() + if r.Err() == nil { + t.Fatal("settled into removed home") + } + if replaced { + entries, err := os.ReadDir(h) + if err != nil || len(entries) != 0 { + t.Fatalf("replacement changed: %v %v", entries, err) + } + } else if _, err := os.Stat(h); !os.IsNotExist(err) { + t.Fatalf("home recreated: %v", err) + } + }) + } +} diff --git a/internal/cellstats/recorder.go b/internal/cellstats/recorder.go index 58ff690dc9..0a04be67c6 100644 --- a/internal/cellstats/recorder.go +++ b/internal/cellstats/recorder.go @@ -6,6 +6,7 @@ import ( "sync" "github.com/Agent-Field/codeaf/internal/cellsync" + "github.com/Agent-Field/codeaf/internal/home" ) // Counts is a snapshot of what one store client has asked of the store so far. @@ -32,6 +33,8 @@ type Recorder struct { Path string Meter Meter // nil records zero request figures + home string + mu sync.Mutex last Counts err error @@ -39,7 +42,7 @@ type Recorder struct { // NewRecorder records one cell's flushes under home. A meter is optional. func NewRecorder(home, cellID string, m Meter) *Recorder { - return &Recorder{Path: Path(home, cellID), Meter: m} + return &Recorder{Path: Path(home, cellID), Meter: m, home: home} } // OnFlush is the cellsync.Batcher hook. It never fails the flush: a stats file @@ -47,7 +50,7 @@ func NewRecorder(home, cellID string, m Meter) *Recorder { func (r *Recorder) OnFlush(f cellsync.Flush) { r.mu.Lock() defer r.mu.Unlock() - r.err = appendLine(r.Path, r.line(f)) + r.err = appendLine(r.home, r.Path, r.line(f)) } // Settle writes what the meter has counted since the last line as a line of its @@ -59,7 +62,7 @@ func (r *Recorder) Settle() { r.mu.Lock() defer r.mu.Unlock() if l := r.line(cellsync.Flush{}); l != (Line{V: Version}) { - r.err = appendLine(r.Path, l) + r.err = appendLine(r.home, r.Path, l) } } @@ -88,8 +91,8 @@ func (r *Recorder) snapshot() Counts { // appendLine adds one line with a single write on an append-only handle, so a // line is never split and an earlier line is never touched. -func appendLine(path string, l Line) error { - if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { +func appendLine(root, path string, l Line) error { + if err := home.MkdirAllUnder(root, filepath.Dir(path), 0o700); err != nil { return err } f, err := os.OpenFile(path, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o600) diff --git a/internal/home/witness.go b/internal/home/witness.go new file mode 100644 index 0000000000..9f70e02d2d --- /dev/null +++ b/internal/home/witness.go @@ -0,0 +1,135 @@ +package home + +import ( + "errors" + "os" + "path/filepath" + "strings" + "sync" +) + +var witnessed sync.Map + +// ErrRemoved refuses late writes into a home this process has lost. +var ErrRemoved = errors.New("codeaf home was removed or replaced; restart codeaf to use the current folder") + +// Witness remembers the first directory this process saw at dir. A later +// missing or replaced root must not be mistaken for a first launch. +func Witness(dir string) { + key := absolute(dir) + if before, ok := witness(dir); ok { + witnessed.LoadOrStore(key, before) + return + } + if info, err := os.Stat(dir); err == nil && info.IsDir() { + before, _ := witnessed.LoadOrStore(canonicalPath(key), info) + witnessed.LoadOrStore(key, before) + } +} + +func witness(dir string) (any, bool) { + key := absolute(dir) + if before, ok := witnessed.Load(key); ok { + return before, true + } + return witnessed.Load(canonicalPath(key)) +} + +// canonicalPath resolves the surviving ancestor when the home itself is gone. +// This preserves aliases such as /tmp and /private/tmp after a removal. +func canonicalPath(dir string) string { + ancestor := dir + var links int + var rest []string + for { + if resolved, err := filepath.EvalSymlinks(ancestor); err == nil { + for i := len(rest) - 1; i >= 0; i-- { + resolved = filepath.Join(resolved, rest[i]) + } + return resolved + } + // A dangling symlink still identifies its missing target. Resolving only + // its parent would turn that alias back into an unwitnessed first use. + if target, err := os.Readlink(ancestor); err == nil { + if links >= 255 { + return dir + } + links++ + if !filepath.IsAbs(target) { + target = filepath.Join(filepath.Dir(ancestor), target) + } + ancestor = target + continue + } + parent := filepath.Dir(ancestor) + if parent == ancestor { + return dir + } + rest = append(rest, filepath.Base(ancestor)) + ancestor = parent + } +} + +// Removed reports whether a witnessed root has disappeared or been replaced. +// Unwitnessed roots remain available for first use. +func Removed(dir string) bool { + before, ok := witness(dir) + if !ok { + return false + } + after, err := os.Stat(dir) + return os.IsNotExist(err) || (err == nil && !os.SameFile(before.(os.FileInfo), after)) +} + +// MkdirAllUnder makes directories without ever creating a witnessed root. +// Making children one at a time lets removal race the write without a recursive +// mkdir silently rebuilding the home. First use retains recursive creation. +func MkdirAllUnder(root, dir string, perm os.FileMode) error { + root, dir = absolute(root), absolute(dir) + rel, err := filepath.Rel(root, dir) + if err != nil { + return err + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return &os.PathError{Op: "mkdir", Path: dir, Err: os.ErrInvalid} + } + Witness(root) + if _, ok := witness(root); !ok { + if err := os.MkdirAll(dir, perm); err != nil { + return err + } + Witness(root) + return nil + } + if rel != "." { + if info, err := os.Stat(dir); err == nil && info.IsDir() { + if Removed(root) { + return ErrRemoved + } + return nil + } + } + if Removed(root) { + return ErrRemoved + } + if rel == "." { + return nil + } + next := root + for _, part := range strings.Split(rel, string(filepath.Separator)) { + next = filepath.Join(next, part) + if err := os.Mkdir(next, perm); err != nil { + if Removed(root) { + return ErrRemoved + } + info, statErr := os.Stat(next) + if !os.IsExist(err) || statErr != nil || !info.IsDir() { + return err + } + } + } + if Removed(root) { + return ErrRemoved + } + return nil +} diff --git a/internal/home/witness_test.go b/internal/home/witness_test.go new file mode 100644 index 0000000000..ca334adb08 --- /dev/null +++ b/internal/home/witness_test.go @@ -0,0 +1,69 @@ +package home + +import ( + "os" + "path/filepath" + "testing" +) + +// SPEC-3 item 4: alias spellings share the witness even after descendants vanish. +func TestFixRound4WitnessAliasesSurviveRemoval(t *testing.T) { + for _, viaAlias := range []bool{false, true} { + t.Run(map[bool]string{false: "physical", true: "alias"}[viaAlias], func(t *testing.T) { + base := t.TempDir() + real := filepath.Join(base, "real") + alias := filepath.Join(base, "alias") + if err := os.Mkdir(real, 0700); err != nil { + t.Fatal(err) + } + if err := os.Symlink(real, alias); err != nil { + t.Fatal(err) + } + physical := filepath.Join(real, "hh", "b") + spelled := filepath.Join(alias, "hh", "b") + if err := os.MkdirAll(physical, 0700); err != nil { + t.Fatal(err) + } + if viaAlias { + Witness(spelled) + } else { + Witness(physical) + } + if Removed(physical) || Removed(spelled) { + t.Fatal("intact alias refused") + } + if err := os.Rename(filepath.Join(real, "hh"), filepath.Join(real, "old")); err != nil { + t.Fatal(err) + } + if !Removed(physical) || !Removed(spelled) { + t.Fatal("alias lost its witness after ancestor removal") + } + if err := os.MkdirAll(physical, 0700); err != nil { + t.Fatal(err) + } + if !Removed(physical) || !Removed(spelled) { + t.Fatal("replacement accepted through alias") + } + }) + } +} + +// SPEC-3 item 4: a dangling symlink still names the witnessed home it pointed at. +func TestFixRound4DanglingHomeAliasKeepsWitness(t *testing.T) { + base := t.TempDir() + real := filepath.Join(base, "real") + alias := filepath.Join(base, "alias") + if err := os.Mkdir(real, 0700); err != nil { + t.Fatal(err) + } + if err := os.Symlink("real", alias); err != nil { + t.Fatal(err) + } + Witness(real) + if err := os.Rename(real, real+".old"); err != nil { + t.Fatal(err) + } + if !Removed(alias) { + t.Fatal("dangling alias forgot the witnessed target") + } +} diff --git a/internal/identity/device.go b/internal/identity/device.go index 6c8f2124b1..de1669b563 100644 --- a/internal/identity/device.go +++ b/internal/identity/device.go @@ -84,6 +84,9 @@ func Device(home string) (Dev, error) { // deviceFor returns the stored device when its cert verifies under id, else a // fresh device. A replaced identity therefore never keeps a stale device. func deviceFor(home string, id Identity) (Dev, error) { + if err := checkHome(home); err != nil { + return Dev{}, err + } if d, err := LoadDevice(home); err == nil && d.Cert.Verify(id.PublicKey()) == nil { return d, nil } diff --git a/internal/identity/home_removed_test.go b/internal/identity/home_removed_test.go new file mode 100644 index 0000000000..207c512a26 --- /dev/null +++ b/internal/identity/home_removed_test.go @@ -0,0 +1,158 @@ +package identity + +import ( + "errors" + "os" + "path/filepath" + "testing" + + statehome "github.com/Agent-Field/codeaf/internal/home" +) + +// C4.1: every identity and device writer refuses a removed or replaced root. +func TestIdentityRefusesRemovedHome(t *testing.T) { + for _, replaced := range []bool{false, true} { + t.Run(map[bool]string{false: "missing", true: "replaced"}[replaced], func(t *testing.T) { + h := filepath.Join(t.TempDir(), "home") + id, err := EnsureSolo(h) + if err != nil { + t.Fatal(err) + } + if err := os.Rename(h, h+".old"); err != nil { + t.Fatal(err) + } + if replaced { + if err := os.Mkdir(h, 0o700); err != nil { + t.Fatal(err) + } + } + writers := map[string]func() error{ + "solo": func() error { _, err := EnsureSolo(h); return err }, + "ensure": func() error { _, err := Ensure(h); return err }, + "root": func() error { _, err := ensureRoot(h); return err }, + "save": func() error { return Save(h, id) }, + "device": func() error { _, err := deviceFor(h, id); return err }, + } + for name, write := range writers { + if err := write(); !errors.Is(err, ErrHomeRemoved) { + t.Errorf("%s: %v", name, err) + } + } + if replaced { + entries, err := os.ReadDir(h) + if err != nil || len(entries) != 0 { + t.Fatalf("replacement changed: %v, %v", entries, err) + } + } else if _, err := os.Stat(h); !os.IsNotExist(err) { + t.Fatalf("home recreated: %v", err) + } + }) + } +} + +// C4.2: first use still creates keys, and witnessing an intact home permits loads and saves. +func TestIdentityFirstUseAndIntactWitness(t *testing.T) { + h := filepath.Join(t.TempDir(), "home") + statehome.Witness(h) + id, err := EnsureSolo(h) + if err != nil { + t.Fatal(err) + } + if !Solo(h) { + t.Fatal("first use lost its solo marker") + } + statehome.Witness(h) + loaded, err := EnsureSolo(h) + if err != nil || loaded.ID() != id.ID() { + t.Fatalf("identity changed: %v", err) + } + dev, err := Device(h) + if err != nil { + t.Fatal(err) + } + if err := Save(h, id); err != nil { + t.Fatal(err) + } + again, err := Device(h) + if err != nil || again.ID() != dev.ID() { + t.Fatalf("device changed: %v", err) + } + if loaded, err := Load(h); err != nil || loaded.ID() != id.ID() { + t.Fatalf("saved identity: %v", err) + } +} + +// SPEC-3 item 1: removal after the writer's check is still a removed-home error. +func TestFixRound1IdentityWriteMapsConcurrentRemoval(t *testing.T) { + for _, replaced := range []bool{false, true} { + t.Run(map[bool]string{false: "missing", true: "replaced"}[replaced], func(t *testing.T) { + h := filepath.Join(t.TempDir(), "home") + if _, err := Ensure(h); err != nil { + t.Fatal(err) + } + err := write(filepath.Join(h, DeviceFile), removeHomeDuringMarshal{home: h, replaced: replaced}, os.Rename) + if !errors.Is(err, ErrHomeRemoved) { + t.Fatalf("concurrent removal = %v", err) + } + if replaced { + entries, err := os.ReadDir(h) + if err != nil || len(entries) != 0 { + t.Fatalf("replacement changed: %v %v", entries, err) + } + } else if _, err := os.Stat(h); !os.IsNotExist(err) { + t.Fatalf("home recreated: %v", err) + } + }) + } +} + +type removeHomeDuringMarshal struct { + home string + replaced bool +} + +func (v removeHomeDuringMarshal) MarshalJSON() ([]byte, error) { + if err := os.Rename(v.home, v.home+".old"); err != nil { + return nil, err + } + if v.replaced { + if err := os.Mkdir(v.home, 0700); err != nil { + return nil, err + } + } + return []byte(`{}`), nil +} + +// SPEC-3 item 1: removal at publication maps the filesystem failure to the home error. +func TestFixRound1IdentityWriteMapsRemovalDuringPublish(t *testing.T) { + for _, replaced := range []bool{false, true} { + t.Run(map[bool]string{false: "missing", true: "replaced"}[replaced], func(t *testing.T) { + h := filepath.Join(t.TempDir(), "home") + if _, err := Ensure(h); err != nil { + t.Fatal(err) + } + err := write(filepath.Join(h, DeviceFile), struct{}{}, func(from, to string) error { + if err := os.Rename(h, h+".old"); err != nil { + return err + } + if replaced { + if err := os.Mkdir(h, 0700); err != nil { + return err + } + } + return os.Rename(from, to) + }) + if !errors.Is(err, ErrHomeRemoved) { + t.Fatalf("publication after removal = %v", err) + } + if replaced { + entries, err := os.ReadDir(h) + if err != nil || len(entries) != 0 { + t.Fatalf("replacement changed: %v %v", entries, err) + } + } else if _, err := os.Stat(h); !os.IsNotExist(err) { + t.Fatalf("home recreated: %v", err) + } + }) + } +} diff --git a/internal/identity/store.go b/internal/identity/store.go index ea80cb86b9..b0d0c6fcef 100644 --- a/internal/identity/store.go +++ b/internal/identity/store.go @@ -9,6 +9,8 @@ import ( "os" "path/filepath" "strings" + + statehome "github.com/Agent-Field/codeaf/internal/home" ) const ( @@ -21,6 +23,16 @@ const ( // ErrNone reports that no identity exists under the home yet. var ErrNone = errors.New("identity: none on this machine") +// ErrHomeRemoved refuses to make new keys in a home this process has lost. +var ErrHomeRemoved = statehome.ErrRemoved + +func checkHome(home string) error { + if statehome.Removed(home) { + return ErrHomeRemoved + } + return nil +} + func path(home string) string { return filepath.Join(home, File) } // Load reads the identity under home, or ErrNone. @@ -53,11 +65,16 @@ func Ensure(home string) (Identity, error) { // EnsureSolo is Ensure for a computer that needs an identity for itself (a // vault key, a first launch) and has not been asked to share it: an identity // it makes is solo (solo.go). One that already exists is left as it is. -func EnsureSolo(home string) (Identity, error) { +func EnsureSolo(home string) (id Identity, err error) { + defer func() { err = homeWriteError(home, err) }() + if err := checkHome(home); err != nil { + return Identity{}, err + } if _, err := Load(home); errors.Is(err, ErrNone) { - if err := os.MkdirAll(home, 0o700); err != nil { + if err := statehome.MkdirAllUnder(home, home, 0o700); err != nil { return Identity{}, err } + statehome.Witness(home) if err := markSolo(home); err != nil { return Identity{}, err } @@ -69,12 +86,17 @@ func EnsureSolo(home string) (Identity, error) { // that already has a vault.key keeps it as the cell key, so its vault still // opens. Two processes racing to create agree on whichever wrote first. func ensureRoot(home string) (Identity, error) { + if err := checkHome(home); err != nil { + return Identity{}, err + } + statehome.Witness(home) if id, err := Load(home); !errors.Is(err, ErrNone) { return id, err } - if err := os.MkdirAll(home, 0o700); err != nil { + if err := statehome.MkdirAllUnder(home, home, 0o700); err != nil { return Identity{}, err } + statehome.Witness(home) cell, err := inheritedCellKey(home) if err != nil { return Identity{}, err @@ -92,9 +114,13 @@ func ensureRoot(home string) (Identity, error) { // Save replaces the identity under home. func Save(home string, id Identity) error { - if err := os.MkdirAll(home, 0o700); err != nil { + if err := checkHome(home); err != nil { + return err + } + if err := statehome.MkdirAllUnder(home, home, 0o700); err != nil { return err } + statehome.Witness(home) return write(path(home), id.document(), fsreplace.Rename) } @@ -121,11 +147,15 @@ func inheritedCellKey(home string) (key, error) { // write puts v as JSON at target through a private temp file and publish, // which is fsreplace.Rename to replace and os.Link to refuse replacing. -func write(target string, v any, publish func(from, to string) error) error { +func write(target string, v any, publish func(from, to string) error) (err error) { + defer func() { err = homeWriteError(filepath.Dir(target), err) }() raw, err := json.Marshal(v) if err != nil { return err } + if err := checkHome(filepath.Dir(target)); err != nil { + return err + } tmp, err := privateTemp(filepath.Dir(target), raw) if err != nil { return err @@ -152,3 +182,11 @@ func writeSync(f *os.File, data []byte) error { serr := f.Sync() return errors.Join(werr, serr, f.Close()) } + +// homeWriteError preserves the removed-home contract when removal follows a check. +func homeWriteError(home string, err error) error { + if errors.Is(err, fs.ErrNotExist) && statehome.Removed(home) { + return ErrHomeRemoved + } + return err +} diff --git a/internal/syncsetup/home_removed_test.go b/internal/syncsetup/home_removed_test.go new file mode 100644 index 0000000000..209e3f01b1 --- /dev/null +++ b/internal/syncsetup/home_removed_test.go @@ -0,0 +1,107 @@ +package syncsetup + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "github.com/Agent-Field/codeaf/internal/cell" + "github.com/Agent-Field/codeaf/internal/cellstore" + "github.com/Agent-Field/codeaf/internal/identity" + "github.com/Agent-Field/codeaf/internal/vaultsync" +) + +// C4.1: a late .env import must leave a removed home absent. +func TestGuardLeavesRemovedHomeAbsent(t *testing.T) { + h := filepath.Join(t.TempDir(), "home") + if _, err := identity.Ensure(h); err != nil { + t.Fatal(err) + } + c, err := cell.Create(h, cell.Options{Class: cell.HostBound}) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(c.Root, ".env"), []byte("L6_FIXTURE=local\n"), 0o600); err != nil { + t.Fatal(err) + } + deleted := false + g := cellstore.Guard{Home: h, Notify: func(string) { + if !deleted { + deleted = true + if err := os.RemoveAll(h); err != nil { + t.Fatal(err) + } + } + }} + // Screening can refuse the import or keep going without it; neither may create keys. + _ = g.Screen(c, c.Root, filepath.Join(c.Root, cell.StateDir), []string{".env"}) + if !deleted { + t.Fatal("fixture never found .env") + } + if _, err := os.Stat(h); !os.IsNotExist(err) { + t.Fatalf("removed home was recreated: %v", err) + } +} + +// C4.1: capture refuses a replacement root before creating even its lock file. +func TestVaultCaptureLeavesReplacedHomeUntouched(t *testing.T) { + h := filepath.Join(t.TempDir(), "home") + if _, err := identity.Ensure(h); err != nil { + t.Fatal(err) + } + if err := os.Rename(h, h+".old"); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(h, 0o700); err != nil { + t.Fatal(err) + } + s := &Sync{Home: h} + err := s.withVault(func(vaultsync.Syncer) error { t.Fatal("capture reached replacement home"); return nil }) + if !errors.Is(err, identity.ErrHomeRemoved) { + t.Fatalf("capture = %v", err) + } + entries, err := os.ReadDir(h) + if err != nil || len(entries) != 0 { + t.Fatalf("capture changed replacement: %v, %v", entries, err) + } +} + +// SPEC-3 item 2: writing or retiring a send hold cannot recreate a lost root. +func TestFixRound2SendHoldRefusesRemovedHome(t *testing.T) { + for _, replaced := range []bool{false, true} { + t.Run(map[bool]string{false: "missing", true: "replaced"}[replaced], func(t *testing.T) { + h := filepath.Join(t.TempDir(), "home") + id, err := identity.Ensure(h) + if err != nil { + t.Fatal(err) + } + dev, err := identity.Device(h) + if err != nil { + t.Fatal(err) + } + s := &Sync{Home: h, Identity: id, Device: dev, Ledger: "local"} + if err := os.Rename(h, h+".old"); err != nil { + t.Fatal(err) + } + if replaced { + if err := os.Mkdir(h, 0700); err != nil { + t.Fatal(err) + } + } + held := sendLease{Cell: "chat", Fence: 1} + if err := s.recordSendLease("chat", held); err == nil { + t.Fatal("wrote hold into removed home") + } + _ = s.forgetSendLease("chat", held) + if replaced { + entries, err := os.ReadDir(h) + if err != nil || len(entries) != 0 { + t.Fatalf("replacement changed: %v %v", entries, err) + } + } else if _, err := os.Stat(h); !os.IsNotExist(err) { + t.Fatalf("home recreated: %v", err) + } + }) + } +} diff --git a/internal/syncsetup/sendlease.go b/internal/syncsetup/sendlease.go index 63a89b19d1..b78455918f 100644 --- a/internal/syncsetup/sendlease.go +++ b/internal/syncsetup/sendlease.go @@ -11,6 +11,7 @@ import ( "github.com/Agent-Field/codeaf/internal/directory" "github.com/Agent-Field/codeaf/internal/filelock" "github.com/Agent-Field/codeaf/internal/fsreplace" + "github.com/Agent-Field/codeaf/internal/home" ) type sendLease struct { @@ -85,7 +86,7 @@ func (s *Sync) recordSendLease(id string, held sendLease) error { return err } path := s.sendLeasePath(id) - if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + if err := home.MkdirAllUnder(s.Home, filepath.Dir(path), 0700); err != nil { return err } tmp, err := os.CreateTemp(filepath.Dir(path), ".hold-*") @@ -138,7 +139,7 @@ func (s *Sync) forgetSendLease(id string, expected sendLease) error { func (s *Sync) lockSendLease(id string) (*os.File, error) { path := s.sendLeasePath(id) - if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + if err := home.MkdirAllUnder(s.Home, filepath.Dir(path), 0700); err != nil { return nil, err } lock, err := os.OpenFile(path+".lock", os.O_CREATE|os.O_RDWR, 0600) diff --git a/internal/syncsetup/take.go b/internal/syncsetup/take.go index 88706bb97f..0f343c7ee6 100644 --- a/internal/syncsetup/take.go +++ b/internal/syncsetup/take.go @@ -18,6 +18,8 @@ import ( "github.com/Agent-Field/codeaf/internal/directory" "github.com/Agent-Field/codeaf/internal/filelock" "github.com/Agent-Field/codeaf/internal/handoff" + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/identity" "github.com/Agent-Field/codeaf/internal/inventory" "github.com/Agent-Field/codeaf/internal/keys" "github.com/Agent-Field/codeaf/internal/preflight" @@ -642,6 +644,11 @@ func (s *Sync) files(eng cellstore.Engine, c cell.Cell) (vaultsync.Files, error) func (s *Sync) withVault(use func(vaultsync.Syncer) error, extra ...vaultsync.Carrier) error { stateMu.Lock() defer stateMu.Unlock() + // Refuse before opening the lock file as well as before creating keys, so + // a replacement directory receives none of the previous home's state. + if home.Removed(s.Home) { + return identity.ErrHomeRemoved + } lock, err := os.OpenFile(filepath.Join(s.Home, "state-sync.lock"), os.O_CREATE|os.O_RDWR, 0600) if err != nil { return err From 65f4918b53689c2a9d43e4e2f4600adeac18d4fe Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 30 Oct 2026 22:39:09 -0400 Subject: [PATCH 05/18] A stopping engine ends at once when its home is deleted An engine asked to stop closes every conversation before it exits, and for a synced chat that can take tens of seconds while the relay is unreachable. If the home was removed meanwhile, everything it still wrote went into a folder that no longer existed. While shutting down, the host now watches its home; if it is removed or replaced, the engine exits within about a tenth of a second instead of saving into it. With the home intact the stop does what it did before, in the same order and with the same limits. Co-Authored-By: Claude Opus 5.5 --- cmd/codeaf/engine.go | 3 +- internal/enginehost/home_removed_test.go | 88 ++++++++++++++++++++++++ internal/enginehost/host.go | 29 ++++++++ 3 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 internal/enginehost/home_removed_test.go diff --git a/cmd/codeaf/engine.go b/cmd/codeaf/engine.go index e4ed320811..c1fe33eca2 100644 --- a/cmd/codeaf/engine.go +++ b/cmd/codeaf/engine.go @@ -945,7 +945,8 @@ func runEngineHost(workspaceFlag, sessionFlag string) error { return fmt.Errorf("open %s: %w", workspace, err) } err = enginehost.Run(workspace, enginehost.Options{ - Cells: cell.Available(), + HomeRemoved: func() { os.Exit(0) }, + Cells: cell.Available(), Boot: func(hello remote.Hello) (*remote.Engine, error) { return bootEngine(hello, workspace, sessionFlag) }, // usage writer owner: see CloseUsage after the host returns diff --git a/internal/enginehost/home_removed_test.go b/internal/enginehost/home_removed_test.go new file mode 100644 index 0000000000..d0dfa6e3d1 --- /dev/null +++ b/internal/enginehost/home_removed_test.go @@ -0,0 +1,88 @@ +package enginehost + +import ( + "net" + "os" + "path/filepath" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/remote" +) + +type closingHomeAgent struct { + stubAgent + closing chan struct{} + release chan struct{} +} + +func (a *closingHomeAgent) Close() error { + close(a.closing) + <-a.release + return nil +} + +// C4.3: removal during a blocked close calls the command's exit hook promptly; +// an intact home keeps waiting for the same close, without calling the hook. +func TestShutdownExitsOnlyWhenHomeRemoved(t *testing.T) { + for _, removed := range []bool{false, true} { + t.Run(map[bool]string{false: "intact", true: "removed"}[removed], func(t *testing.T) { + root := filepath.Join(t.TempDir(), "home") + if err := os.Mkdir(root, 0o700); err != nil { + t.Fatal(err) + } + t.Setenv(home.EnvVar, root) + home.Witness(root) + dir := filepath.Join(root, "host") + if err := os.Mkdir(dir, 0o700); err != nil { + t.Fatal(err) + } + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + defer listener.Close() + agent := &closingHomeAgent{closing: make(chan struct{}), release: make(chan struct{})} + defer close(agent.release) + exit := make(chan struct{}, 1) + h := &Host{dir: dir, listener: listener, opts: Options{HomeRemoved: func() { exit <- struct{}{} }}, + sessions: map[string]*remote.Session{"chat": remote.NewSession(&remote.Engine{Agent: agent}, true)}} + finished := make(chan struct{}) + go func() { h.shutdown(); close(finished) }() + select { + case <-agent.closing: + case <-time.After(time.Second): + t.Fatal("close never started") + } + if removed { + if err := os.RemoveAll(root); err != nil { + t.Fatal(err) + } + select { + case <-exit: + case <-time.After(time.Second): + t.Fatal("exit hook did not run after removal") + } + if _, err := os.Stat(root); !os.IsNotExist(err) { + t.Fatalf("home recreated: %v", err) + } + } else { + select { + case <-exit: + t.Fatal("exit hook called for intact home") + case <-finished: + t.Fatal("close returned before release") + case <-time.After(200 * time.Millisecond): + } + } + // Release and join before the next fixture changes the process environment. + agent.release <- struct{}{} + select { + case <-finished: + case <-time.After(time.Second): + t.Fatal("close did not finish") + } + }) + } +} diff --git a/internal/enginehost/host.go b/internal/enginehost/host.go index 78a4382df1..91f24d2952 100644 --- a/internal/enginehost/host.go +++ b/internal/enginehost/host.go @@ -24,6 +24,7 @@ import ( "github.com/Agent-Field/codeaf/internal/filelock" "github.com/Agent-Field/codeaf/internal/guard" + "github.com/Agent-Field/codeaf/internal/home" "github.com/Agent-Field/codeaf/internal/remote" ) @@ -124,6 +125,10 @@ type Options struct { // told to every asker ([remote.HostSelf.Cells]) so a door in the other mode // can replace an idle host or refuse a busy one instead of attaching. Cells bool + + // HomeRemoved ends the process when shutdown can no longer save into its + // original home. The command owns process exit; libraries only call this hook. + HomeRemoved func() } // Host is one workspace's conversations, and the socket they are reached @@ -819,6 +824,30 @@ func (h *Host) stopped() bool { // not wait at all. A second host already saving holds it too; this one then // saves without it. func (h *Host) shutdown() { + if h.opts.HomeRemoved != nil { + root := home.Dir() + if home.Removed(root) { + h.opts.HomeRemoved() + return + } + finished := make(chan struct{}) + defer close(finished) + go func() { + ticker := time.NewTicker(100 * time.Millisecond) + defer ticker.Stop() + for { + select { + case <-finished: + return + case <-ticker.C: + if home.Removed(root) { + h.opts.HomeRemoved() + return + } + } + } + }() + } h.mu.Lock() h.closed = true sessions := make([]*remote.Session, 0, len(h.sessions)) From d9839acd4dcf04d494cca0e17f22bd329f7868d9 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 30 Oct 2026 22:39:09 -0400 Subject: [PATCH 06/18] Bound how long a busy engine store spends naming its holder #2078 named the process holding a busy engine store. On macOS it asked lsof about every lock file it found, 500 ms each with no cap, so a failing seal could add seconds. One shared budget now covers the whole lookup, and it stops at the first busy holder; when the budget runs out the message is the plain busy sentence. Requests that are not busy never inspect anything. Co-Authored-By: Claude Opus 5.5 --- internal/cellstore/store_busy.go | 29 +++++++--- internal/cellstore/store_busy_budget_test.go | 60 ++++++++++++++++++++ internal/cellstore/store_busy_unix.go | 48 +++++++++------- internal/cellstore/store_busy_windows.go | 11 ++-- 4 files changed, 115 insertions(+), 33 deletions(-) create mode 100644 internal/cellstore/store_busy_budget_test.go diff --git a/internal/cellstore/store_busy.go b/internal/cellstore/store_busy.go index d4cd9772ab..1112940d84 100644 --- a/internal/cellstore/store_busy.go +++ b/internal/cellstore/store_busy.go @@ -1,6 +1,7 @@ package cellstore import ( + "context" "encoding/json" "errors" "fmt" @@ -10,6 +11,7 @@ import ( "strconv" "strings" "sync" + "time" "github.com/Agent-Field/codeaf/internal/filelock" "github.com/Agent-Field/codeaf/internal/home" @@ -19,8 +21,9 @@ import ( var ErrStoreBusy = errors.New("engine store is busy") const ( - storeWriterLockName = "writer.lock" - maxLockPayloadBytes = 4 << 10 + storeWriterLockName = "writer.lock" + maxLockPayloadBytes = 4 << 10 + storeBusyInspectWithin = time.Second ) // StoreHolder identifies the process holding a store writer lock. @@ -122,15 +125,25 @@ func HoldStoreWriter(dataDir, command string) (func() error, error) { }, nil } +var inspectStoreLock = probeStoreLockBusy + // InspectStoreBusy checks whether another live process holds an advisory writer // lock on dataDir. A daemon serving ownSocket is treated as this client's own // transport rather than a competing holder. func InspectStoreBusy(dataDir, ownSocket string) (StoreHolder, bool) { + ctx, cancel := context.WithTimeout(context.Background(), storeBusyInspectWithin) + defer cancel() if strings.TrimSpace(dataDir) == "" { return StoreHolder{}, false } for _, lockPath := range storeLockCandidates(dataDir, ownSocket) { - holder, busy := probeStoreLockBusy(lockPath, ownSocket) + if ctx.Err() != nil { + break + } + holder, busy := inspectStoreLock(ctx, lockPath, ownSocket) + if ctx.Err() != nil { + return StoreHolder{}, busy + } if busy { return holder, true } @@ -201,7 +214,7 @@ func storeLockCandidates(dataDir, ownSocket string) []string { return paths } -func probeStoreLockBusy(lockPath, ownSocket string) (StoreHolder, bool) { +func probeStoreLockBusy(ctx context.Context, lockPath, ownSocket string) (StoreHolder, bool) { if strings.HasSuffix(lockPath, ".sock.lock") { // An active engine daemon socket lock is the normal daemon lease, not a // stuck store writer, unless its socket file is missing. @@ -229,11 +242,11 @@ func probeStoreLockBusy(lockPath, ownSocket string) (StoreHolder, bool) { return StoreHolder{}, false } - holder := lockHolderFromOS(lockPath) + holder := lockHolderFromOS(ctx, lockPath) if holder.PID == os.Getpid() { return StoreHolder{}, false } - if holder.PID > 0 && pidServesSocket(holder.PID, ownSocket) { + if holder.PID > 0 && pidServesSocket(ctx, holder.PID, ownSocket) { return StoreHolder{}, false } if holder.PID == 0 || holder.Command == "" { @@ -241,7 +254,7 @@ func probeStoreLockBusy(lockPath, ownSocket string) (StoreHolder, bool) { if recorded.PID == os.Getpid() { return StoreHolder{}, false } - if recorded.PID > 0 && pidServesSocket(recorded.PID, ownSocket) { + if recorded.PID > 0 && pidServesSocket(ctx, recorded.PID, ownSocket) { return StoreHolder{}, false } if holder.PID == 0 { @@ -252,7 +265,7 @@ func probeStoreLockBusy(lockPath, ownSocket string) (StoreHolder, bool) { } } if holder.Command == "" && holder.PID > 0 { - holder.Command = describeProcess(holder.PID) + holder.Command = describeProcess(ctx, holder.PID) } holder.Lock = lockPath return holder, true diff --git a/internal/cellstore/store_busy_budget_test.go b/internal/cellstore/store_busy_budget_test.go new file mode 100644 index 0000000000..01b8f11b6a --- /dev/null +++ b/internal/cellstore/store_busy_budget_test.go @@ -0,0 +1,60 @@ +package cellstore + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "testing" + "time" +) + +// C4.4: classification shares one total budget across all candidates, and +// preserves the plain busy sentence when no holder can be found in time. +func TestStoreBusyClassificationSharesOneBudget(t *testing.T) { + root := t.TempDir() + for i := 0; i < 30; i++ { + if err := os.WriteFile(filepath.Join(root, fmt.Sprintf("%02d.lock", i)), nil, 0o600); err != nil { + t.Fatal(err) + } + } + old := inspectStoreLock + t.Cleanup(func() { inspectStoreLock = old }) + calls := 0 + inspectStoreLock = func(ctx context.Context, _, _ string) (StoreHolder, bool) { + calls++ + select { + case <-time.After(storeBusyInspectWithin / 4): + return StoreHolder{}, false + case <-ctx.Done(): + return StoreHolder{}, false + } + } + start := time.Now() + err := classifyStoreBusyError(root, "", errors.New("engine store is busy")) + if elapsed := time.Since(start); elapsed > storeBusyInspectWithin+500*time.Millisecond { + t.Fatalf("classification took %s, budget %s", elapsed, storeBusyInspectWithin) + } + if calls >= 30 || calls == 0 { + t.Fatalf("inspected %d candidates", calls) + } + if err.Error() != "engine store is busy" { + t.Fatal(err) + } + // A timely holder ends the scan immediately and keeps the existing message. + calls = 0 + inspectStoreLock = func(context.Context, string, string) (StoreHolder, bool) { + calls++ + return StoreHolder{PID: 42, Command: "codeaf chat"}, true + } + err = classifyStoreBusyError(root, "", errors.New("engine store is busy")) + if calls != 1 || err.Error() != "engine store is busy (held by codeaf chat, pid 42)" { + t.Fatalf("calls=%d: %v", calls, err) + } + calls = 0 + _ = classifyStoreBusyError(root, "", errors.New("unrelated failure")) + if calls != 0 { + t.Fatal("healthy path inspected locks") + } +} diff --git a/internal/cellstore/store_busy_unix.go b/internal/cellstore/store_busy_unix.go index 62cea8e336..31823c3ecf 100644 --- a/internal/cellstore/store_busy_unix.go +++ b/internal/cellstore/store_busy_unix.go @@ -13,7 +13,6 @@ import ( "strconv" "strings" "syscall" - "time" "golang.org/x/sys/unix" @@ -28,8 +27,8 @@ func storeProcessAlive(pid int) bool { return err == nil || errors.Is(err, syscall.EPERM) } -func pidServesSocket(pid int, socket string) bool { - uid, argv := storageProcess(context.Background(), pid) +func pidServesSocket(ctx context.Context, pid int, socket string) bool { + uid, argv := storageProcess(ctx, pid) ownUID := strconv.Itoa(os.Getuid()) if socket != "" && daemonServesSocket(uid, ownUID, argv, socket) { return true @@ -46,19 +45,19 @@ func pidServesSocket(pid int, socket string) bool { return filepath.Dir(filepath.Clean(served)) == filepath.Clean(home.Join("v3")) } -func describeProcess(pid int) string { - _, argv := storageProcess(context.Background(), pid) +func describeProcess(ctx context.Context, pid int) string { + _, argv := storageProcess(ctx, pid) return formatHolderCommand(argv) } -func lockHolderFromOS(lockPath string) StoreHolder { +func lockHolderFromOS(ctx context.Context, lockPath string) StoreHolder { if runtime.GOOS == "linux" { - return lockHolderLinux(lockPath) + return lockHolderLinux(ctx, lockPath) } - return lockHolderDarwin(lockPath) + return lockHolderDarwin(ctx, lockPath) } -func lockHolderLinux(lockPath string) StoreHolder { +func lockHolderLinux(ctx context.Context, lockPath string) StoreHolder { info, err := os.Stat(lockPath) if err != nil { return StoreHolder{} @@ -67,17 +66,17 @@ func lockHolderLinux(lockPath string) StoreHolder { if !ok { return StoreHolder{} } - pid := pidFromProcLocks(uint64(stat.Dev), stat.Ino) + pid := pidFromProcLocks(ctx, uint64(stat.Dev), stat.Ino) if pid <= 0 { - pid = pidFromProcFDs(info) + pid = pidFromProcFDs(ctx, info) } if pid <= 0 { return StoreHolder{} } - return resolveUnixHolder(pid) + return resolveUnixHolder(ctx, pid) } -func pidFromProcLocks(dev, ino uint64) int { +func pidFromProcLocks(ctx context.Context, dev, ino uint64) int { file, err := os.Open("/proc/locks") if err != nil { return 0 @@ -90,6 +89,9 @@ func pidFromProcLocks(dev, ino uint64) int { scanner := bufio.NewScanner(file) for scanner.Scan() { + if ctx.Err() != nil { + return 0 + } fields := strings.Fields(scanner.Text()) // Standard format: "1: FLOCK ADVISORY WRITE :: 0 EOF" // Blocked waiters carry "->" after the index ("1: -> FLOCK ..."); skip them. @@ -120,7 +122,7 @@ func pidFromProcLocks(dev, ino uint64) int { return 0 } -func pidFromProcFDs(target os.FileInfo) int { +func pidFromProcFDs(ctx context.Context, target os.FileInfo) int { entries, err := os.ReadDir("/proc") if err != nil { return 0 @@ -128,6 +130,9 @@ func pidFromProcFDs(target os.FileInfo) int { ownUID := uint32(os.Getuid()) ownPID := os.Getpid() for _, entry := range entries { + if ctx.Err() != nil { + return 0 + } if !entry.IsDir() { continue } @@ -148,6 +153,9 @@ func pidFromProcFDs(target os.FileInfo) int { continue } for _, fd := range fds { + if ctx.Err() != nil { + return 0 + } fdInfo, err := os.Stat(filepath.Join(procDir, "fd", fd.Name())) if err == nil && os.SameFile(target, fdInfo) { return pid @@ -157,14 +165,14 @@ func pidFromProcFDs(target os.FileInfo) int { return 0 } -func resolveUnixHolder(pid int) StoreHolder { - _, argv := storageProcess(context.Background(), pid) +func resolveUnixHolder(ctx context.Context, pid int) StoreHolder { + _, argv := storageProcess(ctx, pid) // A one-shot furrow verb belongs to the codeaf process that launched it; a // furrow serve daemon is identified by its own socket so this home's normal // writer is never mistaken for a competing codeaf process. if len(argv) > 0 && storageProgramName.MatchString(filepath.Base(argv[0])) && (len(argv) < 2 || argv[1] != "serve") { if ppid := linuxParentPID(pid); ppid > 1 && ppid != os.Getpid() { - _, parentArgv := storageProcess(context.Background(), ppid) + _, parentArgv := storageProcess(ctx, ppid) if len(parentArgv) > 0 { base := filepath.Base(parentArgv[0]) if base == "codeaf" || strings.HasPrefix(base, "codeaf-") { @@ -193,9 +201,7 @@ func linuxParentPID(pid int) int { return 0 } -func lockHolderDarwin(lockPath string) StoreHolder { - ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) - defer cancel() +func lockHolderDarwin(ctx context.Context, lockPath string) StoreHolder { raw, err := exec.CommandContext(ctx, "lsof", "-n", "-P", "-F", "pc", "--", lockPath).Output() //codeaf:plumbing inspects which local process holds an advisory store lock on macOS if err != nil { return StoreHolder{} @@ -219,7 +225,7 @@ func lockHolderDarwin(lockPath string) StoreHolder { if pid <= 0 { return StoreHolder{} } - holder := resolveUnixHolder(pid) + holder := resolveUnixHolder(ctx, pid) if holder.Command == "" { holder.Command = cmd } diff --git a/internal/cellstore/store_busy_windows.go b/internal/cellstore/store_busy_windows.go index 093a0c1258..00aac03b62 100644 --- a/internal/cellstore/store_busy_windows.go +++ b/internal/cellstore/store_busy_windows.go @@ -2,7 +2,10 @@ package cellstore -import "golang.org/x/sys/windows" +import ( + "context" + "golang.org/x/sys/windows" +) const windowsStillActive = 259 @@ -22,8 +25,8 @@ func storeProcessAlive(pid int) bool { return code == windowsStillActive } -func pidServesSocket(int, string) bool { return false } +func pidServesSocket(context.Context, int, string) bool { return false } -func describeProcess(int) string { return "" } +func describeProcess(context.Context, int) string { return "" } -func lockHolderFromOS(string) StoreHolder { return StoreHolder{} } +func lockHolderFromOS(context.Context, string) StoreHolder { return StoreHolder{} } From f9bb2e77b8c12989f16fb8610c5341b24f0c2c03 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 30 Oct 2026 22:39:16 -0400 Subject: [PATCH 07/18] A chat made with chat --once always reaches your other computers runChatV3Once returned before the interactive road registered its process cleanup, so a paired --once never closed its sync drive. Publishing uploads a chat's frames before it registers the chat with the relay, and a --once that exited in between left the uploads with no record pointing at them: the other computer never listed the chat, and doctor said nothing (Hello saw it in 1 of 48 runs, about 1 s after pairing). --once now always registers drive cleanup; with no drive it returns at once, and a paired chat gets its final send and release inside the same limit the interactive road uses, with one plain stderr sentence if that fails. A paired drive records a small local publication marker, so codeaf doctor can report a chat this computer started that the relay does not list, and doctor --fix registers it through the ordinary road, create-only, never over another computer's record and never for a chat put away meanwhile. A never-paired computer writes no marker and makes no relay request. Co-Authored-By: Claude Opus 5.5 --- cmd/codeaf/chatv3.go | 17 +- cmd/codeaf/doctor_publication.go | 145 +++++++ cmd/codeaf/doctor_repair.go | 6 + cmd/codeaf/once_publication_test.go | 393 ++++++++++++++++++ cmd/codeaf/publication_manual_test.go | 61 +++ cmd/codeaf/publication_round_test.go | 402 +++++++++++++++++++ cmd/codeaf/syncdrive.go | 17 +- internal/cellstore/first_turn_test.go | 37 ++ internal/cellstore/log.go | 26 ++ internal/cellsync/create_only_test.go | 35 ++ internal/cellsync/publish.go | 5 +- internal/syncsetup/drive.go | 34 +- internal/syncsetup/publication.go | 105 +++++ internal/syncsetup/publication_round_test.go | 151 +++++++ 14 files changed, 1418 insertions(+), 16 deletions(-) create mode 100644 cmd/codeaf/doctor_publication.go create mode 100644 cmd/codeaf/once_publication_test.go create mode 100644 cmd/codeaf/publication_manual_test.go create mode 100644 cmd/codeaf/publication_round_test.go create mode 100644 internal/cellstore/first_turn_test.go create mode 100644 internal/cellsync/create_only_test.go create mode 100644 internal/syncsetup/publication.go create mode 100644 internal/syncsetup/publication_round_test.go diff --git a/cmd/codeaf/chatv3.go b/cmd/codeaf/chatv3.go index 073899addd..80ed00e8ee 100644 --- a/cmd/codeaf/chatv3.go +++ b/cmd/codeaf/chatv3.go @@ -2754,9 +2754,9 @@ func warmV3Models(ctx context.Context, models *catalog.Catalog, agent *session.A func runChatV3Once(ctx context.Context, cfg session.Config, workspace, text, level string, resumed bool) error { // The leaving road stands before session opening because opening can take // time, and a signal there would otherwise take the default disposition and - // skip every defer — the whole of #471. Cancelling the turn is all the - // leaving work needed here: the deferred agent.Close below settles the tasks - // and checkpoint. Defers run last-in-first-out, so Close runs before this + // skip every defer — the whole of #471. Cancelling the turn stops new work; + // deferred closes settle the tasks, checkpoint and paired publication. + // Defers run last-in-first-out, so Close runs before this // road stands down and keeps the second signal live through a close that can // take the turn's grace plus two job rounds. ctx, cancel := context.WithCancel(ctx) @@ -2776,6 +2776,17 @@ func runChatV3Once(ctx context.Context, cfg session.Config, workspace, text, lev cancel() }, nil) defer stopLeaving() + // Seating or pairing during the turn may start a drive. Register its close + // before opening so the agent's last seal precedes the final publish. + defer func() { + said := false + syncDrives.closeAllWith(func(_ *liveDrive, _ error) { + if !said { + fmt.Fprintln(os.Stderr, "this chat could not finish syncing with your other computers; run codeaf doctor --fix or reopen this chat to try again.") + said = true + } + }) + }() if resumed && cfg.SessionFile != "" { fmt.Fprintln(os.Stderr, "resumed "+cfg.SessionFile) diff --git a/cmd/codeaf/doctor_publication.go b/cmd/codeaf/doctor_publication.go new file mode 100644 index 0000000000..ba919f0016 --- /dev/null +++ b/cmd/codeaf/doctor_publication.go @@ -0,0 +1,145 @@ +package main + +import ( + "context" + "errors" + "fmt" + "maps" + "os" + "path/filepath" + "slices" + + "github.com/Agent-Field/codeaf/internal/cell" + "github.com/Agent-Field/codeaf/internal/cellstore" + "github.com/Agent-Field/codeaf/internal/directory" + "github.com/Agent-Field/codeaf/internal/filelock" + "github.com/Agent-Field/codeaf/internal/session" + "github.com/Agent-Field/codeaf/internal/syncsetup" +) + +var ( + errDoctorPublicationPresent = errors.New("this chat already reached your other computers") + errDoctorPublicationUnavailable = errors.New("this chat is no longer available to send") + errDoctorPublicationArchive = errors.New("could not put away the chat on your other computers") +) + +// doctorUnlistedChats compares only chats with this paired writer's durable +// intent against a successful listing. Absence of a receipt proves nothing. +func doctorUnlistedChats(s *syncsetup.Sync, inv localChatInventory, listing directory.Listing, fix bool) (issues, fixes []string) { + for _, id := range slices.Sorted(maps.Keys(inv.foldersByID)) { + if _, listed := listing.Cells[id]; listed || inv.inUseByID[id] || len(inv.foldersByID[id]) != 1 { + continue + } + dir := inv.foldersByID[id][0] + c, err := cell.OpenAt(dir, id) + if err != nil || !s.PublicationExpected(c) { + continue + } + meta, err := session.LoadMeta(dir) + if err != nil || !meta.Owned || meta.Archived { + continue + } + head, err := cellstore.Head(c) + if err != nil || head == nil { + continue + } + if spoken, _ := session.ConversationIn(session.Place{Dir: dir}.Transcript(), head.Receipt.Transcript.End); !spoken { + continue + } + if fix { + err := repairDoctorPublication(s, c, meta) + switch { + case err == nil: + fixes = append(fixes, fmt.Sprintf("fixed: sent chat %s to your other computers", id)) + continue + case errors.Is(err, errDoctorPublicationPresent), errors.Is(err, errDoctorPublicationUnavailable): + continue + case errors.Is(err, errDoctorPublicationArchive): + issues = append(issues, fmt.Sprintf("could not put away chat on your other computers · chat %s", id)) + continue + } + } + issues = append(issues, fmt.Sprintf("chat has not reached your other computers · chat %s", id)) + } + return issues, fixes +} + +func repairDoctorPublication(s *syncsetup.Sync, c cell.Cell, meta session.Meta) error { + // Hold the same transcript lock as an open conversation so repair cannot + // race a window starting a new turn on this computer. + f, err := os.OpenFile(session.Place{Dir: c.Root}.Transcript(), os.O_RDWR, 0) + if err != nil { + return err + } + defer f.Close() + if err := filelock.Lock(f, true, true); err != nil { + return err + } + defer filelock.Unlock(f) + current, err := session.LoadMeta(c.Root) + if err != nil { + return err + } + if current.Archived || !current.Owned { + return errDoctorPublicationUnavailable + } + meta = current + ctx, cancel := context.WithTimeout(context.Background(), driveCloseWithin) + defer cancel() + // A record created since the listing needs no missing-chat report. + // Repair never acquires an existing record or replaces unfamiliar history. + if _, err := s.Dir.Cell(ctx, c.ID); !errors.Is(err, directory.ErrNotFound) { + if err == nil { + return errDoctorPublicationPresent + } + return err + } + e := cellstore.EngineFor(meta.Workspace) + e.Workspace = filepath.Join(c.Root, "work") + d, err := s.Drive(ctx, e, c, syncsetup.DriveOptions{CreateOnly: true, DeviceName: deviceName(), Title: func() string { return meta.Title }}) + if errors.Is(err, directory.ErrExists) { + return errDoctorPublicationPresent + } + if err != nil { + return err + } + closeErr := errors.Join(d.Gate(), d.Close(ctx)) + alreadyPresent := errors.Is(closeErr, directory.ErrExists) + // Remote creation is too long for the metadata lock. Recheck after the + // writer stops, then put away only the record this repair just created. + current, err = session.LoadMeta(c.Root) + if err != nil { + return errors.Join(closeErr, err) + } + if current.Archived { + archiveCtx, archiveCancel := context.WithTimeout(context.Background(), driveCloseWithin) + defer archiveCancel() + if closeErr != nil { + // A lost create reply may return as ErrExists on retry. Only a record + // still held by this writer at this exact head can be that completion; + // an unrelated record retains its own history and archive state. + view, err := s.Dir.Cell(archiveCtx, c.ID) + if errors.Is(err, directory.ErrNotFound) { + return errDoctorPublicationUnavailable + } + if err != nil { + return errors.Join(errDoctorPublicationArchive, err) + } + head, err := cellstore.Head(c) + if err != nil || head == nil { + return errors.Join(errDoctorPublicationArchive, err) + } + if view.Cell.Lease.Device != s.Device.ID() || view.Cell.Head != head.Turn.ID { + return errDoctorPublicationUnavailable + } + } + if err := s.Dir.Archive(archiveCtx, c.ID); err != nil && !errors.Is(err, directory.ErrNotFound) { + return errors.Join(errDoctorPublicationArchive, err) + } + return errDoctorPublicationUnavailable + } + if alreadyPresent { + return errDoctorPublicationPresent + } + return closeErr +} diff --git a/cmd/codeaf/doctor_repair.go b/cmd/codeaf/doctor_repair.go index 447deb9019..5b39824945 100644 --- a/cmd/codeaf/doctor_repair.go +++ b/cmd/codeaf/doctor_repair.go @@ -303,9 +303,15 @@ func inspectDoctorSync(fix bool) (issues []string, fixes []string) { // Any local orphan .taking folder for a chat id absent from the directory listing. issues, fixes = finishOrphanStaging(inv.orphanStaging, handledStaging, fix, issues, fixes) + // Publication has its own per-chat bound after the older repair passes. + cancel() + missing, sent := doctorUnlistedChats(s, inv, l, fix) + issues = append(issues, missing...) + fixes = append(fixes, sent...) return issues, fixes } } + issues = append(issues, "could not check whether chats reached your other computers; check this computer's network and run codeaf doctor again") } // Sync is off or unreachable: still report local duplicate Home rows and orphaned .taking folders. diff --git a/cmd/codeaf/once_publication_test.go b/cmd/codeaf/once_publication_test.go new file mode 100644 index 0000000000..264bbbb2c8 --- /dev/null +++ b/cmd/codeaf/once_publication_test.go @@ -0,0 +1,393 @@ +package main + +import ( + "context" + "fmt" + "net" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "slices" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/cell" + "github.com/Agent-Field/codeaf/internal/cellstore" + "github.com/Agent-Field/codeaf/internal/cellsync" + "github.com/Agent-Field/codeaf/internal/directory" + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/identity" + "github.com/Agent-Field/codeaf/internal/session" + "github.com/Agent-Field/codeaf/internal/syncsetup" +) + +// publicationChat uses the repository's scripted model endpoint. Like the +// engine fixture, it is optional on machines without the staged test program. +func publicationChat(t *testing.T) (session.Config, cell.Cell) { + t.Helper() + stub := os.Getenv("CODEAF_TEST_SCRIPTED_MODEL_URL") + if stub == "" { + t.Skip("stage test/remote/stub and set CODEAF_TEST_SCRIPTED_MODEL_URL") + } + cfg, _ := doorChat(t) + c, err := cell.Create(home.Dir(), cell.Options{Class: cell.HostBound}) + if err != nil { + t.Fatal(err) + } + work := filepath.Join(c.Root, "work") + if err := os.MkdirAll(work, 0o700); err != nil { + t.Fatal(err) + } + cfg.Place = session.Place{Dir: c.Root, Workspace: work, Owned: true} + cfg.Workspace, cfg.SessionFile = work, cfg.Place.Transcript() + cfg.Model, cfg.APIKey, cfg.BaseURL = "stub/scripted", "stub", stub + return cfg, c +} + +func publicationRelay(t *testing.T, handler http.Handler) *httptest.Server { + t.Helper() + for port := 8900; port <= 8999; port++ { + listener, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) + if err != nil { + continue + } + srv := httptest.NewUnstartedServer(handler) + srv.Listener.Close() + srv.Listener = listener + srv.Start() + t.Cleanup(srv.Close) + return srv + } + t.Fatal("no local relay port available in 8900-8999") + return nil +} + +func publicationPair(t *testing.T, relay string) *syncsetup.Sync { + t.Helper() + id, err := identity.Ensure(home.Dir()) + if err != nil { + t.Fatal(err) + } + bHome := t.TempDir() + if err := identity.Save(bHome, id); err != nil { + t.Fatal(err) + } + t.Setenv(syncsetup.URLVar, relay) + t.Setenv(syncsetup.IntervalVar, "50") + b, ok, err := syncsetup.Open(bHome) + if err != nil || !ok { + t.Fatalf("open paired computer: %v, %v", ok, err) + } + return b +} + +func capturePublicationOutput(t *testing.T) (stdout, stderr *os.File) { + t.Helper() + oldOut, oldErr := os.Stdout, os.Stderr + var err error + stdout, err = os.CreateTemp(t.TempDir(), "stdout") + if err != nil { + t.Fatal(err) + } + stderr, err = os.CreateTemp(t.TempDir(), "stderr") + if err != nil { + t.Fatal(err) + } + os.Stdout, os.Stderr = stdout, stderr + t.Cleanup(func() { os.Stdout, os.Stderr = oldOut, oldErr; stdout.Close(); stderr.Close() }) + return +} + +// C1: the real one-message road waits through first registration and releases +// its hold before the other computer lists the chat, without cleanup helping it. +func TestChatOnceWaitsForFirstRegistration(t *testing.T) { + const wantReply = "STUB-REPLY: PROBE-ECHO publish\n" + cfg, c := publicationChat(t) + entered, release := make(chan struct{}), make(chan struct{}) + var enterOnce, releaseOnce sync.Once + unblock := func() { releaseOnce.Do(func() { close(release) }) } + next := relayserveForMove(t) + srv := publicationRelay(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && r.URL.Path == "/v1/dir/cells/"+c.ID { + enterOnce.Do(func() { close(entered) }) + select { + case <-release: + case <-r.Context().Done(): + return + } + } + next.ServeHTTP(w, r) + })) + b := publicationPair(t, srv.URL) + stdout, _ := capturePublicationOutput(t) + done := make(chan error, 1) + go func() { + done <- runChatV3Once(t.Context(), cfg, cfg.Workspace, "PROBE-ECHO publish", "", false) + close(done) + }() + t.Cleanup(func() { + // A failing test must cancel its writer rather than publish during + // cleanup and accidentally make the missing registration disappear. + if t.Failed() { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + for _, d := range syncDrives.take() { + _ = d.Close(ctx) + } + } + unblock() + select { + case <-done: + case <-time.After(driveCloseWithin): + t.Error("one-message test did not stop") + } + }) + select { + case <-entered: + case err := <-done: + t.Fatalf("returned before registration: %v", err) + case <-time.After(cellstore.SealWithin): + t.Fatal("no registration request") + } + // The answer itself must be visible before testing the closing wait. + // A slow model must not make a missing final publish look correct. + answeredBy := time.After(cellstore.SealWithin) + for { + out, err := os.ReadFile(stdout.Name()) + if err != nil { + t.Fatal(err) + } + if string(out) == wantReply { + break + } + select { + case err := <-done: + t.Fatalf("returned before registration finished: %v", err) + case <-answeredBy: + t.Fatal("the one-message answer did not arrive") + case <-time.After(10 * time.Millisecond): + } + } + select { + case err := <-done: + t.Fatalf("returned while registration was held: %v", err) + case <-time.After(200 * time.Millisecond): + } + unblock() + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-time.After(driveCloseWithin): + t.Fatal("one-message close did not finish") + } + listing, err := b.Dir.List(t.Context()) + if err != nil { + t.Fatal(err) + } + rec, exists := listing.Cells[c.ID] + if !exists || rec.Head == "" || rec.Lease.Expires > listing.Now { + t.Fatalf("other computer cannot take completed chat: %+v", listing) + } + out, err := os.ReadFile(stdout.Name()) + if err != nil || string(out) != wantReply { + t.Fatalf("reply = %q: %v", out, err) + } +} + +// C2/C5: a never-paired one-message chat and doctor retain local behavior, +// produce no paired publication records, and make no relay requests. +func TestChatOnceNeverPairedRemainsLocal(t *testing.T) { + cfg, _ := publicationChat(t) + var requests atomic.Int32 + srv := publicationRelay(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { requests.Add(1); w.WriteHeader(503) })) + t.Setenv(syncsetup.URLVar, srv.URL) + stdout, stderr := capturePublicationOutput(t) + if err := runChatV3Once(t.Context(), cfg, cfg.Workspace, "PROBE-ECHO local", "", false); err != nil { + t.Fatal(err) + } + out, _ := os.ReadFile(stdout.Name()) + if string(out) != "STUB-REPLY: PROBE-ECHO local\n" { + t.Fatalf("reply = %q", out) + } + errout, _ := os.ReadFile(stderr.Name()) + if strings.Contains(string(errout), "other computers") { + t.Fatalf("unexpected paired warning: %s", errout) + } + issues, fixes := inspectDoctorSync(true) + if len(issues) != 0 || len(fixes) != 0 { + t.Fatalf("doctor: %v, %v", issues, fixes) + } + if requests.Load() != 0 { + t.Fatalf("relay requests = %d", requests.Load()) + } + if _, err := os.Stat(filepath.Join(home.Dir(), "v3", "sync", "publication")); !os.IsNotExist(err) { + t.Fatalf("unpaired publication record: %v", err) + } +} + +// C1/C3/C4: an upload whose first registration failed is reported and repaired +// by doctor through the normal publisher; unavailable listings never prove absence. +func TestDoctorFixRegistersUnlistedChat(t *testing.T) { + cfg, c := publicationChat(t) + var dropCreate, dropList atomic.Bool + dropCreate.Store(true) + var uploads, failedListings atomic.Int32 + next := relayserveForMove(t) + srv := publicationRelay(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && r.URL.Path == "/v1/store/frames" { + uploads.Add(1) + } + if dropList.Load() && r.Method == http.MethodGet && r.URL.Path == "/v1/dir/list" { + failedListings.Add(1) + } + if (dropCreate.Load() && r.Method == http.MethodPost && r.URL.Path == "/v1/dir/cells/"+c.ID) || (dropList.Load() && r.Method == http.MethodGet && r.URL.Path == "/v1/dir/list") { + w.WriteHeader(http.StatusServiceUnavailable) + return + } + next.ServeHTTP(w, r) + })) + b := publicationPair(t, srv.URL) + _, stderr := capturePublicationOutput(t) + if err := runChatV3Once(t.Context(), cfg, cfg.Workspace, "PROBE-ECHO stranded", "", false); err != nil { + t.Fatalf("publish failure changed turn status: %v", err) + } + if uploads.Load() == 0 { + t.Fatal("fixture never uploaded") + } + errout, _ := os.ReadFile(stderr.Name()) + if !strings.Contains(string(errout), "this chat could not finish syncing with your other computers; run codeaf doctor --fix or reopen this chat to try again.") { + t.Fatalf("missing final publish warning: %s", errout) + } + missing := "chat has not reached your other computers · chat " + c.ID + // Another sealed local chat has no paired intent of its own. The first + // chat's upload ledger cannot make this pre-pairing history a repair target. + local, err := cell.Create(home.Dir(), cell.Options{Class: cell.HostBound}) + if err != nil { + t.Fatal(err) + } + localWork := filepath.Join(local.Root, "work") + if err := os.MkdirAll(localWork, 0o700); err != nil { + t.Fatal(err) + } + localCfg := cfg + localCfg.Place = session.Place{Dir: local.Root, Workspace: localWork, Owned: true} + localCfg.Workspace, localCfg.SessionFile = localWork, localCfg.Place.Transcript() + localAgent, err := session.New(localCfg) + if err != nil { + t.Fatal(err) + } + defer localAgent.Close() + events, err := localAgent.SubmitBash(t.Context(), "! printf local-only") + if err != nil { + localAgent.Close() + t.Fatal(err) + } + for event := range events { + if event.Kind == session.EventError { + t.Fatal(event.Text) + } + } + if err := localAgent.Close(); err != nil { + t.Fatal(err) + } + engine := cellstore.EngineFor(localWork) + engine.Workspace = localWork + if _, err := engine.Seal(t.Context(), local, cellstore.TurnInfo{}); err != nil { + t.Fatal(err) + } + if err := cellstore.Await(t.Context(), local); err != nil { + t.Fatal(err) + } + issues, fixes := inspectDoctorSync(false) + if !slices.Contains(issues, missing) || len(fixes) != 0 { + t.Fatalf("doctor: %v, %v", issues, fixes) + } + if strings.Contains(strings.Join(issues, "\n"), local.ID) { + t.Fatalf("unrelated upload made local chat a repair target: %v", issues) + } + dropList.Store(true) + issues, fixes = inspectDoctorSync(true) + if failedListings.Load() == 0 { + t.Fatal("fixture did not fail the listing") + } + if slices.Contains(issues, missing) || len(fixes) != 0 || !strings.Contains(strings.Join(issues, "\n"), "could not check whether chats reached your other computers") { + t.Fatalf("failed listing claimed absence: %v, %v", issues, fixes) + } + dropList.Store(false) + // Local archives and branch copies cannot become missing-chat repairs. + if err := session.SetArchived(c.Root, true); err != nil { + t.Fatal(err) + } + issues, fixes = inspectDoctorSync(true) + if slices.Contains(issues, missing) || len(fixes) != 0 { + t.Fatalf("archived chat was offered: %v, %v", issues, fixes) + } + if err := session.SetArchived(c.Root, false); err != nil { + t.Fatal(err) + } + branches, err := cellsync.OpenBranchMap(home.Dir()) + if err != nil { + t.Fatal(err) + } + if err := branches.Set(c.ID, "01J00000000000000000000999"); err != nil { + t.Fatal(err) + } + issues, fixes = inspectDoctorSync(true) + if slices.Contains(issues, missing) || len(fixes) != 0 { + t.Fatalf("branch copy was offered: %v, %v", issues, fixes) + } + if err := branches.Set(c.ID, c.ID); err != nil { + t.Fatal(err) + } + dropCreate.Store(false) + issues, fixes = inspectDoctorSync(true) + wantFix := "fixed: sent chat " + c.ID + " to your other computers" + if !slices.Contains(fixes, wantFix) || slices.Contains(issues, missing) { + t.Fatalf("repair: %v, %v", issues, fixes) + } + listing, err := b.Dir.List(t.Context()) + if err != nil { + t.Fatal(err) + } + if rec, ok := listing.Cells[c.ID]; !ok || rec.Head == "" || rec.Lease.Expires > listing.Now { + t.Fatalf("repair not visible or still held: %+v", listing) + } + issues, fixes = inspectDoctorSync(true) + if len(fixes) != 0 || slices.Contains(issues, missing) { + t.Fatalf("second repair changed state: %v, %v", issues, fixes) + } + // An existing hold and unfamiliar history are never acquired by this repair. + view, err := b.Dir.Acquire(t.Context(), c.ID, directory.AcquireOpts{}) + if err != nil { + t.Fatal(err) + } + view, err = b.Dir.Publish(t.Context(), c.ID, directory.Publish{Fence: view.Cell.Lease.Fence, OldHead: view.Cell.Head, Head: strings.Repeat("f", 64), Class: "chat"}) + if err != nil { + t.Fatal(err) + } + s, _, err := syncsetup.Open(home.Dir()) + if err != nil { + t.Fatal(err) + } + meta, _ := session.LoadMeta(c.Root) + if err := repairDoctorPublication(s, c, meta); err == nil { + t.Fatal("repair accepted another computer's record") + } + after, err := b.Dir.Cell(t.Context(), c.ID) + if err != nil || after.Cell.Lease != view.Cell.Lease || after.Cell.Head != view.Cell.Head { + t.Fatalf("repair changed live hold: %+v, %v", after, err) + } + if err := b.Dir.Archive(context.Background(), c.ID); err != nil { + t.Fatal(err) + } + issues, fixes = inspectDoctorSync(true) + if len(fixes) != 0 || slices.Contains(issues, missing) { + t.Fatalf("remote archive was offered: %v, %v", issues, fixes) + } +} diff --git a/cmd/codeaf/publication_manual_test.go b/cmd/codeaf/publication_manual_test.go new file mode 100644 index 0000000000..28e9aaec1d --- /dev/null +++ b/cmd/codeaf/publication_manual_test.go @@ -0,0 +1,61 @@ +package main + +import ( + "fmt" + "net/http" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/identity" + "github.com/Agent-Field/codeaf/internal/syncsetup" +) + +// C1: the manual quotes the closing limit from the same bound as both chat doors. +func TestOncePublicationManualMatchesCloseBudget(t *testing.T) { + raw, err := os.ReadFile(filepath.Join("..", "..", "internal", "manual", "chat", "running-from-the-terminal.md")) + if err != nil { + t.Fatal(err) + } + text := strings.Join(strings.Fields(string(raw)), " ") + if want := fmt.Sprintf("up to %.0f seconds for the final send and release", driveCloseWithin.Seconds()); !strings.Contains(text, want) { + t.Fatalf("manual must say %q", want) + } +} + +// C5: doctor has no sync output, writes or requests before pairing, even with --fix. +func TestDoctorNeverPairedMakesNoRelayRequests(t *testing.T) { + root := t.TempDir() + t.Setenv(home.EnvVar, root) + if _, err := identity.EnsureSolo(root); err != nil { + t.Fatal(err) + } + var requests atomic.Int32 + srv := publicationRelay(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { requests.Add(1); w.WriteHeader(503) })) + t.Setenv(syncsetup.URLVar, srv.URL) + before, err := os.ReadDir(root) + if err != nil { + t.Fatal(err) + } + for _, fix := range []bool{false, true} { + issues, fixes := inspectDoctorSync(fix) + if len(issues) != 0 || len(fixes) != 0 { + t.Fatalf("doctor: %v, %v", issues, fixes) + } + } + after, err := os.ReadDir(root) + if err != nil || len(after) != len(before) { + t.Fatalf("doctor changed home: %v, %v", after, err) + } + for i := range before { + if after[i].Name() != before[i].Name() { + t.Fatal("doctor changed home entries") + } + } + if requests.Load() != 0 { + t.Fatalf("relay requests = %d", requests.Load()) + } +} diff --git a/cmd/codeaf/publication_round_test.go b/cmd/codeaf/publication_round_test.go new file mode 100644 index 0000000000..9490138f88 --- /dev/null +++ b/cmd/codeaf/publication_round_test.go @@ -0,0 +1,402 @@ +package main + +import ( + "bytes" + "context" + "io" + "log" + "net/http" + "net/http/httptest" + "net/http/httputil" + "net/url" + "os" + "path/filepath" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/cell" + "github.com/Agent-Field/codeaf/internal/cellstore" + "github.com/Agent-Field/codeaf/internal/directory" + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/identity" + "github.com/Agent-Field/codeaf/internal/session" + "github.com/Agent-Field/codeaf/internal/syncsetup" +) + +func unlistedRepairChat(t *testing.T, wrap func(http.Handler) http.Handler) (*syncsetup.Sync, cell.Cell, localChatInventory) { + t.Helper() + cfg, _ := doorChat(t) + c, err := cell.Create(home.Dir(), cell.Options{Class: cell.HostBound}) + if err != nil { + t.Fatal(err) + } + work := filepath.Join(c.Root, "work") + if err := os.MkdirAll(work, 0700); err != nil { + t.Fatal(err) + } + cfg.Place = session.Place{Dir: c.Root, Workspace: work, Owned: true} + cfg.Workspace, cfg.SessionFile = work, cfg.Place.Transcript() + cfg.Model, cfg.APIKey, cfg.BaseURL = "stub/chat", "stub", "http://127.0.0.1:1" + var reject atomic.Bool + reject.Store(true) + handler := relayserveForMove(t) + if wrap != nil { + handler = wrap(handler) + } + srv := publicationRelay(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if reject.Load() && r.Method == http.MethodPost && r.URL.Path == "/v1/dir/cells/"+c.ID { + w.WriteHeader(503) + return + } + handler.ServeHTTP(w, r) + })) + publicationPair(t, srv.URL) + s, ok, err := syncsetup.Open(home.Dir()) + if err != nil || !ok { + t.Fatalf("open sync: %v %v", ok, err) + } + a, err := session.New(cfg) + if err != nil { + t.Fatal(err) + } + events, err := a.SubmitBash(t.Context(), "! printf repair-fixture") + if err != nil { + a.Close() + t.Fatal(err) + } + for event := range events { + if event.Kind == session.EventError { + a.Close() + t.Fatal(event.Text) + } + } + if err := a.Close(); err != nil { + t.Fatal(err) + } + e := cellstore.EngineFor(work) + e.Workspace = work + e.Identity = cellstore.Identity{Device: s.Device.Cert.Device} + if _, err := e.Seal(t.Context(), c, cellstore.TurnInfo{}); err != nil { + t.Fatal(err) + } + if err := cellstore.Await(t.Context(), c); err != nil { + t.Fatal(err) + } + d, err := s.Drive(t.Context(), e, c, syncsetup.DriveOptions{}) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _ = d.Close(ctx) + if !s.PublicationExpected(c) { + t.Fatal("fixture has no paired publication intent") + } + reject.Store(false) + return s, c, localChatInventory{foldersByID: map[string][]string{c.ID: {c.Root}}, inUseByID: map[string]bool{}} +} + +// SPEC-3 item 3: an archive during remote creation also puts away the new record. +func TestFixRound3DoctorArchivesChatPutAwayDuringRepair(t *testing.T) { + for _, lostReply := range []bool{false, true} { + t.Run(map[bool]string{false: "acknowledged", true: "lost-reply"}[lostReply], func(t *testing.T) { + entered, release := make(chan struct{}), make(chan struct{}) + var once sync.Once + var loseReply atomic.Bool + var repeats atomic.Int32 + loseReply.Store(lostReply) + s, c, inv := unlistedRepairChat(t, func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && strings.HasPrefix(r.URL.Path, "/v1/dir/cells/") && !strings.Contains(strings.TrimPrefix(r.URL.Path, "/v1/dir/cells/"), "/") { + once.Do(func() { close(entered) }) + select { + case <-release: + case <-r.Context().Done(): + return + } + if loseReply.Swap(false) { + // Discard the committed reply and replay the identical request before + // returning. This forces ErrExists without depending on which of the + // background publisher and Close made the first network attempt. + raw, err := io.ReadAll(r.Body) + if err != nil { + t.Error(err) + w.WriteHeader(503) + return + } + r.Body = io.NopCloser(bytes.NewReader(raw)) + saved := httptest.NewRecorder() + next.ServeHTTP(saved, r) + if saved.Code != http.StatusOK && saved.Code != http.StatusCreated { + t.Errorf("lost-reply fixture did not create: %d %s", saved.Code, saved.Body.String()) + } + r.Body = io.NopCloser(bytes.NewReader(raw)) + repeated := httptest.NewRecorder() + next.ServeHTTP(repeated, r) + if repeated.Code != http.StatusConflict { + t.Errorf("repeated create did not return exists: %d %s", repeated.Code, repeated.Body.String()) + } + repeats.Add(1) + for key, values := range repeated.Header() { + w.Header()[key] = values + } + w.WriteHeader(repeated.Code) + _, _ = w.Write(repeated.Body.Bytes()) + return + } + } + next.ServeHTTP(w, r) + }) + }) + var releaseOnce sync.Once + unblock := func() { releaseOnce.Do(func() { close(release) }) } + defer unblock() + type result struct{ issues, fixes []string } + done := make(chan result, 1) + go func() { + issues, fixes := doctorUnlistedChats(s, inv, directory.Listing{}, true) + done <- result{issues, fixes} + }() + select { + case <-entered: + case <-time.After(driveCloseWithin): + t.Fatal("repair never created record") + } + if err := session.SetArchived(c.Root, true); err != nil { + t.Fatal(err) + } + unblock() + select { + case got := <-done: + if len(got.issues) != 0 || len(got.fixes) != 0 { + t.Fatalf("archived repair: %+v", got) + } + case <-time.After(driveCloseWithin): + t.Fatal("repair did not finish") + } + if lostReply && repeats.Load() != 1 { + t.Fatal("lost-reply fixture never returned a repeated-create conflict") + } + view, err := s.Dir.Cell(t.Context(), c.ID) + if err != nil || !view.Cell.Archived { + t.Fatalf("remote chat revived: %+v %v", view, err) + } + }) + } +} + +// SPEC-3 item 8: a fresh lookup finding a record must not print the missing sentence. +func TestFixRound8DoctorAlreadyPresentIsSilent(t *testing.T) { + s, c, inv := unlistedRepairChat(t, nil) + created, err := s.Dir.Create(t.Context(), c.ID, directory.CellInit{Head: strings.Repeat("f", 64), Class: "chat"}) + if err != nil { + t.Fatal(err) + } + before := created.Cell + issues, fixes := doctorUnlistedChats(s, inv, directory.Listing{}, true) + if len(issues) != 0 || len(fixes) != 0 { + t.Fatalf("already present was reported missing: %v %v", issues, fixes) + } + after, err := s.Dir.Cell(t.Context(), c.ID) + if err != nil || after.Cell.Head != before.Head || after.Cell.Lease != before.Lease { + t.Fatalf("existing record changed: %+v %v", after, err) + } +} + +// SPEC-3 item 7: an empty cleanup has no timer allocations, output or relay work. +func TestFixRound7OnceEmptyCleanupReturnsImmediately(t *testing.T) { + t.Setenv(home.EnvVar, t.TempDir()) + var requests atomic.Int32 + srv := publicationRelay(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { requests.Add(1); w.WriteHeader(503) })) + t.Setenv(syncsetup.URLVar, srv.URL) + out, errout := capturePublicationOutput(t) + b := &driveBook{} + allocs := testing.AllocsPerRun(100, func() { b.closeAllWith(func(*liveDrive, error) { t.Fatal("empty cleanup reported a failure") }) }) + if allocs != 0 { + t.Fatalf("empty cleanup created closing work: %g allocations", allocs) + } + stdout, _ := os.ReadFile(out.Name()) + stderr, _ := os.ReadFile(errout.Name()) + if len(stdout) != 0 || len(stderr) != 0 || requests.Load() != 0 { + t.Fatalf("empty cleanup: stdout=%q stderr=%q requests=%d", stdout, stderr, requests.Load()) + } +} + +// SPEC-3 item 7: a drive starting after entry is drained before --once returns. +func TestFixRound7OnceClosesDriveStartedAfterEntry(t *testing.T) { + cfg, c := publicationChat(t) + next := relayserveForMove(t) + relay := publicationRelay(t, next) + t.Setenv(syncsetup.URLVar, relay.URL) + target, err := url.Parse(cfg.BaseURL) + if err != nil { + t.Fatal(err) + } + target.Path = "" + target.RawPath = "" + proxy := httputil.NewSingleHostReverseProxy(target) + entered, release := make(chan struct{}), make(chan struct{}) + var once, releaseOnce sync.Once + unblock := func() { releaseOnce.Do(func() { close(release) }) } + defer unblock() + model := publicationRelay(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/chat/completions") { + once.Do(func() { close(entered) }) + select { + case <-release: + case <-r.Context().Done(): + return + } + } + proxy.ServeHTTP(w, r) + })) + cfg.BaseURL = model.URL + "/api/v1" + capturePublicationOutput(t) + if syncsetup.MayTalk(home.Dir()) { + t.Fatal("fixture paired at entry") + } + done := make(chan error, 1) + go func() { done <- runChatV3Once(t.Context(), cfg, cfg.Workspace, "PROBE-ECHO late-pair", "", false) }() + select { + case <-entered: + case err := <-done: + t.Fatalf("returned before model: %v", err) + case <-time.After(driveCloseWithin): + t.Fatal("no model call") + } + b := publicationPair(t, relay.URL) + if err := identity.EndSolo(home.Dir()); err != nil { + t.Fatal(err) + } + unblock() + select { + case err := <-done: + if err != nil { + t.Fatal(err) + } + case <-time.After(driveCloseWithin): + t.Fatal("late drive did not close") + } + if len(syncDrives.drive) != 0 { + t.Fatal("late drive left for unrelated cleanup") + } + view, err := b.Dir.Cell(t.Context(), c.ID) + if err != nil || view.Cell.Head == "" || view.Cell.Lease.Expires > view.Now { + t.Fatalf("late drive not released: %+v %v", view, err) + } +} + +// SPEC-3 item 6: older repairs finish before a publication send can consume time. +func TestFixRound6DoctorRepairsOlderHoldsBeforePublication(t *testing.T) { + var oldReleased, createdBeforeOld atomic.Bool + var target string + s, c, _ := unlistedRepairChat(t, func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodPost && r.URL.Path == "/v1/dir/cells/"+target && !oldReleased.Load() { + createdBeforeOld.Store(true) + } + next.ServeHTTP(w, r) + if r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/release") { + oldReleased.Store(true) + } + }) + }) + target = c.ID + oldID, err := cell.NewID(time.Now()) + if err != nil { + t.Fatal(err) + } + if _, err := s.Dir.Create(t.Context(), oldID, directory.CellInit{Head: strings.Repeat("e", 64), Class: "chat"}); err != nil { + t.Fatal(err) + } + _, fixes := inspectDoctorSync(true) + if !oldReleased.Load() { + t.Fatalf("older repair did not release its hold: %v", fixes) + } + if createdBeforeOld.Load() { + t.Fatal("publication repair ran before older repairs") + } + if !strings.Contains(strings.Join(fixes, "\n"), "fixed: sent chat "+c.ID) { + t.Fatalf("publication repair did not run: %v", fixes) + } +} + +// SPEC-3 item 9: the real one-message door keeps marker failure off stdout and stderr. +func TestFixRound9OnceMarkerFailureIsSilent(t *testing.T) { + cfg, c := publicationChat(t) + next := relayserveForMove(t) + relay := publicationRelay(t, next) + b := publicationPair(t, relay.URL) + parent := home.Join("v3", "sync", "publication") + if err := os.MkdirAll(filepath.Dir(parent), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(parent, []byte("blocked"), 0600); err != nil { + t.Fatal(err) + } + out, errout := capturePublicationOutput(t) + oldLog := log.Writer() + log.SetOutput(errout) + defer log.SetOutput(oldLog) + if err := runChatV3Once(t.Context(), cfg, cfg.Workspace, "PROBE-ECHO marker-failure", "", false); err != nil { + t.Fatal(err) + } + stdout, _ := os.ReadFile(out.Name()) + stderr, _ := os.ReadFile(errout.Name()) + if string(stdout) != "STUB-REPLY: PROBE-ECHO marker-failure\n" { + t.Fatalf("reply=%q", stdout) + } + if len(stderr) != 0 { + t.Fatalf("marker failure reached standard error: %s", stderr) + } + view, err := b.Dir.Cell(t.Context(), c.ID) + if err != nil || view.Cell.Head == "" || view.Cell.Lease.Expires > view.Now { + t.Fatalf("marker failure stopped publication: %+v %v", view, err) + } +} + +// publicationCreateRace installs unfamiliar history after the final lookup. +type publicationCreateRace struct { + directory.Client + dir string + archive bool + once sync.Once + injectErr error +} + +func (r *publicationCreateRace) Create(ctx context.Context, id string, in directory.CellInit) (directory.CellView, error) { + r.once.Do(func() { + if r.archive { + r.injectErr = session.SetArchived(r.dir, true) + if r.injectErr != nil { + return + } + } + _, r.injectErr = r.Client.Create(ctx, id, directory.CellInit{Head: strings.Repeat("f", 64), Class: "chat"}) + }) + if r.injectErr != nil { + return directory.CellView{}, r.injectErr + } + return r.Client.Create(ctx, id, in) +} + +// SPEC-3 items 3/8: a create race is silent and an archive cannot touch unfamiliar history. +func TestFixRound8DoctorAlreadyPresentAtCreateIsSilent(t *testing.T) { + for _, archive := range []bool{false, true} { + t.Run(map[bool]string{false: "present", true: "archived-during-create"}[archive], func(t *testing.T) { + s, c, inv := unlistedRepairChat(t, nil) + s.Dir = &publicationCreateRace{Client: s.Dir, dir: c.Root, archive: archive} + issues, fixes := doctorUnlistedChats(s, inv, directory.Listing{}, true) + if len(issues) != 0 || len(fixes) != 0 { + t.Fatalf("already present: %v %v", issues, fixes) + } + view, err := s.Dir.Cell(t.Context(), c.ID) + if err != nil || view.Cell.Head != strings.Repeat("f", 64) || view.Cell.Archived || view.Cell.Lease.Device != s.Device.ID() { + t.Fatalf("unfamiliar record changed: %+v %v", view, err) + } + }) + } +} diff --git a/cmd/codeaf/syncdrive.go b/cmd/codeaf/syncdrive.go index 9bf6fb9a46..681bc2923b 100644 --- a/cmd/codeaf/syncdrive.go +++ b/cmd/codeaf/syncdrive.go @@ -19,7 +19,7 @@ import ( const driveCloseWithin = cellstore.SealWithin // syncDrives is the drive side of every conversation the chat door opened in -// this process. It exists at all only for a chat: a headless run never syncs. +// this process. Other command doors, such as do and exec, never sync chats. var syncDrives = &driveBook{} // driveBook keeps one drive side per cell, so a conversation opened again in the @@ -148,13 +148,24 @@ func chatTitle(c cell.Cell) string { // given back, so another machine can take the chat at once. It runs on the // door's way out, after the conversations closed, so their last seal is in. func (b *driveBook) closeAll() { + b.closeAllWith(func(d *liveDrive, err error) { + log.Print("sync: closing a chat: ", err) + recordFinalSaveFailure(session.SavingError(d.transcript, err)) + }) +} + +// closeAllWith lets the one-message door report a failed publish without +// changing the turn's exit status. Both doors share the same closing budget. +func (b *driveBook) closeAllWith(report func(*liveDrive, error)) { drives := b.take() + if len(drives) == 0 { + return + } ctx, cancel := context.WithTimeout(context.Background(), driveCloseWithin) defer cancel() for _, d := range drives { if err := d.Close(ctx); err != nil { - log.Print("sync: closing a chat: ", err) - recordFinalSaveFailure(session.SavingError(d.transcript, err)) + report(d, err) } } } diff --git a/internal/cellstore/first_turn_test.go b/internal/cellstore/first_turn_test.go new file mode 100644 index 0000000000..bbbaface1a --- /dev/null +++ b/internal/cellstore/first_turn_test.go @@ -0,0 +1,37 @@ +package cellstore + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Agent-Field/codeaf/internal/cell" +) + +// SPEC-3 item 5: reading just the first turn preserves valid large first records. +func TestFixRound5FirstTurnHasNoNewSizeLimit(t *testing.T) { + c, err := cell.Create(t.TempDir(), cell.Options{Class: cell.HostBound}) + if err != nil { + t.Fatal(err) + } + first := Turn{ID: "first", Device: "original", ExcludedPaths: []string{strings.Repeat("long-path", 1<<14)}} + raw, err := json.Marshal(first) + if err != nil { + t.Fatal(err) + } + path := filepath.Join(c.Root, filepath.FromSlash(TurnsPath)) + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + for _, tail := range []string{"", "\nnot another turn"} { + if err := os.WriteFile(path, append(append([]byte{}, raw...), tail...), 0600); err != nil { + t.Fatal(err) + } + got, err := FirstTurn(c) + if err != nil || got == nil || got.ID != first.ID || got.Device != first.Device || len(got.ExcludedPaths) != 1 || got.ExcludedPaths[0] != first.ExcludedPaths[0] { + t.Fatalf("large first turn was not read whole: %v", err) + } + } +} diff --git a/internal/cellstore/log.go b/internal/cellstore/log.go index ee9050092e..db07eabee7 100644 --- a/internal/cellstore/log.go +++ b/internal/cellstore/log.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "io/fs" "os" "slices" @@ -48,6 +49,31 @@ func Turns(c cell.Cell) ([]Turn, error) { return turns, sc.Err() } +// FirstTurn reads only the oldest seal when a caller needs its original writer. +// Later history cannot change that writer or add work to this decision. +func FirstTurn(c cell.Cell) (*Turn, error) { + f, err := os.Open(rel(c, TurnsPath)) + if errors.Is(err, fs.ErrNotExist) { + return nil, nil + } + if err != nil { + return nil, err + } + defer f.Close() + raw, err := bufio.NewReader(f).ReadBytes('\n') + if err != nil && !errors.Is(err, io.EOF) { + return nil, err + } + if len(raw) == 0 { + return nil, nil + } + var turn Turn + if err := json.Unmarshal(raw, &turn); err != nil { + return nil, fmt.Errorf("read first turn: %w", err) + } + return &turn, nil +} + // Log is the chain with receipts, newest first. func Log(c cell.Cell) ([]Entry, error) { turns, err := Turns(c) diff --git a/internal/cellsync/create_only_test.go b/internal/cellsync/create_only_test.go new file mode 100644 index 0000000000..64427553a7 --- /dev/null +++ b/internal/cellsync/create_only_test.go @@ -0,0 +1,35 @@ +package cellsync + +import ( + "errors" + "testing" + + "github.com/Agent-Field/codeaf/internal/directory" +) + +// C4: doctor repair cannot acquire a record that appeared after its listing, +// even when the other computer has already released an unfamiliar head. +func TestCreateOnlyPublishLeavesExistingHistoryAlone(t *testing.T) { + r := newRig(t) + head := r.seal(map[string]string{"local": "only here"}) + other := r.dir.For(devB) + v, err := other.Create(t.Context(), cellID, directory.CellInit{Head: otherHead, Class: "chat"}) + if err != nil { + t.Fatal(err) + } + if err := other.Release(t.Context(), cellID, v.Cell.Lease.Fence); err != nil { + t.Fatal(err) + } + before, err := other.Cell(t.Context(), cellID) + if err != nil { + t.Fatal(err) + } + r.pub.CreateOnly = true + if err := r.pub.Publish(t.Context(), r.drv, head, r.info()); !errors.Is(err, directory.ErrExists) { + t.Fatalf("publish = %v", err) + } + after, err := other.Cell(t.Context(), cellID) + if err != nil || after.Cell.Head != before.Cell.Head || after.Cell.Lease != before.Cell.Lease { + t.Fatalf("existing history changed: %+v, %v", after, err) + } +} diff --git a/internal/cellsync/publish.go b/internal/cellsync/publish.go index a33587db66..8662eddef2 100644 --- a/internal/cellsync/publish.go +++ b/internal/cellsync/publish.go @@ -18,6 +18,9 @@ var ErrSuperseded = fmt.Errorf("cellsync: another device continued this chat: %w // Publisher makes turns durable: objects into the store first, the head in the // directory last. Dir is already bound to this device. type Publisher struct { + // CreateOnly refuses a record that appeared during a missing-chat repair. + // Ordinary chat publication still resumes a create whose reply was lost. + CreateOnly bool // WarmFrameHints opts into the bounded existing-frame prototype. WarmFrameHints bool Engine Engine @@ -149,7 +152,7 @@ func (p *Publisher) advance(ctx context.Context, d Driving, head string, in Publ return p.move(ctx, d, head, in, frames) } fence, err := p.create(ctx, d, head, in, frames) - if errors.Is(err, directory.ErrExists) { + if errors.Is(err, directory.ErrExists) && !p.CreateOnly { return p.resume(ctx, d, head, in, frames, err) } return fence, err diff --git a/internal/syncsetup/drive.go b/internal/syncsetup/drive.go index 71cc7013a6..c76dc539aa 100644 --- a/internal/syncsetup/drive.go +++ b/internal/syncsetup/drive.go @@ -24,6 +24,9 @@ const unknownDevice = "another device" // DriveOptions is what a chat tells its drive side about itself. type DriveOptions struct { + // CreateOnly is used by repair after a missing directory record. It never + // acquires a record that appeared since the check or replaces its history. + CreateOnly bool // DeviceName is this machine's name, sealed into its directory record. DeviceName string // Title is the chat's title as it is now; nil publishes none. @@ -51,12 +54,16 @@ type Drive struct { directoryID string recordedFence uint64 - cancel context.CancelFunc - done chan struct{} - once sync.Once - closeErr error - publicationDone func() - ownership chan struct{} + cancel context.CancelFunc + done chan struct{} + once sync.Once + closeErr error + publicationDone func() + publicationMu sync.Mutex + publicationChecked bool + publicationEligible bool + publicationSaved bool + ownership chan struct{} sent string // fingerprint of the vault as last sent; touched by the flush loop and Close only @@ -84,7 +91,7 @@ func (s *Sync) Drive(ctx context.Context, eng cellstore.Engine, c cell.Cell, opt return nil, err } d.syncVault(ctx) - drv, held, err := s.driving(ctx, c) + drv, held, err := s.driving(ctx, c, opt.CreateOnly) if err != nil { return nil, err } @@ -160,6 +167,9 @@ func (d *Drive) note(sealed cellstore.Sealed) { spoken := d.spoken d.mu.Unlock() if spoken { + if err := d.expectPublication(); err != nil { + d.sync.logPublicationError(err) + } d.batcher.Note(sealed.Turn) } } @@ -238,13 +248,16 @@ func (s *Sync) Rename(ctx context.Context, name string) error { return s.putDevi // driving finds where this chat stands in the directory: no record yet (fence // 0), a record this device now holds, or a record another device holds, in // which case held names that device. A chat that was branched drives its branch. -func (s *Sync) driving(ctx context.Context, c cell.Cell) (cellsync.Driving, string, error) { +func (s *Sync) driving(ctx context.Context, c cell.Cell, createOnly bool) (cellsync.Driving, string, error) { branches, err := cellsync.OpenBranchMap(s.Home) if err != nil { return cellsync.Driving{}, "", err } drv := cellsync.Driving{Cell: c} if id := branches.Resolve(c.ID); id != c.ID { + if createOnly { + return drv, "", directory.ErrExists + } drv.Remote = id } view, err := s.Dir.Cell(ctx, drv.ID()) @@ -254,6 +267,9 @@ func (s *Sync) driving(ctx context.Context, c cell.Cell) (cellsync.Driving, stri if err != nil { return drv, "", err } + if createOnly { + return drv, "", directory.ErrExists + } if err := s.reconcileSendLease(c.ID, drv.ID(), view); err != nil { return drv, "", err } @@ -320,7 +336,7 @@ func (s *Sync) batcher(eng cellstore.Engine, c cell.Cell, drv cellsync.Driving, engine := eng.Sync(cellstore.SyncKeys{CellKey: s.Identity.CellKey(), Dedup: s.Identity.DedupSecret()}, s.Ledger) engine.MaxFrame = blobstore.BulkFrame // a flush's objects share frames up to the bulk size (contract §22.6); a small save still writes one small frame d.scope = s.scope(c.ID) - pub := &cellsync.Publisher{Engine: engine, Store: d.scope.Store, Dir: s.Dir} + pub := &cellsync.Publisher{Engine: engine, Store: d.scope.Store, Dir: s.Dir, CreateOnly: d.opt.CreateOnly} return &cellsync.Batcher{ Publisher: pub, Brancher: cellsync.Brancher{ diff --git a/internal/syncsetup/publication.go b/internal/syncsetup/publication.go new file mode 100644 index 0000000000..cc3e6d02fc --- /dev/null +++ b/internal/syncsetup/publication.go @@ -0,0 +1,105 @@ +package syncsetup + +import ( + "log" + "os" + "path/filepath" + + "github.com/Agent-Field/codeaf/internal/cell" + "github.com/Agent-Field/codeaf/internal/cellstore" + "github.com/Agent-Field/codeaf/internal/cellsync" + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/fsreplace" + "github.com/Agent-Field/codeaf/internal/home" +) + +func (s *Sync) publicationPath(c cell.Cell) string { + return filepath.Join(s.Home, "v3", "sync", "publication", c.ID) +} + +func (s *Sync) publicationScope() string { return s.Ledger + "\n" + s.Device.ID() + "\n" } + +// PublicationExpected identifies chats this computer began publishing while +// paired. Upload counters are shared by chats, so they cannot establish this. +// A receipt from another identity, relay or device never grants repair permission. +func (s *Sync) PublicationExpected(c cell.Cell) bool { + if !s.publicationRecorded(c) { + return false + } + branches, err := cellsync.OpenBranchMap(s.Home) + return err == nil && branches.Resolve(c.ID) == c.ID +} + +func (s *Sync) publicationRecorded(c cell.Cell) bool { + if !cell.ValidID(c.ID) { + return false + } + raw, err := os.ReadFile(s.publicationPath(c)) + return err == nil && string(raw) == s.publicationScope() +} + +// expectPublication decides eligibility once per drive. An ineligible chat never +// reads its growing history again; a failed marker write can still be retried. +func (d *Drive) expectPublication() error { + d.publicationMu.Lock() + defer d.publicationMu.Unlock() + if !d.publicationChecked { + if d.sync.publicationRecorded(d.cell) { + d.publicationChecked, d.publicationSaved = true, true + } else { + first, err := cellstore.FirstTurn(d.cell) + if err != nil { + return err + } + d.publicationEligible = first != nil && first.Device == d.sync.Device.Cert.Device + d.publicationChecked = true + } + } + if d.publicationSaved || !d.publicationEligible { + return nil + } + if err := d.sync.recordPublication(d.cell); err != nil { + return err + } + d.publicationSaved = true + return nil +} + +// recordPublication keeps the paired writer's intent outside sealed chat content. +func (s *Sync) recordPublication(c cell.Cell) error { + path := s.publicationPath(c) + if err := home.MkdirAllUnder(s.Home, filepath.Dir(path), 0o700); err != nil { + return err + } + f, err := os.CreateTemp(filepath.Dir(path), ".publication-*") + if err != nil { + return err + } + defer os.Remove(f.Name()) + _, err = f.WriteString(s.publicationScope()) + if err == nil { + err = f.Sync() + } + closeErr := f.Close() + if err != nil { + return err + } + if closeErr != nil { + return closeErr + } + return fsreplace.Rename(f.Name(), path) +} + +// logPublicationError uses the chat's file log even at the one-message door, +// where the standard logger still points at the person's terminal. +func (s *Sync) logPublicationError(err error) { + if home.Removed(s.Home) { + return + } + f, openErr := os.OpenFile(config.ProfilePath(s.profile(), "chat.log"), os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644) + if openErr != nil { + return + } + defer f.Close() + log.New(f, log.Prefix(), log.Flags()).Print("sync: recording chat publication: ", err) +} diff --git a/internal/syncsetup/publication_round_test.go b/internal/syncsetup/publication_round_test.go new file mode 100644 index 0000000000..daf61427c7 --- /dev/null +++ b/internal/syncsetup/publication_round_test.go @@ -0,0 +1,151 @@ +package syncsetup + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/Agent-Field/codeaf/internal/cell" + "github.com/Agent-Field/codeaf/internal/cellstore" + "github.com/Agent-Field/codeaf/internal/cellsync" + "github.com/Agent-Field/codeaf/internal/identity" +) + +// SPEC-3 item 5: eligibility needs only the first turn, then no per-seal reads. +func TestFixRound5PublicationEligibilityReadsFirstTurnOnce(t *testing.T) { + h := t.TempDir() + id, err := identity.Ensure(h) + if err != nil { + t.Fatal(err) + } + dev, err := identity.Device(h) + if err != nil { + t.Fatal(err) + } + s := &Sync{Home: h, Identity: id, Device: dev, Ledger: "local"} + for _, eligible := range []bool{false, true} { + t.Run(map[bool]string{false: "ineligible", true: "eligible"}[eligible], func(t *testing.T) { + c, err := cell.Create(h, cell.Options{Class: cell.HostBound}) + if err != nil { + t.Fatal(err) + } + device := "before pairing" + if eligible { + device = dev.Cert.Device + } + first, _ := json.Marshal(cellstore.Turn{Device: device}) + path := filepath.Join(c.Root, filepath.FromSlash(cellstore.TurnsPath)) + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + // Malformed trailing bytes must never be read to decide who made the first seal. + if err := os.WriteFile(path, append(append(first, '\n'), []byte(strings.Repeat("unneeded", 1<<17))...), 0600); err != nil { + t.Fatal(err) + } + d := &Drive{sync: s, cell: c} + if err := d.expectPublication(); err != nil { + t.Fatal(err) + } + if got := s.publicationRecorded(c); got != eligible { + t.Fatalf("publication recorded=%v, want %v", got, eligible) + } + if err := os.Remove(path); err != nil { + t.Fatal(err) + } + if err := d.expectPublication(); err != nil { + t.Fatalf("decision reread chain: %v", err) + } + allocs := testing.AllocsPerRun(100, func() { + if err := d.expectPublication(); err != nil { + panic(err) + } + }) + if allocs != 0 { + t.Fatalf("cached decision allocated %g times", allocs) + } + t.Logf("cached decision: %g allocations per seal; fixture tail=%d bytes", allocs, 8<<17) + }) + } +} + +// SPEC-3 item 5 and PERF.md: repeated decisions do no allocation work as history grows. +func TestFixRound5IneligiblePublicationSealWork(t *testing.T) { + h := t.TempDir() + s := &Sync{Home: h, Ledger: "local"} + c, err := cell.Create(h, cell.Options{Class: cell.HostBound}) + if err != nil { + t.Fatal(err) + } + first, _ := json.Marshal(cellstore.Turn{Device: "before pairing"}) + path := filepath.Join(c.Root, filepath.FromSlash(cellstore.TurnsPath)) + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, bytes.Repeat(append(first, '\n'), 4096), 0600); err != nil { + t.Fatal(err) + } + d := &Drive{sync: s, cell: c} + if err := d.expectPublication(); err != nil { + t.Fatal(err) + } + allocs := testing.AllocsPerRun(100, func() { + if err := d.expectPublication(); err != nil { + panic(err) + } + }) + t.Logf("%s %s/%s: 4096 turns, cached decision allocations=%g", runtime.Version(), runtime.GOOS, runtime.GOARCH, allocs) + if allocs != 0 { + t.Fatalf("history added per-seal allocation work: %g", allocs) + } +} + +// SPEC-3 item 9: a failed recovery record is logged, silent in chat, and still queues the turn. +func TestFixRound9PublicationMarkerFailureIsSilent(t *testing.T) { + h := t.TempDir() + id, err := identity.Ensure(h) + if err != nil { + t.Fatal(err) + } + dev, err := identity.Device(h) + if err != nil { + t.Fatal(err) + } + s := &Sync{Home: h, Identity: id, Device: dev, Ledger: "local"} + c, err := cell.Create(h, cell.Options{Class: cell.HostBound}) + if err != nil { + t.Fatal(err) + } + path := filepath.Join(c.Root, filepath.FromSlash(cellstore.TurnsPath)) + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + first, _ := json.Marshal(cellstore.Turn{ID: "first", Device: dev.Cert.Device}) + if err := os.WriteFile(path, append(first, '\n'), 0600); err != nil { + t.Fatal(err) + } + parent := filepath.Dir(s.publicationPath(c)) + if err := os.MkdirAll(filepath.Dir(parent), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(parent, []byte("blocked"), 0600); err != nil { + t.Fatal(err) + } + var notices []string + batch := &cellsync.Batcher{Driving: &cellsync.Driving{Cell: c}} + d := &Drive{sync: s, cell: c, spoken: true, batcher: batch, opt: DriveOptions{OnNotice: func(line string) { notices = append(notices, line) }}} + d.note(cellstore.Sealed{Turn: cellstore.Turn{ID: "first"}}) + if len(notices) != 0 { + t.Fatalf("marker failure reached chat: %v", notices) + } + logs, err := os.ReadFile(filepath.Join(h, "chat.log")) + if err != nil || !strings.Contains(string(logs), "sync: recording chat publication:") { + t.Fatalf("marker failure not in chat log: %s %v", logs, err) + } + if batch.Pending() != 1 { + t.Fatal("marker failure stopped ordinary publication") + } +} From f28b0f4af211c6b1494a46136fc4f8b33107076b Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 30 Oct 2026 22:39:22 -0400 Subject: [PATCH 08/18] manual: --once waits for your other computers, doctor finds a chat that did not arrive, a deleted home is refused Co-Authored-By: Claude Opus 5.5 --- internal/manual/chat/keys.md | 3 +- .../manual/chat/running-from-the-terminal.md | 54 ++++++++++++++++++- internal/manual/chat/starting-codeaf.md | 2 +- internal/manual/chat/tasks.md | 2 +- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/internal/manual/chat/keys.md b/internal/manual/chat/keys.md index cabfabcec4..a0cfa1f375 100644 --- a/internal/manual/chat/keys.md +++ b/internal/manual/chat/keys.md @@ -690,7 +690,8 @@ in it`. Background jobs go the same way, a moment later. Either way, what a task wrote is on its branch and stays there; a stop is an interruption and never a finding about the work. A headless `codeaf chat --once` leaves by the closing -road: the turn stops, the session closes, and the process exits cleanly. +road: the turn stops, the session closes, and the process exits cleanly. A paired chat +also gives its final send and release the usual closing time limit before exiting. ## Keys — what all the keys do, the keyboard keys, keys on the keyboard, key bindings and keyboard shortcuts diff --git a/internal/manual/chat/running-from-the-terminal.md b/internal/manual/chat/running-from-the-terminal.md index 20122d9a56..1d27caffca 100644 --- a/internal/manual/chat/running-from-the-terminal.md +++ b/internal/manual/chat/running-from-the-terminal.md @@ -1624,11 +1624,61 @@ machine that counted, which is the opposite of the truth. That command is about this *install*. `/status` in the chat is about the conversation you are in. They are related and they are not the same reading. +## Does chat --once wait for my other computers before it exits? + +When you are paired, `codeaf chat --once ""` waits for the chat's last +saved turn to reach your other computers and for this computer to let go of the +chat before it exits. It uses the interactive chat's closing limit: up to 30 +seconds for the final send and release. If the final send or release fails or reaches the limit, +standard error says `this chat could not finish syncing with your other computers; run codeaf doctor --fix or reopen this chat to try again.` +The reply on standard output and the turn's exit status stay the same. On a +computer that has never paired, the command has no added sync wait or requests. + +## My chat did not show up on my other computer — codeaf doctor and --fix + +On a paired computer, `codeaf doctor` can report +`chat has not reached your other computers · chat `. It checks owned chats +started and first saved here while paired, with conversation work saved and a +local record that they were meant to be sent. Older chats without that record, +pre-pairing chats, branch copies, archived chats and chats currently open here +are left out of this check. Their absence from the report does not prove they +have reached another computer. + +If the directory cannot be read, doctor says +`could not check whether chats reached your other computers; check this computer's network and run codeaf doctor again`. +It never reports a chat as missing based on a failed check. + +`codeaf doctor --fix` retries the ordinary send and releases the chat, printing +`fixed: sent chat to your other computers` for each successful repair. +Running it again leaves repaired chats alone. A chat already present at the fresh +check produces no missing-chat line. These sends run after the other repair +passes, each with its own send limit. Doctor never takes another computer's live +hold or overwrites unfamiliar history. + +Chats put away before repair starts stay excluded. If you put a chat away during +its send, doctor also puts away the newly created record on your other computers. +If that request fails, it says `could not put away chat on your other computers · chat `; +put the chat away on the other computer when the connection returns. +A computer that has never paired gets no new check or requests. + +Saving the local recovery record is best effort and silent. The chat still +attempts its ordinary send if recording fails. + +## I deleted my codeaf home while the engine was stopping + +A running process remembers its original home directory. If you remove it or +replace it with a different directory, attempts to make an identity or device +refuse with `codeaf home was removed or replaced; restart codeaf to use the current folder`. +Late saving work does not make a new identity there. If the engine is already +stopping and still closing chats, it exits promptly when its original home is +removed. With the home intact, stopping keeps its usual saving order and limits. +A new process can still create a home on first use. + ## Repair a busy engine store or sync state — codeaf doctor --fix -When an engine store lock or cross-computer sync issue exists on this computer, `codeaf doctor` adds `engine store` and `sync` rows naming a busy store holder (`busy · held by , pid `), an `unfinished continue · chat `, `duplicate rows · chat `, or a `lapsed hold on this computer · chat `. Healthy stores and clean sync state print nothing. +When an engine store lock or cross-computer sync issue exists on this computer, `codeaf doctor` adds `engine store` and `sync` rows naming a busy store holder (`busy · held by , pid `), an `unfinished continue · chat `, `duplicate rows · chat `, a `lapsed hold on this computer · chat `, or a chat that has not reached your other computers. When a busy holder cannot be found in the brief lookup, the store row says only `busy`. Healthy stores and clean sync state print nothing. -Without `--fix`, `codeaf doctor` changes nothing. `codeaf doctor --fix` releases unfinished continues started by this computer, and retires duplicate branch copies whose turns are all already in the main chat, printing one `fixed:` line for each repair. A store lock whose process has ended is already free, so doctor neither reports nor removes it, and a busy store waits for its holder to finish or quit. It never touches another computer's live hold and never deletes a turn that exists only in the copy or scratch folder it inspects. +Without `--fix`, `codeaf doctor` changes nothing. `codeaf doctor --fix` releases unfinished continues started by this computer, retires duplicate branch copies whose turns are all already in the main chat, and sends eligible chats missing from the directory, printing one `fixed:` line for each repair. A store lock whose process has ended is already free, so doctor neither reports nor removes it, and a busy store waits for its holder to finish or quit. It never touches another computer's live hold and never deletes a turn that exists only in the copy or scratch folder it inspects. ## codeaf help, and the verbs that take no flags at all diff --git a/internal/manual/chat/starting-codeaf.md b/internal/manual/chat/starting-codeaf.md index e513e6970a..4991688c80 100644 --- a/internal/manual/chat/starting-codeaf.md +++ b/internal/manual/chat/starting-codeaf.md @@ -129,7 +129,7 @@ stale is running. | `--reasoning ` | how hard the model is asked to think: `off`, `low`, `medium` or `high` | | `--session ` | open a particular conversation file instead of the most recent | | `--host ` | run the conversation on another machine over ssh | -| `--once ""` | send one message and print its replies — normally it then exits; with `--yolo` and a budget it stays until handed-over work is home or the limit ends it | +| `--once ""` | send one message and print its replies — when paired, finish the bounded final send to your other computers before exiting; with `--yolo` and a budget it stays until handed-over work is home or the limit ends it | | `--no-compact` | never shorten the conversation automatically | | `--yolo` | run every tool without asking, subject to the limits that nothing lifts | | `--max-hours ` | with `--yolo`: elapsed-time limit; interactive chat checks before new turns, and the window closes itself two minutes after it | diff --git a/internal/manual/chat/tasks.md b/internal/manual/chat/tasks.md index 565bce0143..05b2a06599 100644 --- a/internal/manual/chat/tasks.md +++ b/internal/manual/chat/tasks.md @@ -1373,7 +1373,7 @@ the sentence about it without typing anything. So you can start something, close lid on the conversation, and come back to the answer rather than to a card and silence. On a headless `--once --yolo` run with a budget, the command stays alive for those replies until no work is moving and no reply is in flight; without a budget, `--once` still exits -after its one reply. +after its one reply and, when paired, its bounded final send to your other computers. `jobs list` shows what is still running, and `jobs output ` shows what it has said so far. **A watch is two kinds of news and only one of them wakes you.** Its ordinary updates — the From 0207d3768f4ccdd6c3daec57a9a7d173cb3f01cb Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 30 Oct 2026 22:39:22 -0400 Subject: [PATCH 09/18] test: spell the Option chords the way the product does on macOS macOS names the Option key opt, so Home's continue offer shows opt+c and the Memory switch shows opt+u. Two tests hard-coded alt+. The Memory test now builds its expected line through the app's own chord spelling, and the takeback test waits for the offer's question and checks a continue chord is drawn beside it. Co-Authored-By: Claude Opus 5.5 --- cmd/codeaf/hosted_takeback_test.go | 6 +++++- internal/tui3/placeprose_test.go | 6 +++--- internal/tui3/spendmemwords_test.go | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/cmd/codeaf/hosted_takeback_test.go b/cmd/codeaf/hosted_takeback_test.go index 8567e37395..83d73feffb 100644 --- a/cmd/codeaf/hosted_takeback_test.go +++ b/cmd/codeaf/hosted_takeback_test.go @@ -493,7 +493,11 @@ func TestKilledComputerTakesBackFromHomeAndLocalCard(t *testing.T) { if _, err := io.WriteString(typed, " "); err != nil { t.Fatal(err) } - h.wait("Home continuation offer", func() bool { return strings.Contains(screen.String(), "alt+c") }) + // The question and its continue chord must be visible in the same offer. + h.wait("Home continuation offer", func() bool { + text := screen.String() + return strings.Contains(text, "Continue other computer's latest chat here?") && (strings.Contains(text, "alt+c") || strings.Contains(text, "opt+c")) + }) if _, err := io.WriteString(typed, "\x1bc"); err != nil { t.Fatal(err) } diff --git a/internal/tui3/placeprose_test.go b/internal/tui3/placeprose_test.go index 1aebdc7703..c77c73ba53 100644 --- a/internal/tui3/placeprose_test.go +++ b/internal/tui3/placeprose_test.go @@ -139,7 +139,7 @@ func TestEveryEmptyPlaceDrawsItsHeadingAndOneWhisper(t *testing.T) { a.width, a.height = size[0], size[1] lines, _, _, _ := a.placeDraw(placeFor(lab.id), a.width, a.height) body := make([]string, 0, len(lines)) - for _, line := range lines[emptyPlaceHeadingRow(t, lab.id, lines) : len(lines)-placeFootRowsFor(lab.id, size[1])+1] { + for _, line := range lines[emptyPlaceHeadingRow(t, a.chords, lab.id, lines) : len(lines)-placeFootRowsFor(lab.id, size[1])+1] { body = append(body, strings.TrimRight(plain(line), " ")) } want := placeWhisperLines(lab.id, a.width, newPalette(tokens.NoColor, false)) @@ -159,11 +159,11 @@ func TestEveryEmptyPlaceDrawsItsHeadingAndOneWhisper(t *testing.T) { // Memory has a saved-memory/knowledge switch above its empty-state guidance. // Assert that navigation explicitly before applying the common heading rules. -func emptyPlaceHeadingRow(t *testing.T, id page, rows []string) int { +func emptyPlaceHeadingRow(t *testing.T, chords chordSpelling, id page, rows []string) int { t.Helper() top := placeHeadRows if id == pageMemory { - if len(rows) <= top+2 || strings.TrimSpace(plain(rows[top])) != "Saved memory Knowledge · alt+u" || strings.TrimSpace(plain(rows[top+1])) != "" { + if len(rows) <= top+2 || strings.TrimSpace(plain(rows[top])) != chords.say("Saved memory Knowledge · alt+u") || strings.TrimSpace(plain(rows[top+1])) != "" { t.Fatalf("Memory must show its switch and spacing before empty-state guidance: %q", rows) } top += 2 diff --git a/internal/tui3/spendmemwords_test.go b/internal/tui3/spendmemwords_test.go index 51fbf8b36c..7e17262f06 100644 --- a/internal/tui3/spendmemwords_test.go +++ b/internal/tui3/spendmemwords_test.go @@ -246,7 +246,7 @@ func TestEveryPlaceWhisperWrapsLikeHomesAndIsNeverCut(t *testing.T) { for i, line := range lines { rows[i] = strings.TrimRight(plain(line), " ") } - said := whisperRowsUnder(rows, emptyPlaceHeadingRow(t, lab.id, rows)) + said := whisperRowsUnder(rows, emptyPlaceHeadingRow(t, a.chords, lab.id, rows)) if len(said) == 0 || len(said) > 3 { t.Fatalf("the empty %s place at %d cells whispers on %d rows:\n%s", lab.id.word(), width, len(said), strings.Join(rows, "\n")) From 7faad2dc228961693c43a5bd2a0f5a2ae683bf37 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 30 Oct 2026 23:22:12 -0400 Subject: [PATCH 10/18] changes: entry for #2086 Co-Authored-By: Claude Opus 5.5 --- .../2086-deleted-home-and-once-chats.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 docs/changes/unreleased/2086-deleted-home-and-once-chats.md diff --git a/docs/changes/unreleased/2086-deleted-home-and-once-chats.md b/docs/changes/unreleased/2086-deleted-home-and-once-chats.md new file mode 100644 index 0000000000..d70d7fa60b --- /dev/null +++ b/docs/changes/unreleased/2086-deleted-home-and-once-chats.md @@ -0,0 +1,16 @@ +--- +kind: fixed +title: A deleted codeaf home is never rewritten, and a chat --once chat always reaches your other computers +pr: 2086 +surface: [chat, engine] +invalidates: + - "An engine still saving after it was asked to stop could find its home deleted and write a brand-new identity.json, device.json, identity-solo and sync ledgers into it, as if it were a first run; codeaf now refuses with \"codeaf home was removed or replaced; restart codeaf to use the current folder\", never re-creates a home it started in, and a stopping engine exits at once when its home is removed. A first run is unchanged." + - "A paired codeaf chat --once could exit after uploading its chat but before registering it, so the other computer never listed it and codeaf doctor said nothing; --once now finishes its final send and release within the usual closing limit, codeaf doctor reports a chat this computer started that your other computers do not have, and codeaf doctor --fix registers it. A computer that never paired is unchanged." + - "Naming the process that holds a busy engine store could take many seconds on macOS (one lsof per lock file, 500 ms each); one short budget now covers the whole lookup." +--- + +Also fixed: tests that failed on macOS (Option chords spelled `opt+`, files that +take their folder's group, bare `mktemp` ignoring `TMPDIR`, a bash 3.2 empty +array in the release workflow) and a test that wrote a lock file into the source +tree. Engine-dependent tests skip on GitHub's full tests because no furrow is +staged there; that is a known follow-up. From 7d228308733c8992418fec7f74906efaad85220b Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 30 Oct 2026 23:32:37 -0400 Subject: [PATCH 11/18] test(syncsetup): make the quiet solo home in a fresh folder The quiet case removed a paired machine's folder and created a solo identity at the same path in the same process. codeaf now refuses to create an identity in a home it saw and that was then deleted, so the case uses a fresh folder, which is what a never-paired home is. Co-Authored-By: Claude Opus 5.5 --- internal/syncsetup/serve_state_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/syncsetup/serve_state_test.go b/internal/syncsetup/serve_state_test.go index 34917c78b3..7d13f977e9 100644 --- a/internal/syncsetup/serve_state_test.go +++ b/internal/syncsetup/serve_state_test.go @@ -105,7 +105,11 @@ func TestR42ServeHomeStateIneligibleHomesStaySilent(t *testing.T) { mustNil(t, os.RemoveAll(home)) mustNil(t, os.MkdirAll(home, 0o700)) case "quiet": - mustNil(t, os.RemoveAll(home)) + // A solo home is made in a fresh folder, because codeaf now refuses to + // create an identity in a home this process saw and that was then + // deleted (internal/home Witness), which is what removing the paired + // machine's folder and reusing its path would look like. + home = filepath.Join(t.TempDir(), "solo") _, err := identity.EnsureSolo(home) mustNil(t, err) case "cells unavailable": From e1a6b3b8f49ef57b00400c005427432785dd1045 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Sat, 31 Oct 2026 00:08:18 -0400 Subject: [PATCH 12/18] test(session): task-copy fixtures that hold on macOS 42 internal/session subtests failed on macOS and passed on Linux: - On macOS a new file takes its folder's group, so the Apply fixtures' files often carried a group other than the process's and the portable-metadata check refused them during setup, before the test's own behaviour ran. The success fixtures now give their files the process's uid and gid, resolve temp-path aliases first, and clear extended attributes. The product refusal stays: private Apply stages replacements with the process's group, so a foreign group cannot be carried. A new setgid-directory test pins that refusal on Linux. - Bare mktemp on macOS ignores TMPDIR, so five scratch probes now use mktemp "$TMPDIR/codeaf-runtime.XXXXXX". - Two path checks compared a canonical path with a raw temp path; both sides are canonicalized now. Co-Authored-By: Claude Opus 5.5 --- internal/executor/environment_test.go | 3 +- internal/session/task_workspace_apply_test.go | 89 +++++++++--- .../task_workspace_fixture_other_test.go | 10 ++ .../task_workspace_fixture_unix_test.go | 127 ++++++++++++++++++ .../task_workspace_transaction_test.go | 10 ++ internal/session/workspace_chat_test.go | 5 +- internal/session/workspace_identity_test.go | 14 +- .../session/workspace_node_ownership_test.go | 3 +- .../workspace_ownership_public_test.go | 3 +- 9 files changed, 238 insertions(+), 26 deletions(-) create mode 100644 internal/session/task_workspace_fixture_other_test.go create mode 100644 internal/session/task_workspace_fixture_unix_test.go diff --git a/internal/executor/environment_test.go b/internal/executor/environment_test.go index 3776670d81..07ee003d06 100644 --- a/internal/executor/environment_test.go +++ b/internal/executor/environment_test.go @@ -13,10 +13,11 @@ import ( ) func TestWorkspaceEnvironmentRoutesRealTemporaryFilesThroughBothRunnerDoors(t *testing.T) { + // macOS mktemp needs an explicit template to create its file inside the assigned TMPDIR. scratch, cache := t.TempDir(), t.TempDir() originalHOME := os.Getenv("HOME") seat := executor.WithEnvironment(executor.Host, []string{"TMPDIR=" + scratch, "XDG_CACHE_HOME=" + cache, "CODEAF_WORKSPACE_URI=codeaf-workspace://example/run"}) - command := `printf '%s\n' "$HOME" "$XDG_CACHE_HOME" "$CODEAF_WORKSPACE_URI"; mktemp` + command := `printf '%s\n' "$HOME" "$XDG_CACHE_HOME" "$CODEAF_WORKSPACE_URI"; mktemp "$TMPDIR/codeaf-runtime.XXXXXX"` for _, door := range []string{"exec", "command", "setup"} { runner := seat.In(t.TempDir()) if door == "setup" { diff --git a/internal/session/task_workspace_apply_test.go b/internal/session/task_workspace_apply_test.go index ae040353e5..3adcb6b9da 100644 --- a/internal/session/task_workspace_apply_test.go +++ b/internal/session/task_workspace_apply_test.go @@ -13,20 +13,49 @@ import ( "github.com/Agent-Field/codeaf/internal/home" ) +// macOS temp roots can contain symlink ancestors that explicit workspace admission refuses. +func workspaceApplyFixtureDir(t *testing.T) string { + t.Helper() + root, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + prepareWorkspaceApplyFixture(t, root) + return root +} + +// macOS success fixtures need supported ownership and no inherited extended attributes. +func prepareWorkspaceApplyFixture(t *testing.T, roots ...string) { + t.Helper() + for _, root := range roots { + if err := filepath.WalkDir(root, func(path string, entry os.DirEntry, err error) error { + if err != nil { + return err + } + normalizeWorkspaceApplyFixtureMetadata(t, path) + return nil + }); err != nil { + t.Fatalf("prepare Apply fixture %s: %v", root, err) + } + } +} + func TestTaskWorkspaceExplicitReviewAndApply(t *testing.T) { + // macOS needs canonical, metadata-free fixtures before the successful Review and Apply. for _, repository := range []bool{false, true} { t.Run(map[bool]string{false: "folder", true: "repository"}[repository], func(t *testing.T) { - t.Setenv("CODEAF_HOME", t.TempDir()) + t.Setenv("CODEAF_HOME", workspaceApplyFixtureDir(t)) t.Setenv("CODEAF_PROTOTYPE_WORKSPACE_OWNERSHIP", "1") t.Setenv("CODEAF_TASK_BELT", "bash") t.Setenv("CODEAF_PROTOTYPE_TASK_STORAGE", "1") - source := t.TempDir() + source := workspaceApplyFixtureDir(t) if repository { - source = newTestRepo(t) + source = canonicalPath(newTestRepo(t)) } else { writeFile(t, filepath.Join(source, "shared.txt"), "the original line\n") } - place := Place{Dir: t.TempDir(), Workspace: source} + prepareWorkspaceApplyFixture(t, source) + place := Place{Dir: workspaceApplyFixtureDir(t), Workspace: source} engine := newBeltRunDouble("done") engine.real = true registerBeltRunEngine(t, engine) @@ -68,6 +97,7 @@ func TestTaskWorkspaceExplicitReviewAndApply(t *testing.T) { t.Fatal(err) } t.Setenv("CODEAF_PROTOTYPE_WORKSPACE_OWNERSHIP", "") + prepareWorkspaceApplyFixture(t, copyDir) review, err := agent.ReviewTaskWorkspaceRoot(context.Background(), id, "work") if err != nil { t.Fatal(err) @@ -111,12 +141,14 @@ func TestTaskWorkspaceExplicitReviewAndApply(t *testing.T) { } func TestTaskWorkspaceApplyPreservesConflictingOriginalAndPrivateCopy(t *testing.T) { - t.Setenv("CODEAF_HOME", t.TempDir()) + // macOS needs a supported successful Review before this test introduces its content conflict. + t.Setenv("CODEAF_HOME", workspaceApplyFixtureDir(t)) t.Setenv("CODEAF_PROTOTYPE_WORKSPACE_OWNERSHIP", "1") t.Setenv("CODEAF_TASK_BELT", "bash") - source := t.TempDir() + source := workspaceApplyFixtureDir(t) writeFile(t, filepath.Join(source, "shared.txt"), "baseline\n") - place := Place{Dir: t.TempDir(), Workspace: source} + prepareWorkspaceApplyFixture(t, source) + place := Place{Dir: workspaceApplyFixtureDir(t), Workspace: source} engine := newBeltRunDouble("done") engine.real = true registerBeltRunEngine(t, engine) @@ -131,6 +163,7 @@ func TestTaskWorkspaceApplyPreservesConflictingOriginalAndPrivateCopy(t *testing engine.mu.Unlock() writeFile(t, filepath.Join(copyDir, "shared.txt"), "private version\n") endBeltRun(t, agent, engine) + prepareWorkspaceApplyFixture(t, copyDir) if _, err := agent.ReviewTaskWorkspaceRoot(context.Background(), id, "work"); err != nil { t.Fatal(err) } @@ -155,21 +188,31 @@ func completedWorkspaceApplyFixture(t *testing.T, repository bool) (*Agent, uint } func completedWorkspaceApplyFixtureWithInput(t *testing.T, repository bool, setup func(string)) (*Agent, uint64, Place, string, string) { + // macOS success fixtures need primary ownership while setup may deliberately retain a different root group. t.Helper() - t.Setenv("CODEAF_HOME", t.TempDir()) + t.Setenv("CODEAF_HOME", workspaceApplyFixtureDir(t)) t.Setenv("CODEAF_PROTOTYPE_WORKSPACE_OWNERSHIP", "1") t.Setenv("CODEAF_TASK_BELT", "bash") t.Setenv("CODEAF_FURROW", filepath.Join(t.TempDir(), "unavailable-native-engine")) - source := t.TempDir() + source := workspaceApplyFixtureDir(t) if repository { - source = newTestRepo(t) + source = canonicalPath(newTestRepo(t)) } else { writeFile(t, filepath.Join(source, "shared.txt"), "the original line\n") } + prepareWorkspaceApplyFixture(t, source) if setup != nil { setup(source) + // Keep the root group chosen by setup so the inheritance test still exercises it. + entries, err := os.ReadDir(source) + if err != nil { + t.Fatal(err) + } + for _, entry := range entries { + prepareWorkspaceApplyFixture(t, filepath.Join(source, entry.Name())) + } } - place := Place{Dir: t.TempDir(), Workspace: source} + place := Place{Dir: workspaceApplyFixtureDir(t), Workspace: source} if err := SaveMeta(place.Dir, Meta{ID: place.ID(), Workspace: source, WorkspacePolicy: DefaultWorkspacePolicy()}); err != nil { t.Fatal(err) } @@ -187,6 +230,7 @@ func completedWorkspaceApplyFixtureWithInput(t *testing.T, repository bool, setu engine.mu.Unlock() writeFile(t, filepath.Join(copyDir, "shared.txt"), "private result\n") endBeltRun(t, agent, engine) + prepareWorkspaceApplyFixture(t, copyDir) if _, err := agent.ReviewTaskWorkspaceRoot(context.Background(), id, "work"); err != nil { t.Fatal(err) } @@ -194,6 +238,7 @@ func completedWorkspaceApplyFixtureWithInput(t *testing.T, repository bool, setu } func TestTaskWorkspaceApplyRejectsAlteredStageReceipt(t *testing.T) { + // macOS needs an unadorned foreign fixture so the refusal still exercises the altered receipt. for _, kind := range []string{"cleanup-path", "stage-destination", "comparison-baseline"} { t.Run(kind, func(t *testing.T) { agent, id, _, source, copyDir := completedWorkspaceApplyFixture(t, kind != "comparison-baseline") @@ -215,9 +260,10 @@ func TestTaskWorkspaceApplyRejectsAlteredStageReceipt(t *testing.T) { case "cleanup-path": ticket.Stage.Dir = copyDir case "stage-destination": - foreign := newTestRepo(t) + foreign := canonicalPath(newTestRepo(t)) + prepareWorkspaceApplyFixture(t, foreign) // A second authentic review can be transplanted without inventing any URI. - otherPlace := Place{Dir: t.TempDir(), Workspace: foreign} + otherPlace := Place{Dir: workspaceApplyFixtureDir(t), Workspace: foreign} if err := SaveMeta(otherPlace.Dir, Meta{ID: otherPlace.ID(), Workspace: foreign, WorkspacePolicy: DefaultWorkspacePolicy()}); err != nil { t.Fatal(err) } @@ -235,6 +281,7 @@ func TestTaskWorkspaceApplyRejectsAlteredStageReceipt(t *testing.T) { engine.mu.Unlock() writeFile(t, filepath.Join(foreignCopy, "foreign.txt"), "unselected root\n") endBeltRun(t, other, engine) + prepareWorkspaceApplyFixture(t, foreignCopy) if _, err := other.ReviewTaskWorkspaceRoot(context.Background(), otherID, "work"); err != nil { t.Fatal(err) } @@ -300,8 +347,10 @@ func TestTaskWorkspaceReviewReplacesItsOwnedStagingCopy(t *testing.T) { } func TestTaskWorkspaceLargeReviewNamesItsImmutableDiff(t *testing.T) { + // macOS can inherit unsupported metadata on files and directories added after the initial Review. agent, id, _, _, copyDir := completedWorkspaceApplyFixture(t, true) writeFile(t, filepath.Join(copyDir, "large.txt"), strings.Repeat("large immutable line\n", 20000)) + prepareWorkspaceApplyFixture(t, copyDir) review, err := agent.ReviewTaskWorkspaceRoot(context.Background(), id, "work") if err != nil { t.Fatal(err) @@ -324,6 +373,7 @@ func TestTaskWorkspaceLargeReviewNamesItsImmutableDiff(t *testing.T) { } func TestTaskWorkspaceApplyPreservesReviewedPortableMetadata(t *testing.T) { + // macOS can inherit unsupported metadata on files and directories added after the initial Review. for _, kind := range []string{"mode-and-crlf", "empty-directory", "directory-mode"} { t.Run(kind, func(t *testing.T) { agent, id, _, source, copyDir := completedWorkspaceApplyFixture(t, false) @@ -345,6 +395,7 @@ func TestTaskWorkspaceApplyPreservesReviewedPortableMetadata(t *testing.T) { } writeFile(t, filepath.Join(copyDir, "nested", "result.txt"), "nested result\n") } + prepareWorkspaceApplyFixture(t, copyDir) if _, err := agent.ReviewTaskWorkspaceRoot(context.Background(), id, "work"); err != nil { t.Fatal(err) } @@ -381,14 +432,15 @@ func TestTaskWorkspaceApplyPreservesReviewedPortableMetadata(t *testing.T) { } func TestTaskWorkspaceStartsWithPortableSourceRepresentation(t *testing.T) { + // macOS temp aliases must resolve before these explicit source roots are admitted. for _, repository := range []bool{false, true} { t.Run(map[bool]string{false: "folder", true: "repository"}[repository], func(t *testing.T) { - t.Setenv("CODEAF_HOME", t.TempDir()) + t.Setenv("CODEAF_HOME", workspaceApplyFixtureDir(t)) t.Setenv("CODEAF_PROTOTYPE_WORKSPACE_OWNERSHIP", "1") t.Setenv("CODEAF_TASK_BELT", "bash") - source := t.TempDir() + source := workspaceApplyFixtureDir(t) if repository { - source = newTestRepo(t) + source = canonicalPath(newTestRepo(t)) } writeFile(t, filepath.Join(source, ".gitattributes"), "shared.txt text eol=lf\n") writeFile(t, filepath.Join(source, "shared.txt"), "input\r\n") @@ -398,7 +450,8 @@ func TestTaskWorkspaceStartsWithPortableSourceRepresentation(t *testing.T) { if err := os.Mkdir(filepath.Join(source, "empty"), 0750); err != nil { t.Fatal(err) } - place := Place{Dir: t.TempDir(), Workspace: source} + prepareWorkspaceApplyFixture(t, source) + place := Place{Dir: workspaceApplyFixtureDir(t), Workspace: source} engine := newBeltRunDouble("done") engine.real = true registerBeltRunEngine(t, engine) @@ -430,6 +483,7 @@ func TestTaskWorkspaceStartsWithPortableSourceRepresentation(t *testing.T) { } func TestTaskWorkspaceApplyUsesExplicitDirectoryDeletionCoverage(t *testing.T) { + // macOS can inherit unsupported metadata on files and directories added after the initial Review. for _, remove := range []bool{false, true} { t.Run(map[bool]string{false: "newly-ignored-retained", true: "explicit-empty-directory-deletion"}[remove], func(t *testing.T) { agent, id, _, source, copyDir := completedWorkspaceApplyFixtureWithInput(t, false, func(source string) { @@ -444,6 +498,7 @@ func TestTaskWorkspaceApplyUsesExplicitDirectoryDeletionCoverage(t *testing.T) { } else { writeFile(t, filepath.Join(copyDir, ".gitignore"), "keep/\n") } + prepareWorkspaceApplyFixture(t, copyDir) if _, err := agent.ReviewTaskWorkspaceRoot(context.Background(), id, "work"); err != nil { t.Fatal(err) } diff --git a/internal/session/task_workspace_fixture_other_test.go b/internal/session/task_workspace_fixture_other_test.go new file mode 100644 index 0000000000..f7b7836036 --- /dev/null +++ b/internal/session/task_workspace_fixture_other_test.go @@ -0,0 +1,10 @@ +//go:build !linux && !darwin + +package session + +import "testing" + +// Private Apply is supported only on Linux and macOS, where ownership can be prepared. +func normalizeWorkspaceApplyFixtureMetadata(t *testing.T, path string) { + t.Helper() +} diff --git a/internal/session/task_workspace_fixture_unix_test.go b/internal/session/task_workspace_fixture_unix_test.go new file mode 100644 index 0000000000..7a1aeddec3 --- /dev/null +++ b/internal/session/task_workspace_fixture_unix_test.go @@ -0,0 +1,127 @@ +//go:build linux || darwin + +package session + +import ( + "errors" + "os" + "path/filepath" + "runtime" + "strings" + "syscall" + "testing" + + "golang.org/x/sys/unix" +) + +// macOS inherits directory groups and can attach provenance to otherwise plain fixture files. +func normalizeWorkspaceApplyFixtureMetadata(t *testing.T, path string) { + t.Helper() + if err := os.Lchown(path, os.Getuid(), os.Getgid()); err != nil { + t.Fatalf("set Apply fixture ownership for %s: %v", path, err) + } + for _, name := range workspaceApplyFixtureAttributes(t, path) { + if err := unix.Lremovexattr(path, name); err != nil { + t.Fatalf("remove extended attribute %q from Apply fixture %s: %v", name, path, err) + } + } + if names := workspaceApplyFixtureAttributes(t, path); len(names) != 0 { + t.Fatalf("extended attribute %q remains on Apply fixture %s after removal", names[0], path) + } +} + +func workspaceApplyFixtureAttributes(t *testing.T, path string) []string { + t.Helper() + size, err := unix.Llistxattr(path, nil) + if errors.Is(err, unix.ENOTSUP) { + return nil + } + if err != nil { + t.Fatalf("list extended attributes on Apply fixture %s: %v", path, err) + } + if size == 0 { + return nil + } + buffer := make([]byte, size) + n, err := unix.Llistxattr(path, buffer) + if err != nil { + t.Fatalf("read extended attributes on Apply fixture %s: %v", path, err) + } + return strings.Split(strings.TrimSuffix(string(buffer[:n]), "\x00"), "\x00") +} + +func TestTaskWorkspaceReviewRefusesInheritedGroupUntilFixtureIsNormalized(t *testing.T) { + // SPEC-4 pins macOS directory-group inheritance on Linux with a setgid fixture. + groups, err := os.Getgroups() + if err != nil { + t.Fatal(err) + } + secondary := -1 + for _, group := range groups { + if group != os.Getgid() { + secondary = group + break + } + } + if secondary < 0 { + t.Skip("no supplementary group differs from the process primary group for the setgid inheritance fixture") + } + agent, id, _, source, copyDir := completedWorkspaceApplyFixture(t, false) + root := workspaceApplyFixtureDir(t) + if err := os.Chown(root, os.Getuid(), secondary); err != nil { + t.Fatalf("set inherited-group fixture to supplementary group %d: %v", secondary, err) + } + if err := os.Chmod(root, 0750|os.ModeSetgid); err != nil { + t.Fatal(err) + } + path := filepath.Join(root, "inherited.txt") + writeFile(t, path, "inherited group\n") + info, err := os.Lstat(path) + if err != nil { + t.Fatal(err) + } + if stat := info.Sys().(*syscall.Stat_t); stat.Gid != uint32(secondary) { + t.Fatalf("setgid fixture file group = %d, want inherited group %d", stat.Gid, secondary) + } + private := filepath.Join(copyDir, "inherited.txt") + if err := os.Rename(path, private); err != nil { + t.Fatal(err) + } + if _, err := agent.ReviewTaskWorkspaceRoot(t.Context(), id, "work"); err == nil || !strings.Contains(err.Error(), "portable apply cannot preserve a different owner or group") { + t.Fatalf("Review did not refuse inherited file ownership: %v", err) + } + if _, err := os.Stat(filepath.Join(source, "inherited.txt")); !os.IsNotExist(err) { + t.Fatalf("refused Review changed the original: %v", err) + } + prepareWorkspaceApplyFixture(t, private) + if _, err := agent.ReviewTaskWorkspaceRoot(t.Context(), id, "work"); err != nil { + t.Fatalf("normalized fixture still refused Review: %v", err) + } +} + +func TestTaskWorkspaceReviewAcceptsFixtureAfterRemovingExtendedAttributes(t *testing.T) { + // macOS provenance must be removed from success fixtures without relaxing Review's metadata refusal. + agent, id, _, source, copyDir := completedWorkspaceApplyFixture(t, false) + attribute := "user.codeaf.fixture" + if runtime.GOOS == "darwin" { + attribute = "com.codeaf.fixture" + } + for _, root := range []string{source, copyDir} { + if err := unix.Lsetxattr(filepath.Join(root, "shared.txt"), attribute, []byte("fixture metadata"), 0); err != nil { + t.Fatalf("set extended attribute %q on fixture %s: %v", attribute, root, err) + } + } + if _, err := agent.ReviewTaskWorkspaceRoot(t.Context(), id, "work"); err == nil || !strings.Contains(err.Error(), "portable apply cannot preserve extended attributes or access-control metadata") { + t.Fatalf("Review did not refuse fixture attributes: %v", err) + } + prepareWorkspaceApplyFixture(t, source, copyDir) + if _, err := agent.ReviewTaskWorkspaceRoot(t.Context(), id, "work"); err != nil { + t.Fatalf("metadata-free fixture still refused Review: %v", err) + } + if _, err := agent.ApplyTaskWorkspaceRoot(t.Context(), id, "work"); err != nil { + t.Fatalf("metadata-free fixture still refused Apply: %v", err) + } + if got := readFile(t, filepath.Join(source, "shared.txt")); got != "private result\n" { + t.Fatalf("metadata-free Apply changed reviewed bytes: %q", got) + } +} diff --git a/internal/session/task_workspace_transaction_test.go b/internal/session/task_workspace_transaction_test.go index ca8f7f3425..33978cb637 100644 --- a/internal/session/task_workspace_transaction_test.go +++ b/internal/session/task_workspace_transaction_test.go @@ -98,6 +98,7 @@ func TestTaskWorkspaceTransactionRejectsChangedPermissions(t *testing.T) { } func TestTaskWorkspaceTransactionGitRefFailureRollsBackPortableState(t *testing.T) { + // macOS additions need supported metadata without changing the original project root group. agent, id, _, source, copyDir := completedWorkspaceApplyFixtureWithInput(t, true, func(root string) { if err := os.Symlink("shared.txt", filepath.Join(root, "pointer")); err != nil { t.Fatal(err) @@ -120,6 +121,7 @@ func TestTaskWorkspaceTransactionGitRefFailureRollsBackPortableState(t *testing. if err := os.Mkdir(filepath.Join(copyDir, "empty"), 0750); err != nil { t.Fatal(err) } + prepareWorkspaceApplyFixture(t, copyDir) if _, err := agent.ReviewTaskWorkspaceRoot(context.Background(), id, "work"); err != nil { t.Fatal(err) } @@ -193,10 +195,12 @@ func TestTaskWorkspaceTransactionPreservesUnrelatedStagedAndDirtyFiles(t *testin } func TestTaskWorkspaceTransactionPreservesRawBytesWithCleanGitRepresentation(t *testing.T) { + // macOS additions need supported metadata without changing the original project root group. agent, id, _, source, copyDir := completedWorkspaceApplyFixture(t, true) mustGit(t, source, "config", "core.safecrlf", "warn") writeFile(t, filepath.Join(copyDir, ".gitattributes"), "*.txt text eol=lf\n") writeFile(t, filepath.Join(copyDir, "shared.txt"), "reviewed CRLF\r\n") + prepareWorkspaceApplyFixture(t, copyDir) if _, err := agent.ReviewTaskWorkspaceRoot(context.Background(), id, "work"); err != nil { t.Fatal(err) } @@ -272,11 +276,13 @@ func TestTaskWorkspaceTransactionCheckpointFailureReplaysWithoutApplyingAgain(t } func TestTaskWorkspaceTransactionPreservesGitFilenames(t *testing.T) { + // macOS additions need supported metadata without changing the original project root group. agent, id, _, source, copyDir := completedWorkspaceApplyFixture(t, true) paths := []string{"has space.txt", "line\nbreak.txt", "-leading.txt", "café-文件.txt"} for _, path := range paths { writeFile(t, filepath.Join(copyDir, path), "reviewed "+path+"\r\n") } + prepareWorkspaceApplyFixture(t, copyDir) if _, err := agent.ReviewTaskWorkspaceRoot(context.Background(), id, "work"); err != nil { t.Fatal(err) } @@ -294,12 +300,14 @@ func TestTaskWorkspaceTransactionPreservesGitFilenames(t *testing.T) { } func TestTaskWorkspaceTransactionApplyScale(t *testing.T) { + // macOS additions need supported metadata without changing the original project root group. for _, count := range []int{100, 1000} { t.Run(fmt.Sprint(count), func(t *testing.T) { agent, id, _, source, copyDir := completedWorkspaceApplyFixture(t, true) for index := 0; index < count; index++ { writeFile(t, filepath.Join(copyDir, fmt.Sprintf("result-%04d.txt", index)), "reviewed result\r\n") } + prepareWorkspaceApplyFixture(t, copyDir) if _, err := agent.ReviewTaskWorkspaceRoot(context.Background(), id, "work"); err != nil { t.Fatal(err) } @@ -413,6 +421,7 @@ func TestTaskWorkspaceTransactionRefusesConfiguredSplitPreference(t *testing.T) } func TestTaskWorkspaceTransactionKeepsRootGroupAndNormalizesNewDirectory(t *testing.T) { + // macOS additions need supported metadata without changing the original project root group. groups, err := os.Getgroups() if err != nil { t.Fatal(err) @@ -435,6 +444,7 @@ func TestTaskWorkspaceTransactionKeepsRootGroupAndNormalizesNewDirectory(t *test if err := os.Mkdir(filepath.Join(copyDir, "new-directory"), 0750); err != nil { t.Fatal(err) } + prepareWorkspaceApplyFixture(t, copyDir) if _, err := agent.ReviewTaskWorkspaceRoot(context.Background(), id, "work"); err != nil { t.Fatal(err) } diff --git a/internal/session/workspace_chat_test.go b/internal/session/workspace_chat_test.go index 31073263b4..340a96c830 100644 --- a/internal/session/workspace_chat_test.go +++ b/internal/session/workspace_chat_test.go @@ -13,6 +13,7 @@ import ( ) func TestOwnershipChatRoutesScratchWithoutMovingTheProject(t *testing.T) { + // macOS mktemp needs an explicit template to create its file inside the assigned TMPDIR. t.Setenv("CODEAF_HOME", t.TempDir()) t.Setenv("CODEAF_PROTOTYPE_WORKSPACE_OWNERSHIP", "1") project := t.TempDir() @@ -20,7 +21,7 @@ func TestOwnershipChatRoutesScratchWithoutMovingTheProject(t *testing.T) { c.Workspace = project c.Place = Place{Dir: t.TempDir(), Workspace: project} }) - if a.config.Workspace != project { + if canonicalPath(a.config.Workspace) != canonicalPath(project) { t.Fatal("a borrowed project was moved") } registry, err := workspaceidentity.Open(home.Join("workspace-identities")) @@ -31,7 +32,7 @@ func TestOwnershipChatRoutesScratchWithoutMovingTheProject(t *testing.T) { if err != nil || chat.Role != workspaceidentity.Chat { t.Fatalf("chat identity missing: %v", err) } - result, err := a.config.seat().In(project).Exec(context.Background(), executor.ExecRequest{Argv: []string{"sh", "-c", "mktemp"}}, nil) + result, err := a.config.seat().In(project).Exec(context.Background(), executor.ExecRequest{Argv: []string{"sh", "-c", `mktemp "$TMPDIR/codeaf-runtime.XXXXXX"`}}, nil) if err != nil { t.Fatal(err) } diff --git a/internal/session/workspace_identity_test.go b/internal/session/workspace_identity_test.go index 706cdf1ba6..c31355ba5f 100644 --- a/internal/session/workspace_identity_test.go +++ b/internal/session/workspace_identity_test.go @@ -128,6 +128,7 @@ func TestOneConversationCanOwnRunsForDifferentProjects(t *testing.T) { } func TestOwnedBeltWorkerUsesRealScratchWithoutChangingHome(t *testing.T) { + // macOS mktemp needs an explicit template to create its file inside the assigned TMPDIR. t.Setenv("CODEAF_PROTOTYPE_WORKSPACE_OWNERSHIP", "1") t.Setenv(home.EnvVar, t.TempDir()) t.Setenv("CODEAF_TASK_BELT", "bash") @@ -148,7 +149,7 @@ func TestOwnedBeltWorkerUsesRealScratchWithoutChangingHome(t *testing.T) { defer store.Close() script := &scriptedCompleter{steps: []step{ func(context.Context, []ai.Message) (*ai.Response, error) { - return toolResponse("scratch", "bash", `{"command":"printf '%s\n' \"$HOME\" \"$TMPDIR\" \"$XDG_CACHE_HOME\" \"$CODEAF_WORKSPACE_URI\" > runtime.txt; mktemp >> runtime.txt"}`), nil + return toolResponse("scratch", "bash", `{"command":"printf '%s\n' \"$HOME\" \"$TMPDIR\" \"$XDG_CACHE_HOME\" \"$CODEAF_WORKSPACE_URI\" > runtime.txt; mktemp \"$TMPDIR/codeaf-runtime.XXXXXX\" >> runtime.txt"}`), nil }, func(context.Context, []ai.Message) (*ai.Response, error) { return textResponse("done"), nil }, }} @@ -426,11 +427,16 @@ func TestOwnedCopyCannotLandInAnotherConversationsKnownRun(t *testing.T) { } func TestExplicitInPlaceWorkerRoutesScratchWithoutOwningTheProject(t *testing.T) { + // macOS mktemp needs an explicit template to create its file inside the assigned TMPDIR. t.Setenv("CODEAF_PROTOTYPE_WORKSPACE_OWNERSHIP", "1") t.Setenv(home.EnvVar, t.TempDir()) t.Setenv("CODEAF_TASK_BELT", "bash") t.Setenv(planCLIBinEnv, filepath.Join(t.TempDir(), "stub-codeaf")) - project := t.TempDir() + project := filepath.Join(t.TempDir(), "project") + // A symlink alias exercises the canonical path comparison used with macOS temp directories. + if err := os.Symlink(t.TempDir(), project); err != nil { + t.Fatal(err) + } place := Place{Dir: t.TempDir()} tree, err := registerOwnedWorkspace(taskTree{dir: project, ground: project, mode: TaskModeInPlace}, place) if err != nil { @@ -443,7 +449,7 @@ func TestExplicitInPlaceWorkerRoutesScratchWithoutOwningTheProject(t *testing.T) defer store.Close() script := &scriptedCompleter{steps: []step{ func(context.Context, []ai.Message) (*ai.Response, error) { - arguments, _ := json.Marshal(map[string]string{"command": `pwd > placement.txt; printf '%s\n' "$TMPDIR" "$XDG_CACHE_HOME" "$CODEAF_WORKSPACE_URI" >> placement.txt; mktemp >> placement.txt`}) + arguments, _ := json.Marshal(map[string]string{"command": `pwd > placement.txt; printf '%s\n' "$TMPDIR" "$XDG_CACHE_HOME" "$CODEAF_WORKSPACE_URI" >> placement.txt; mktemp "$TMPDIR/codeaf-runtime.XXXXXX" >> placement.txt`}) return toolResponse("runtime", "bash", string(arguments)), nil }, func(context.Context, []ai.Message) (*ai.Response, error) { return textResponse("done"), nil }, @@ -459,7 +465,7 @@ func TestExplicitInPlaceWorkerRoutesScratchWithoutOwningTheProject(t *testing.T) t.Fatal(err) } lines := strings.Split(strings.TrimSpace(string(raw)), "\n") - if len(lines) != 5 || canonicalPath(lines[0]) != project || lines[3] != tree.projectURI || filepath.Dir(lines[4]) != lines[1] { + if len(lines) != 5 || canonicalPath(lines[0]) != canonicalPath(project) || lines[3] != tree.projectURI || filepath.Dir(lines[4]) != lines[1] { t.Fatalf("in-place worker changed placement: %q", raw) } registry, err := workspaceidentity.Open(home.Join("workspace-identities")) diff --git a/internal/session/workspace_node_ownership_test.go b/internal/session/workspace_node_ownership_test.go index 9c858099ad..0a674e97bc 100644 --- a/internal/session/workspace_node_ownership_test.go +++ b/internal/session/workspace_node_ownership_test.go @@ -142,11 +142,12 @@ func TestLegacyOwnedTaskUnknownIdentityRefusesFreshWorld(t *testing.T) { } func TestLegacyOwnedWorkerUsesItsAdmittedRuntime(t *testing.T) { + // macOS mktemp needs an explicit template to create its file inside the assigned TMPDIR. t.Setenv(home.EnvVar, t.TempDir()) t.Setenv("CODEAF_PROTOTYPE_WORKSPACE_OWNERSHIP", "1") script := &scriptedCompleter{steps: []step{ func(context.Context, []ai.Message) (*ai.Response, error) { - return toolResponse("runtime", "bash", `{"command":"printf '%s\\n' \"$CODEAF_WORKSPACE_URI\" \"$TMPDIR\" \"$XDG_CACHE_HOME\" > runtime.txt; mktemp >> runtime.txt"}`), nil + return toolResponse("runtime", "bash", `{"command":"printf '%s\\n' \"$CODEAF_WORKSPACE_URI\" \"$TMPDIR\" \"$XDG_CACHE_HOME\" > runtime.txt; mktemp \"$TMPDIR/codeaf-runtime.XXXXXX\" >> runtime.txt"}`), nil }, func(context.Context, []ai.Message) (*ai.Response, error) { return textResponse("done"), nil }, }} diff --git a/internal/session/workspace_ownership_public_test.go b/internal/session/workspace_ownership_public_test.go index 3078b022a1..bfb74e3abe 100644 --- a/internal/session/workspace_ownership_public_test.go +++ b/internal/session/workspace_ownership_public_test.go @@ -151,6 +151,7 @@ func TestOwnershipStartTaskReopensPendingWorkspaceSet(t *testing.T) { } func TestOwnershipStartTaskPreservesExplicitInPlaceAndHomeScratch(t *testing.T) { + // macOS temp paths may use aliases for the canonical placement recorded by the worker. for _, choice := range []string{"in place", "home"} { t.Run(choice, func(t *testing.T) { t.Setenv("CODEAF_HOME", t.TempDir()) @@ -186,7 +187,7 @@ func TestOwnershipStartTaskPreservesExplicitInPlaceAndHomeScratch(t *testing.T) if spec.WorkspaceSet != nil { t.Fatal("explicit in-place or home reference was converted into a snapshot") } - if choice == "in place" && spec.Workspace != project { + if choice == "in place" && canonicalPath(spec.Workspace) != canonicalPath(project) { t.Fatalf("explicit placement moved to %s", spec.Workspace) } if choice == "home" { From 765254c86b3c85096f2be3b23699d9431bfdac0e Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Sat, 31 Oct 2026 00:35:19 -0400 Subject: [PATCH 13/18] Revert "manual: --once waits for your other computers, doctor finds a chat that did not arrive, a deleted home is refused" This reverts commit f28b0f4af211c6b1494a46136fc4f8b33107076b. --- internal/manual/chat/keys.md | 3 +- .../manual/chat/running-from-the-terminal.md | 54 +------------------ internal/manual/chat/starting-codeaf.md | 2 +- internal/manual/chat/tasks.md | 2 +- 4 files changed, 5 insertions(+), 56 deletions(-) diff --git a/internal/manual/chat/keys.md b/internal/manual/chat/keys.md index a0cfa1f375..cabfabcec4 100644 --- a/internal/manual/chat/keys.md +++ b/internal/manual/chat/keys.md @@ -690,8 +690,7 @@ in it`. Background jobs go the same way, a moment later. Either way, what a task wrote is on its branch and stays there; a stop is an interruption and never a finding about the work. A headless `codeaf chat --once` leaves by the closing -road: the turn stops, the session closes, and the process exits cleanly. A paired chat -also gives its final send and release the usual closing time limit before exiting. +road: the turn stops, the session closes, and the process exits cleanly. ## Keys — what all the keys do, the keyboard keys, keys on the keyboard, key bindings and keyboard shortcuts diff --git a/internal/manual/chat/running-from-the-terminal.md b/internal/manual/chat/running-from-the-terminal.md index 1d27caffca..20122d9a56 100644 --- a/internal/manual/chat/running-from-the-terminal.md +++ b/internal/manual/chat/running-from-the-terminal.md @@ -1624,61 +1624,11 @@ machine that counted, which is the opposite of the truth. That command is about this *install*. `/status` in the chat is about the conversation you are in. They are related and they are not the same reading. -## Does chat --once wait for my other computers before it exits? - -When you are paired, `codeaf chat --once ""` waits for the chat's last -saved turn to reach your other computers and for this computer to let go of the -chat before it exits. It uses the interactive chat's closing limit: up to 30 -seconds for the final send and release. If the final send or release fails or reaches the limit, -standard error says `this chat could not finish syncing with your other computers; run codeaf doctor --fix or reopen this chat to try again.` -The reply on standard output and the turn's exit status stay the same. On a -computer that has never paired, the command has no added sync wait or requests. - -## My chat did not show up on my other computer — codeaf doctor and --fix - -On a paired computer, `codeaf doctor` can report -`chat has not reached your other computers · chat `. It checks owned chats -started and first saved here while paired, with conversation work saved and a -local record that they were meant to be sent. Older chats without that record, -pre-pairing chats, branch copies, archived chats and chats currently open here -are left out of this check. Their absence from the report does not prove they -have reached another computer. - -If the directory cannot be read, doctor says -`could not check whether chats reached your other computers; check this computer's network and run codeaf doctor again`. -It never reports a chat as missing based on a failed check. - -`codeaf doctor --fix` retries the ordinary send and releases the chat, printing -`fixed: sent chat to your other computers` for each successful repair. -Running it again leaves repaired chats alone. A chat already present at the fresh -check produces no missing-chat line. These sends run after the other repair -passes, each with its own send limit. Doctor never takes another computer's live -hold or overwrites unfamiliar history. - -Chats put away before repair starts stay excluded. If you put a chat away during -its send, doctor also puts away the newly created record on your other computers. -If that request fails, it says `could not put away chat on your other computers · chat `; -put the chat away on the other computer when the connection returns. -A computer that has never paired gets no new check or requests. - -Saving the local recovery record is best effort and silent. The chat still -attempts its ordinary send if recording fails. - -## I deleted my codeaf home while the engine was stopping - -A running process remembers its original home directory. If you remove it or -replace it with a different directory, attempts to make an identity or device -refuse with `codeaf home was removed or replaced; restart codeaf to use the current folder`. -Late saving work does not make a new identity there. If the engine is already -stopping and still closing chats, it exits promptly when its original home is -removed. With the home intact, stopping keeps its usual saving order and limits. -A new process can still create a home on first use. - ## Repair a busy engine store or sync state — codeaf doctor --fix -When an engine store lock or cross-computer sync issue exists on this computer, `codeaf doctor` adds `engine store` and `sync` rows naming a busy store holder (`busy · held by , pid `), an `unfinished continue · chat `, `duplicate rows · chat `, a `lapsed hold on this computer · chat `, or a chat that has not reached your other computers. When a busy holder cannot be found in the brief lookup, the store row says only `busy`. Healthy stores and clean sync state print nothing. +When an engine store lock or cross-computer sync issue exists on this computer, `codeaf doctor` adds `engine store` and `sync` rows naming a busy store holder (`busy · held by , pid `), an `unfinished continue · chat `, `duplicate rows · chat `, or a `lapsed hold on this computer · chat `. Healthy stores and clean sync state print nothing. -Without `--fix`, `codeaf doctor` changes nothing. `codeaf doctor --fix` releases unfinished continues started by this computer, retires duplicate branch copies whose turns are all already in the main chat, and sends eligible chats missing from the directory, printing one `fixed:` line for each repair. A store lock whose process has ended is already free, so doctor neither reports nor removes it, and a busy store waits for its holder to finish or quit. It never touches another computer's live hold and never deletes a turn that exists only in the copy or scratch folder it inspects. +Without `--fix`, `codeaf doctor` changes nothing. `codeaf doctor --fix` releases unfinished continues started by this computer, and retires duplicate branch copies whose turns are all already in the main chat, printing one `fixed:` line for each repair. A store lock whose process has ended is already free, so doctor neither reports nor removes it, and a busy store waits for its holder to finish or quit. It never touches another computer's live hold and never deletes a turn that exists only in the copy or scratch folder it inspects. ## codeaf help, and the verbs that take no flags at all diff --git a/internal/manual/chat/starting-codeaf.md b/internal/manual/chat/starting-codeaf.md index 4991688c80..e513e6970a 100644 --- a/internal/manual/chat/starting-codeaf.md +++ b/internal/manual/chat/starting-codeaf.md @@ -129,7 +129,7 @@ stale is running. | `--reasoning ` | how hard the model is asked to think: `off`, `low`, `medium` or `high` | | `--session ` | open a particular conversation file instead of the most recent | | `--host ` | run the conversation on another machine over ssh | -| `--once ""` | send one message and print its replies — when paired, finish the bounded final send to your other computers before exiting; with `--yolo` and a budget it stays until handed-over work is home or the limit ends it | +| `--once ""` | send one message and print its replies — normally it then exits; with `--yolo` and a budget it stays until handed-over work is home or the limit ends it | | `--no-compact` | never shorten the conversation automatically | | `--yolo` | run every tool without asking, subject to the limits that nothing lifts | | `--max-hours ` | with `--yolo`: elapsed-time limit; interactive chat checks before new turns, and the window closes itself two minutes after it | diff --git a/internal/manual/chat/tasks.md b/internal/manual/chat/tasks.md index 05b2a06599..565bce0143 100644 --- a/internal/manual/chat/tasks.md +++ b/internal/manual/chat/tasks.md @@ -1373,7 +1373,7 @@ the sentence about it without typing anything. So you can start something, close lid on the conversation, and come back to the answer rather than to a card and silence. On a headless `--once --yolo` run with a budget, the command stays alive for those replies until no work is moving and no reply is in flight; without a budget, `--once` still exits -after its one reply and, when paired, its bounded final send to your other computers. +after its one reply. `jobs list` shows what is still running, and `jobs output ` shows what it has said so far. **A watch is two kinds of news and only one of them wakes you.** Its ordinary updates — the From d3b0468f8eb100583eb0bd76ddcfb05d03580eae Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Sat, 31 Oct 2026 00:35:19 -0400 Subject: [PATCH 14/18] Revert "A chat made with chat --once always reaches your other computers" This reverts commit f9bb2e77b8c12989f16fb8610c5341b24f0c2c03. --- cmd/codeaf/chatv3.go | 17 +- cmd/codeaf/doctor_publication.go | 145 ------- cmd/codeaf/doctor_repair.go | 6 - cmd/codeaf/once_publication_test.go | 393 ------------------ cmd/codeaf/publication_manual_test.go | 61 --- cmd/codeaf/publication_round_test.go | 402 ------------------- cmd/codeaf/syncdrive.go | 17 +- internal/cellstore/first_turn_test.go | 37 -- internal/cellstore/log.go | 26 -- internal/cellsync/create_only_test.go | 35 -- internal/cellsync/publish.go | 5 +- internal/syncsetup/drive.go | 34 +- internal/syncsetup/publication.go | 105 ----- internal/syncsetup/publication_round_test.go | 151 ------- 14 files changed, 16 insertions(+), 1418 deletions(-) delete mode 100644 cmd/codeaf/doctor_publication.go delete mode 100644 cmd/codeaf/once_publication_test.go delete mode 100644 cmd/codeaf/publication_manual_test.go delete mode 100644 cmd/codeaf/publication_round_test.go delete mode 100644 internal/cellstore/first_turn_test.go delete mode 100644 internal/cellsync/create_only_test.go delete mode 100644 internal/syncsetup/publication.go delete mode 100644 internal/syncsetup/publication_round_test.go diff --git a/cmd/codeaf/chatv3.go b/cmd/codeaf/chatv3.go index 80ed00e8ee..073899addd 100644 --- a/cmd/codeaf/chatv3.go +++ b/cmd/codeaf/chatv3.go @@ -2754,9 +2754,9 @@ func warmV3Models(ctx context.Context, models *catalog.Catalog, agent *session.A func runChatV3Once(ctx context.Context, cfg session.Config, workspace, text, level string, resumed bool) error { // The leaving road stands before session opening because opening can take // time, and a signal there would otherwise take the default disposition and - // skip every defer — the whole of #471. Cancelling the turn stops new work; - // deferred closes settle the tasks, checkpoint and paired publication. - // Defers run last-in-first-out, so Close runs before this + // skip every defer — the whole of #471. Cancelling the turn is all the + // leaving work needed here: the deferred agent.Close below settles the tasks + // and checkpoint. Defers run last-in-first-out, so Close runs before this // road stands down and keeps the second signal live through a close that can // take the turn's grace plus two job rounds. ctx, cancel := context.WithCancel(ctx) @@ -2776,17 +2776,6 @@ func runChatV3Once(ctx context.Context, cfg session.Config, workspace, text, lev cancel() }, nil) defer stopLeaving() - // Seating or pairing during the turn may start a drive. Register its close - // before opening so the agent's last seal precedes the final publish. - defer func() { - said := false - syncDrives.closeAllWith(func(_ *liveDrive, _ error) { - if !said { - fmt.Fprintln(os.Stderr, "this chat could not finish syncing with your other computers; run codeaf doctor --fix or reopen this chat to try again.") - said = true - } - }) - }() if resumed && cfg.SessionFile != "" { fmt.Fprintln(os.Stderr, "resumed "+cfg.SessionFile) diff --git a/cmd/codeaf/doctor_publication.go b/cmd/codeaf/doctor_publication.go deleted file mode 100644 index ba919f0016..0000000000 --- a/cmd/codeaf/doctor_publication.go +++ /dev/null @@ -1,145 +0,0 @@ -package main - -import ( - "context" - "errors" - "fmt" - "maps" - "os" - "path/filepath" - "slices" - - "github.com/Agent-Field/codeaf/internal/cell" - "github.com/Agent-Field/codeaf/internal/cellstore" - "github.com/Agent-Field/codeaf/internal/directory" - "github.com/Agent-Field/codeaf/internal/filelock" - "github.com/Agent-Field/codeaf/internal/session" - "github.com/Agent-Field/codeaf/internal/syncsetup" -) - -var ( - errDoctorPublicationPresent = errors.New("this chat already reached your other computers") - errDoctorPublicationUnavailable = errors.New("this chat is no longer available to send") - errDoctorPublicationArchive = errors.New("could not put away the chat on your other computers") -) - -// doctorUnlistedChats compares only chats with this paired writer's durable -// intent against a successful listing. Absence of a receipt proves nothing. -func doctorUnlistedChats(s *syncsetup.Sync, inv localChatInventory, listing directory.Listing, fix bool) (issues, fixes []string) { - for _, id := range slices.Sorted(maps.Keys(inv.foldersByID)) { - if _, listed := listing.Cells[id]; listed || inv.inUseByID[id] || len(inv.foldersByID[id]) != 1 { - continue - } - dir := inv.foldersByID[id][0] - c, err := cell.OpenAt(dir, id) - if err != nil || !s.PublicationExpected(c) { - continue - } - meta, err := session.LoadMeta(dir) - if err != nil || !meta.Owned || meta.Archived { - continue - } - head, err := cellstore.Head(c) - if err != nil || head == nil { - continue - } - if spoken, _ := session.ConversationIn(session.Place{Dir: dir}.Transcript(), head.Receipt.Transcript.End); !spoken { - continue - } - if fix { - err := repairDoctorPublication(s, c, meta) - switch { - case err == nil: - fixes = append(fixes, fmt.Sprintf("fixed: sent chat %s to your other computers", id)) - continue - case errors.Is(err, errDoctorPublicationPresent), errors.Is(err, errDoctorPublicationUnavailable): - continue - case errors.Is(err, errDoctorPublicationArchive): - issues = append(issues, fmt.Sprintf("could not put away chat on your other computers · chat %s", id)) - continue - } - } - issues = append(issues, fmt.Sprintf("chat has not reached your other computers · chat %s", id)) - } - return issues, fixes -} - -func repairDoctorPublication(s *syncsetup.Sync, c cell.Cell, meta session.Meta) error { - // Hold the same transcript lock as an open conversation so repair cannot - // race a window starting a new turn on this computer. - f, err := os.OpenFile(session.Place{Dir: c.Root}.Transcript(), os.O_RDWR, 0) - if err != nil { - return err - } - defer f.Close() - if err := filelock.Lock(f, true, true); err != nil { - return err - } - defer filelock.Unlock(f) - current, err := session.LoadMeta(c.Root) - if err != nil { - return err - } - if current.Archived || !current.Owned { - return errDoctorPublicationUnavailable - } - meta = current - ctx, cancel := context.WithTimeout(context.Background(), driveCloseWithin) - defer cancel() - // A record created since the listing needs no missing-chat report. - // Repair never acquires an existing record or replaces unfamiliar history. - if _, err := s.Dir.Cell(ctx, c.ID); !errors.Is(err, directory.ErrNotFound) { - if err == nil { - return errDoctorPublicationPresent - } - return err - } - e := cellstore.EngineFor(meta.Workspace) - e.Workspace = filepath.Join(c.Root, "work") - d, err := s.Drive(ctx, e, c, syncsetup.DriveOptions{CreateOnly: true, DeviceName: deviceName(), Title: func() string { return meta.Title }}) - if errors.Is(err, directory.ErrExists) { - return errDoctorPublicationPresent - } - if err != nil { - return err - } - closeErr := errors.Join(d.Gate(), d.Close(ctx)) - alreadyPresent := errors.Is(closeErr, directory.ErrExists) - // Remote creation is too long for the metadata lock. Recheck after the - // writer stops, then put away only the record this repair just created. - current, err = session.LoadMeta(c.Root) - if err != nil { - return errors.Join(closeErr, err) - } - if current.Archived { - archiveCtx, archiveCancel := context.WithTimeout(context.Background(), driveCloseWithin) - defer archiveCancel() - if closeErr != nil { - // A lost create reply may return as ErrExists on retry. Only a record - // still held by this writer at this exact head can be that completion; - // an unrelated record retains its own history and archive state. - view, err := s.Dir.Cell(archiveCtx, c.ID) - if errors.Is(err, directory.ErrNotFound) { - return errDoctorPublicationUnavailable - } - if err != nil { - return errors.Join(errDoctorPublicationArchive, err) - } - head, err := cellstore.Head(c) - if err != nil || head == nil { - return errors.Join(errDoctorPublicationArchive, err) - } - if view.Cell.Lease.Device != s.Device.ID() || view.Cell.Head != head.Turn.ID { - return errDoctorPublicationUnavailable - } - } - if err := s.Dir.Archive(archiveCtx, c.ID); err != nil && !errors.Is(err, directory.ErrNotFound) { - return errors.Join(errDoctorPublicationArchive, err) - } - return errDoctorPublicationUnavailable - } - if alreadyPresent { - return errDoctorPublicationPresent - } - return closeErr -} diff --git a/cmd/codeaf/doctor_repair.go b/cmd/codeaf/doctor_repair.go index 5b39824945..447deb9019 100644 --- a/cmd/codeaf/doctor_repair.go +++ b/cmd/codeaf/doctor_repair.go @@ -303,15 +303,9 @@ func inspectDoctorSync(fix bool) (issues []string, fixes []string) { // Any local orphan .taking folder for a chat id absent from the directory listing. issues, fixes = finishOrphanStaging(inv.orphanStaging, handledStaging, fix, issues, fixes) - // Publication has its own per-chat bound after the older repair passes. - cancel() - missing, sent := doctorUnlistedChats(s, inv, l, fix) - issues = append(issues, missing...) - fixes = append(fixes, sent...) return issues, fixes } } - issues = append(issues, "could not check whether chats reached your other computers; check this computer's network and run codeaf doctor again") } // Sync is off or unreachable: still report local duplicate Home rows and orphaned .taking folders. diff --git a/cmd/codeaf/once_publication_test.go b/cmd/codeaf/once_publication_test.go deleted file mode 100644 index 264bbbb2c8..0000000000 --- a/cmd/codeaf/once_publication_test.go +++ /dev/null @@ -1,393 +0,0 @@ -package main - -import ( - "context" - "fmt" - "net" - "net/http" - "net/http/httptest" - "os" - "path/filepath" - "slices" - "strings" - "sync" - "sync/atomic" - "testing" - "time" - - "github.com/Agent-Field/codeaf/internal/cell" - "github.com/Agent-Field/codeaf/internal/cellstore" - "github.com/Agent-Field/codeaf/internal/cellsync" - "github.com/Agent-Field/codeaf/internal/directory" - "github.com/Agent-Field/codeaf/internal/home" - "github.com/Agent-Field/codeaf/internal/identity" - "github.com/Agent-Field/codeaf/internal/session" - "github.com/Agent-Field/codeaf/internal/syncsetup" -) - -// publicationChat uses the repository's scripted model endpoint. Like the -// engine fixture, it is optional on machines without the staged test program. -func publicationChat(t *testing.T) (session.Config, cell.Cell) { - t.Helper() - stub := os.Getenv("CODEAF_TEST_SCRIPTED_MODEL_URL") - if stub == "" { - t.Skip("stage test/remote/stub and set CODEAF_TEST_SCRIPTED_MODEL_URL") - } - cfg, _ := doorChat(t) - c, err := cell.Create(home.Dir(), cell.Options{Class: cell.HostBound}) - if err != nil { - t.Fatal(err) - } - work := filepath.Join(c.Root, "work") - if err := os.MkdirAll(work, 0o700); err != nil { - t.Fatal(err) - } - cfg.Place = session.Place{Dir: c.Root, Workspace: work, Owned: true} - cfg.Workspace, cfg.SessionFile = work, cfg.Place.Transcript() - cfg.Model, cfg.APIKey, cfg.BaseURL = "stub/scripted", "stub", stub - return cfg, c -} - -func publicationRelay(t *testing.T, handler http.Handler) *httptest.Server { - t.Helper() - for port := 8900; port <= 8999; port++ { - listener, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", port)) - if err != nil { - continue - } - srv := httptest.NewUnstartedServer(handler) - srv.Listener.Close() - srv.Listener = listener - srv.Start() - t.Cleanup(srv.Close) - return srv - } - t.Fatal("no local relay port available in 8900-8999") - return nil -} - -func publicationPair(t *testing.T, relay string) *syncsetup.Sync { - t.Helper() - id, err := identity.Ensure(home.Dir()) - if err != nil { - t.Fatal(err) - } - bHome := t.TempDir() - if err := identity.Save(bHome, id); err != nil { - t.Fatal(err) - } - t.Setenv(syncsetup.URLVar, relay) - t.Setenv(syncsetup.IntervalVar, "50") - b, ok, err := syncsetup.Open(bHome) - if err != nil || !ok { - t.Fatalf("open paired computer: %v, %v", ok, err) - } - return b -} - -func capturePublicationOutput(t *testing.T) (stdout, stderr *os.File) { - t.Helper() - oldOut, oldErr := os.Stdout, os.Stderr - var err error - stdout, err = os.CreateTemp(t.TempDir(), "stdout") - if err != nil { - t.Fatal(err) - } - stderr, err = os.CreateTemp(t.TempDir(), "stderr") - if err != nil { - t.Fatal(err) - } - os.Stdout, os.Stderr = stdout, stderr - t.Cleanup(func() { os.Stdout, os.Stderr = oldOut, oldErr; stdout.Close(); stderr.Close() }) - return -} - -// C1: the real one-message road waits through first registration and releases -// its hold before the other computer lists the chat, without cleanup helping it. -func TestChatOnceWaitsForFirstRegistration(t *testing.T) { - const wantReply = "STUB-REPLY: PROBE-ECHO publish\n" - cfg, c := publicationChat(t) - entered, release := make(chan struct{}), make(chan struct{}) - var enterOnce, releaseOnce sync.Once - unblock := func() { releaseOnce.Do(func() { close(release) }) } - next := relayserveForMove(t) - srv := publicationRelay(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodPost && r.URL.Path == "/v1/dir/cells/"+c.ID { - enterOnce.Do(func() { close(entered) }) - select { - case <-release: - case <-r.Context().Done(): - return - } - } - next.ServeHTTP(w, r) - })) - b := publicationPair(t, srv.URL) - stdout, _ := capturePublicationOutput(t) - done := make(chan error, 1) - go func() { - done <- runChatV3Once(t.Context(), cfg, cfg.Workspace, "PROBE-ECHO publish", "", false) - close(done) - }() - t.Cleanup(func() { - // A failing test must cancel its writer rather than publish during - // cleanup and accidentally make the missing registration disappear. - if t.Failed() { - ctx, cancel := context.WithCancel(context.Background()) - cancel() - for _, d := range syncDrives.take() { - _ = d.Close(ctx) - } - } - unblock() - select { - case <-done: - case <-time.After(driveCloseWithin): - t.Error("one-message test did not stop") - } - }) - select { - case <-entered: - case err := <-done: - t.Fatalf("returned before registration: %v", err) - case <-time.After(cellstore.SealWithin): - t.Fatal("no registration request") - } - // The answer itself must be visible before testing the closing wait. - // A slow model must not make a missing final publish look correct. - answeredBy := time.After(cellstore.SealWithin) - for { - out, err := os.ReadFile(stdout.Name()) - if err != nil { - t.Fatal(err) - } - if string(out) == wantReply { - break - } - select { - case err := <-done: - t.Fatalf("returned before registration finished: %v", err) - case <-answeredBy: - t.Fatal("the one-message answer did not arrive") - case <-time.After(10 * time.Millisecond): - } - } - select { - case err := <-done: - t.Fatalf("returned while registration was held: %v", err) - case <-time.After(200 * time.Millisecond): - } - unblock() - select { - case err := <-done: - if err != nil { - t.Fatal(err) - } - case <-time.After(driveCloseWithin): - t.Fatal("one-message close did not finish") - } - listing, err := b.Dir.List(t.Context()) - if err != nil { - t.Fatal(err) - } - rec, exists := listing.Cells[c.ID] - if !exists || rec.Head == "" || rec.Lease.Expires > listing.Now { - t.Fatalf("other computer cannot take completed chat: %+v", listing) - } - out, err := os.ReadFile(stdout.Name()) - if err != nil || string(out) != wantReply { - t.Fatalf("reply = %q: %v", out, err) - } -} - -// C2/C5: a never-paired one-message chat and doctor retain local behavior, -// produce no paired publication records, and make no relay requests. -func TestChatOnceNeverPairedRemainsLocal(t *testing.T) { - cfg, _ := publicationChat(t) - var requests atomic.Int32 - srv := publicationRelay(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { requests.Add(1); w.WriteHeader(503) })) - t.Setenv(syncsetup.URLVar, srv.URL) - stdout, stderr := capturePublicationOutput(t) - if err := runChatV3Once(t.Context(), cfg, cfg.Workspace, "PROBE-ECHO local", "", false); err != nil { - t.Fatal(err) - } - out, _ := os.ReadFile(stdout.Name()) - if string(out) != "STUB-REPLY: PROBE-ECHO local\n" { - t.Fatalf("reply = %q", out) - } - errout, _ := os.ReadFile(stderr.Name()) - if strings.Contains(string(errout), "other computers") { - t.Fatalf("unexpected paired warning: %s", errout) - } - issues, fixes := inspectDoctorSync(true) - if len(issues) != 0 || len(fixes) != 0 { - t.Fatalf("doctor: %v, %v", issues, fixes) - } - if requests.Load() != 0 { - t.Fatalf("relay requests = %d", requests.Load()) - } - if _, err := os.Stat(filepath.Join(home.Dir(), "v3", "sync", "publication")); !os.IsNotExist(err) { - t.Fatalf("unpaired publication record: %v", err) - } -} - -// C1/C3/C4: an upload whose first registration failed is reported and repaired -// by doctor through the normal publisher; unavailable listings never prove absence. -func TestDoctorFixRegistersUnlistedChat(t *testing.T) { - cfg, c := publicationChat(t) - var dropCreate, dropList atomic.Bool - dropCreate.Store(true) - var uploads, failedListings atomic.Int32 - next := relayserveForMove(t) - srv := publicationRelay(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodPost && r.URL.Path == "/v1/store/frames" { - uploads.Add(1) - } - if dropList.Load() && r.Method == http.MethodGet && r.URL.Path == "/v1/dir/list" { - failedListings.Add(1) - } - if (dropCreate.Load() && r.Method == http.MethodPost && r.URL.Path == "/v1/dir/cells/"+c.ID) || (dropList.Load() && r.Method == http.MethodGet && r.URL.Path == "/v1/dir/list") { - w.WriteHeader(http.StatusServiceUnavailable) - return - } - next.ServeHTTP(w, r) - })) - b := publicationPair(t, srv.URL) - _, stderr := capturePublicationOutput(t) - if err := runChatV3Once(t.Context(), cfg, cfg.Workspace, "PROBE-ECHO stranded", "", false); err != nil { - t.Fatalf("publish failure changed turn status: %v", err) - } - if uploads.Load() == 0 { - t.Fatal("fixture never uploaded") - } - errout, _ := os.ReadFile(stderr.Name()) - if !strings.Contains(string(errout), "this chat could not finish syncing with your other computers; run codeaf doctor --fix or reopen this chat to try again.") { - t.Fatalf("missing final publish warning: %s", errout) - } - missing := "chat has not reached your other computers · chat " + c.ID - // Another sealed local chat has no paired intent of its own. The first - // chat's upload ledger cannot make this pre-pairing history a repair target. - local, err := cell.Create(home.Dir(), cell.Options{Class: cell.HostBound}) - if err != nil { - t.Fatal(err) - } - localWork := filepath.Join(local.Root, "work") - if err := os.MkdirAll(localWork, 0o700); err != nil { - t.Fatal(err) - } - localCfg := cfg - localCfg.Place = session.Place{Dir: local.Root, Workspace: localWork, Owned: true} - localCfg.Workspace, localCfg.SessionFile = localWork, localCfg.Place.Transcript() - localAgent, err := session.New(localCfg) - if err != nil { - t.Fatal(err) - } - defer localAgent.Close() - events, err := localAgent.SubmitBash(t.Context(), "! printf local-only") - if err != nil { - localAgent.Close() - t.Fatal(err) - } - for event := range events { - if event.Kind == session.EventError { - t.Fatal(event.Text) - } - } - if err := localAgent.Close(); err != nil { - t.Fatal(err) - } - engine := cellstore.EngineFor(localWork) - engine.Workspace = localWork - if _, err := engine.Seal(t.Context(), local, cellstore.TurnInfo{}); err != nil { - t.Fatal(err) - } - if err := cellstore.Await(t.Context(), local); err != nil { - t.Fatal(err) - } - issues, fixes := inspectDoctorSync(false) - if !slices.Contains(issues, missing) || len(fixes) != 0 { - t.Fatalf("doctor: %v, %v", issues, fixes) - } - if strings.Contains(strings.Join(issues, "\n"), local.ID) { - t.Fatalf("unrelated upload made local chat a repair target: %v", issues) - } - dropList.Store(true) - issues, fixes = inspectDoctorSync(true) - if failedListings.Load() == 0 { - t.Fatal("fixture did not fail the listing") - } - if slices.Contains(issues, missing) || len(fixes) != 0 || !strings.Contains(strings.Join(issues, "\n"), "could not check whether chats reached your other computers") { - t.Fatalf("failed listing claimed absence: %v, %v", issues, fixes) - } - dropList.Store(false) - // Local archives and branch copies cannot become missing-chat repairs. - if err := session.SetArchived(c.Root, true); err != nil { - t.Fatal(err) - } - issues, fixes = inspectDoctorSync(true) - if slices.Contains(issues, missing) || len(fixes) != 0 { - t.Fatalf("archived chat was offered: %v, %v", issues, fixes) - } - if err := session.SetArchived(c.Root, false); err != nil { - t.Fatal(err) - } - branches, err := cellsync.OpenBranchMap(home.Dir()) - if err != nil { - t.Fatal(err) - } - if err := branches.Set(c.ID, "01J00000000000000000000999"); err != nil { - t.Fatal(err) - } - issues, fixes = inspectDoctorSync(true) - if slices.Contains(issues, missing) || len(fixes) != 0 { - t.Fatalf("branch copy was offered: %v, %v", issues, fixes) - } - if err := branches.Set(c.ID, c.ID); err != nil { - t.Fatal(err) - } - dropCreate.Store(false) - issues, fixes = inspectDoctorSync(true) - wantFix := "fixed: sent chat " + c.ID + " to your other computers" - if !slices.Contains(fixes, wantFix) || slices.Contains(issues, missing) { - t.Fatalf("repair: %v, %v", issues, fixes) - } - listing, err := b.Dir.List(t.Context()) - if err != nil { - t.Fatal(err) - } - if rec, ok := listing.Cells[c.ID]; !ok || rec.Head == "" || rec.Lease.Expires > listing.Now { - t.Fatalf("repair not visible or still held: %+v", listing) - } - issues, fixes = inspectDoctorSync(true) - if len(fixes) != 0 || slices.Contains(issues, missing) { - t.Fatalf("second repair changed state: %v, %v", issues, fixes) - } - // An existing hold and unfamiliar history are never acquired by this repair. - view, err := b.Dir.Acquire(t.Context(), c.ID, directory.AcquireOpts{}) - if err != nil { - t.Fatal(err) - } - view, err = b.Dir.Publish(t.Context(), c.ID, directory.Publish{Fence: view.Cell.Lease.Fence, OldHead: view.Cell.Head, Head: strings.Repeat("f", 64), Class: "chat"}) - if err != nil { - t.Fatal(err) - } - s, _, err := syncsetup.Open(home.Dir()) - if err != nil { - t.Fatal(err) - } - meta, _ := session.LoadMeta(c.Root) - if err := repairDoctorPublication(s, c, meta); err == nil { - t.Fatal("repair accepted another computer's record") - } - after, err := b.Dir.Cell(t.Context(), c.ID) - if err != nil || after.Cell.Lease != view.Cell.Lease || after.Cell.Head != view.Cell.Head { - t.Fatalf("repair changed live hold: %+v, %v", after, err) - } - if err := b.Dir.Archive(context.Background(), c.ID); err != nil { - t.Fatal(err) - } - issues, fixes = inspectDoctorSync(true) - if len(fixes) != 0 || slices.Contains(issues, missing) { - t.Fatalf("remote archive was offered: %v, %v", issues, fixes) - } -} diff --git a/cmd/codeaf/publication_manual_test.go b/cmd/codeaf/publication_manual_test.go deleted file mode 100644 index 28e9aaec1d..0000000000 --- a/cmd/codeaf/publication_manual_test.go +++ /dev/null @@ -1,61 +0,0 @@ -package main - -import ( - "fmt" - "net/http" - "os" - "path/filepath" - "strings" - "sync/atomic" - "testing" - - "github.com/Agent-Field/codeaf/internal/home" - "github.com/Agent-Field/codeaf/internal/identity" - "github.com/Agent-Field/codeaf/internal/syncsetup" -) - -// C1: the manual quotes the closing limit from the same bound as both chat doors. -func TestOncePublicationManualMatchesCloseBudget(t *testing.T) { - raw, err := os.ReadFile(filepath.Join("..", "..", "internal", "manual", "chat", "running-from-the-terminal.md")) - if err != nil { - t.Fatal(err) - } - text := strings.Join(strings.Fields(string(raw)), " ") - if want := fmt.Sprintf("up to %.0f seconds for the final send and release", driveCloseWithin.Seconds()); !strings.Contains(text, want) { - t.Fatalf("manual must say %q", want) - } -} - -// C5: doctor has no sync output, writes or requests before pairing, even with --fix. -func TestDoctorNeverPairedMakesNoRelayRequests(t *testing.T) { - root := t.TempDir() - t.Setenv(home.EnvVar, root) - if _, err := identity.EnsureSolo(root); err != nil { - t.Fatal(err) - } - var requests atomic.Int32 - srv := publicationRelay(t, http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { requests.Add(1); w.WriteHeader(503) })) - t.Setenv(syncsetup.URLVar, srv.URL) - before, err := os.ReadDir(root) - if err != nil { - t.Fatal(err) - } - for _, fix := range []bool{false, true} { - issues, fixes := inspectDoctorSync(fix) - if len(issues) != 0 || len(fixes) != 0 { - t.Fatalf("doctor: %v, %v", issues, fixes) - } - } - after, err := os.ReadDir(root) - if err != nil || len(after) != len(before) { - t.Fatalf("doctor changed home: %v, %v", after, err) - } - for i := range before { - if after[i].Name() != before[i].Name() { - t.Fatal("doctor changed home entries") - } - } - if requests.Load() != 0 { - t.Fatalf("relay requests = %d", requests.Load()) - } -} diff --git a/cmd/codeaf/publication_round_test.go b/cmd/codeaf/publication_round_test.go deleted file mode 100644 index 9490138f88..0000000000 --- a/cmd/codeaf/publication_round_test.go +++ /dev/null @@ -1,402 +0,0 @@ -package main - -import ( - "bytes" - "context" - "io" - "log" - "net/http" - "net/http/httptest" - "net/http/httputil" - "net/url" - "os" - "path/filepath" - "strings" - "sync" - "sync/atomic" - "testing" - "time" - - "github.com/Agent-Field/codeaf/internal/cell" - "github.com/Agent-Field/codeaf/internal/cellstore" - "github.com/Agent-Field/codeaf/internal/directory" - "github.com/Agent-Field/codeaf/internal/home" - "github.com/Agent-Field/codeaf/internal/identity" - "github.com/Agent-Field/codeaf/internal/session" - "github.com/Agent-Field/codeaf/internal/syncsetup" -) - -func unlistedRepairChat(t *testing.T, wrap func(http.Handler) http.Handler) (*syncsetup.Sync, cell.Cell, localChatInventory) { - t.Helper() - cfg, _ := doorChat(t) - c, err := cell.Create(home.Dir(), cell.Options{Class: cell.HostBound}) - if err != nil { - t.Fatal(err) - } - work := filepath.Join(c.Root, "work") - if err := os.MkdirAll(work, 0700); err != nil { - t.Fatal(err) - } - cfg.Place = session.Place{Dir: c.Root, Workspace: work, Owned: true} - cfg.Workspace, cfg.SessionFile = work, cfg.Place.Transcript() - cfg.Model, cfg.APIKey, cfg.BaseURL = "stub/chat", "stub", "http://127.0.0.1:1" - var reject atomic.Bool - reject.Store(true) - handler := relayserveForMove(t) - if wrap != nil { - handler = wrap(handler) - } - srv := publicationRelay(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if reject.Load() && r.Method == http.MethodPost && r.URL.Path == "/v1/dir/cells/"+c.ID { - w.WriteHeader(503) - return - } - handler.ServeHTTP(w, r) - })) - publicationPair(t, srv.URL) - s, ok, err := syncsetup.Open(home.Dir()) - if err != nil || !ok { - t.Fatalf("open sync: %v %v", ok, err) - } - a, err := session.New(cfg) - if err != nil { - t.Fatal(err) - } - events, err := a.SubmitBash(t.Context(), "! printf repair-fixture") - if err != nil { - a.Close() - t.Fatal(err) - } - for event := range events { - if event.Kind == session.EventError { - a.Close() - t.Fatal(event.Text) - } - } - if err := a.Close(); err != nil { - t.Fatal(err) - } - e := cellstore.EngineFor(work) - e.Workspace = work - e.Identity = cellstore.Identity{Device: s.Device.Cert.Device} - if _, err := e.Seal(t.Context(), c, cellstore.TurnInfo{}); err != nil { - t.Fatal(err) - } - if err := cellstore.Await(t.Context(), c); err != nil { - t.Fatal(err) - } - d, err := s.Drive(t.Context(), e, c, syncsetup.DriveOptions{}) - if err != nil { - t.Fatal(err) - } - ctx, cancel := context.WithCancel(context.Background()) - cancel() - _ = d.Close(ctx) - if !s.PublicationExpected(c) { - t.Fatal("fixture has no paired publication intent") - } - reject.Store(false) - return s, c, localChatInventory{foldersByID: map[string][]string{c.ID: {c.Root}}, inUseByID: map[string]bool{}} -} - -// SPEC-3 item 3: an archive during remote creation also puts away the new record. -func TestFixRound3DoctorArchivesChatPutAwayDuringRepair(t *testing.T) { - for _, lostReply := range []bool{false, true} { - t.Run(map[bool]string{false: "acknowledged", true: "lost-reply"}[lostReply], func(t *testing.T) { - entered, release := make(chan struct{}), make(chan struct{}) - var once sync.Once - var loseReply atomic.Bool - var repeats atomic.Int32 - loseReply.Store(lostReply) - s, c, inv := unlistedRepairChat(t, func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodPost && strings.HasPrefix(r.URL.Path, "/v1/dir/cells/") && !strings.Contains(strings.TrimPrefix(r.URL.Path, "/v1/dir/cells/"), "/") { - once.Do(func() { close(entered) }) - select { - case <-release: - case <-r.Context().Done(): - return - } - if loseReply.Swap(false) { - // Discard the committed reply and replay the identical request before - // returning. This forces ErrExists without depending on which of the - // background publisher and Close made the first network attempt. - raw, err := io.ReadAll(r.Body) - if err != nil { - t.Error(err) - w.WriteHeader(503) - return - } - r.Body = io.NopCloser(bytes.NewReader(raw)) - saved := httptest.NewRecorder() - next.ServeHTTP(saved, r) - if saved.Code != http.StatusOK && saved.Code != http.StatusCreated { - t.Errorf("lost-reply fixture did not create: %d %s", saved.Code, saved.Body.String()) - } - r.Body = io.NopCloser(bytes.NewReader(raw)) - repeated := httptest.NewRecorder() - next.ServeHTTP(repeated, r) - if repeated.Code != http.StatusConflict { - t.Errorf("repeated create did not return exists: %d %s", repeated.Code, repeated.Body.String()) - } - repeats.Add(1) - for key, values := range repeated.Header() { - w.Header()[key] = values - } - w.WriteHeader(repeated.Code) - _, _ = w.Write(repeated.Body.Bytes()) - return - } - } - next.ServeHTTP(w, r) - }) - }) - var releaseOnce sync.Once - unblock := func() { releaseOnce.Do(func() { close(release) }) } - defer unblock() - type result struct{ issues, fixes []string } - done := make(chan result, 1) - go func() { - issues, fixes := doctorUnlistedChats(s, inv, directory.Listing{}, true) - done <- result{issues, fixes} - }() - select { - case <-entered: - case <-time.After(driveCloseWithin): - t.Fatal("repair never created record") - } - if err := session.SetArchived(c.Root, true); err != nil { - t.Fatal(err) - } - unblock() - select { - case got := <-done: - if len(got.issues) != 0 || len(got.fixes) != 0 { - t.Fatalf("archived repair: %+v", got) - } - case <-time.After(driveCloseWithin): - t.Fatal("repair did not finish") - } - if lostReply && repeats.Load() != 1 { - t.Fatal("lost-reply fixture never returned a repeated-create conflict") - } - view, err := s.Dir.Cell(t.Context(), c.ID) - if err != nil || !view.Cell.Archived { - t.Fatalf("remote chat revived: %+v %v", view, err) - } - }) - } -} - -// SPEC-3 item 8: a fresh lookup finding a record must not print the missing sentence. -func TestFixRound8DoctorAlreadyPresentIsSilent(t *testing.T) { - s, c, inv := unlistedRepairChat(t, nil) - created, err := s.Dir.Create(t.Context(), c.ID, directory.CellInit{Head: strings.Repeat("f", 64), Class: "chat"}) - if err != nil { - t.Fatal(err) - } - before := created.Cell - issues, fixes := doctorUnlistedChats(s, inv, directory.Listing{}, true) - if len(issues) != 0 || len(fixes) != 0 { - t.Fatalf("already present was reported missing: %v %v", issues, fixes) - } - after, err := s.Dir.Cell(t.Context(), c.ID) - if err != nil || after.Cell.Head != before.Head || after.Cell.Lease != before.Lease { - t.Fatalf("existing record changed: %+v %v", after, err) - } -} - -// SPEC-3 item 7: an empty cleanup has no timer allocations, output or relay work. -func TestFixRound7OnceEmptyCleanupReturnsImmediately(t *testing.T) { - t.Setenv(home.EnvVar, t.TempDir()) - var requests atomic.Int32 - srv := publicationRelay(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { requests.Add(1); w.WriteHeader(503) })) - t.Setenv(syncsetup.URLVar, srv.URL) - out, errout := capturePublicationOutput(t) - b := &driveBook{} - allocs := testing.AllocsPerRun(100, func() { b.closeAllWith(func(*liveDrive, error) { t.Fatal("empty cleanup reported a failure") }) }) - if allocs != 0 { - t.Fatalf("empty cleanup created closing work: %g allocations", allocs) - } - stdout, _ := os.ReadFile(out.Name()) - stderr, _ := os.ReadFile(errout.Name()) - if len(stdout) != 0 || len(stderr) != 0 || requests.Load() != 0 { - t.Fatalf("empty cleanup: stdout=%q stderr=%q requests=%d", stdout, stderr, requests.Load()) - } -} - -// SPEC-3 item 7: a drive starting after entry is drained before --once returns. -func TestFixRound7OnceClosesDriveStartedAfterEntry(t *testing.T) { - cfg, c := publicationChat(t) - next := relayserveForMove(t) - relay := publicationRelay(t, next) - t.Setenv(syncsetup.URLVar, relay.URL) - target, err := url.Parse(cfg.BaseURL) - if err != nil { - t.Fatal(err) - } - target.Path = "" - target.RawPath = "" - proxy := httputil.NewSingleHostReverseProxy(target) - entered, release := make(chan struct{}), make(chan struct{}) - var once, releaseOnce sync.Once - unblock := func() { releaseOnce.Do(func() { close(release) }) } - defer unblock() - model := publicationRelay(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if strings.HasSuffix(r.URL.Path, "/chat/completions") { - once.Do(func() { close(entered) }) - select { - case <-release: - case <-r.Context().Done(): - return - } - } - proxy.ServeHTTP(w, r) - })) - cfg.BaseURL = model.URL + "/api/v1" - capturePublicationOutput(t) - if syncsetup.MayTalk(home.Dir()) { - t.Fatal("fixture paired at entry") - } - done := make(chan error, 1) - go func() { done <- runChatV3Once(t.Context(), cfg, cfg.Workspace, "PROBE-ECHO late-pair", "", false) }() - select { - case <-entered: - case err := <-done: - t.Fatalf("returned before model: %v", err) - case <-time.After(driveCloseWithin): - t.Fatal("no model call") - } - b := publicationPair(t, relay.URL) - if err := identity.EndSolo(home.Dir()); err != nil { - t.Fatal(err) - } - unblock() - select { - case err := <-done: - if err != nil { - t.Fatal(err) - } - case <-time.After(driveCloseWithin): - t.Fatal("late drive did not close") - } - if len(syncDrives.drive) != 0 { - t.Fatal("late drive left for unrelated cleanup") - } - view, err := b.Dir.Cell(t.Context(), c.ID) - if err != nil || view.Cell.Head == "" || view.Cell.Lease.Expires > view.Now { - t.Fatalf("late drive not released: %+v %v", view, err) - } -} - -// SPEC-3 item 6: older repairs finish before a publication send can consume time. -func TestFixRound6DoctorRepairsOlderHoldsBeforePublication(t *testing.T) { - var oldReleased, createdBeforeOld atomic.Bool - var target string - s, c, _ := unlistedRepairChat(t, func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method == http.MethodPost && r.URL.Path == "/v1/dir/cells/"+target && !oldReleased.Load() { - createdBeforeOld.Store(true) - } - next.ServeHTTP(w, r) - if r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/release") { - oldReleased.Store(true) - } - }) - }) - target = c.ID - oldID, err := cell.NewID(time.Now()) - if err != nil { - t.Fatal(err) - } - if _, err := s.Dir.Create(t.Context(), oldID, directory.CellInit{Head: strings.Repeat("e", 64), Class: "chat"}); err != nil { - t.Fatal(err) - } - _, fixes := inspectDoctorSync(true) - if !oldReleased.Load() { - t.Fatalf("older repair did not release its hold: %v", fixes) - } - if createdBeforeOld.Load() { - t.Fatal("publication repair ran before older repairs") - } - if !strings.Contains(strings.Join(fixes, "\n"), "fixed: sent chat "+c.ID) { - t.Fatalf("publication repair did not run: %v", fixes) - } -} - -// SPEC-3 item 9: the real one-message door keeps marker failure off stdout and stderr. -func TestFixRound9OnceMarkerFailureIsSilent(t *testing.T) { - cfg, c := publicationChat(t) - next := relayserveForMove(t) - relay := publicationRelay(t, next) - b := publicationPair(t, relay.URL) - parent := home.Join("v3", "sync", "publication") - if err := os.MkdirAll(filepath.Dir(parent), 0700); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(parent, []byte("blocked"), 0600); err != nil { - t.Fatal(err) - } - out, errout := capturePublicationOutput(t) - oldLog := log.Writer() - log.SetOutput(errout) - defer log.SetOutput(oldLog) - if err := runChatV3Once(t.Context(), cfg, cfg.Workspace, "PROBE-ECHO marker-failure", "", false); err != nil { - t.Fatal(err) - } - stdout, _ := os.ReadFile(out.Name()) - stderr, _ := os.ReadFile(errout.Name()) - if string(stdout) != "STUB-REPLY: PROBE-ECHO marker-failure\n" { - t.Fatalf("reply=%q", stdout) - } - if len(stderr) != 0 { - t.Fatalf("marker failure reached standard error: %s", stderr) - } - view, err := b.Dir.Cell(t.Context(), c.ID) - if err != nil || view.Cell.Head == "" || view.Cell.Lease.Expires > view.Now { - t.Fatalf("marker failure stopped publication: %+v %v", view, err) - } -} - -// publicationCreateRace installs unfamiliar history after the final lookup. -type publicationCreateRace struct { - directory.Client - dir string - archive bool - once sync.Once - injectErr error -} - -func (r *publicationCreateRace) Create(ctx context.Context, id string, in directory.CellInit) (directory.CellView, error) { - r.once.Do(func() { - if r.archive { - r.injectErr = session.SetArchived(r.dir, true) - if r.injectErr != nil { - return - } - } - _, r.injectErr = r.Client.Create(ctx, id, directory.CellInit{Head: strings.Repeat("f", 64), Class: "chat"}) - }) - if r.injectErr != nil { - return directory.CellView{}, r.injectErr - } - return r.Client.Create(ctx, id, in) -} - -// SPEC-3 items 3/8: a create race is silent and an archive cannot touch unfamiliar history. -func TestFixRound8DoctorAlreadyPresentAtCreateIsSilent(t *testing.T) { - for _, archive := range []bool{false, true} { - t.Run(map[bool]string{false: "present", true: "archived-during-create"}[archive], func(t *testing.T) { - s, c, inv := unlistedRepairChat(t, nil) - s.Dir = &publicationCreateRace{Client: s.Dir, dir: c.Root, archive: archive} - issues, fixes := doctorUnlistedChats(s, inv, directory.Listing{}, true) - if len(issues) != 0 || len(fixes) != 0 { - t.Fatalf("already present: %v %v", issues, fixes) - } - view, err := s.Dir.Cell(t.Context(), c.ID) - if err != nil || view.Cell.Head != strings.Repeat("f", 64) || view.Cell.Archived || view.Cell.Lease.Device != s.Device.ID() { - t.Fatalf("unfamiliar record changed: %+v %v", view, err) - } - }) - } -} diff --git a/cmd/codeaf/syncdrive.go b/cmd/codeaf/syncdrive.go index 681bc2923b..9bf6fb9a46 100644 --- a/cmd/codeaf/syncdrive.go +++ b/cmd/codeaf/syncdrive.go @@ -19,7 +19,7 @@ import ( const driveCloseWithin = cellstore.SealWithin // syncDrives is the drive side of every conversation the chat door opened in -// this process. Other command doors, such as do and exec, never sync chats. +// this process. It exists at all only for a chat: a headless run never syncs. var syncDrives = &driveBook{} // driveBook keeps one drive side per cell, so a conversation opened again in the @@ -148,24 +148,13 @@ func chatTitle(c cell.Cell) string { // given back, so another machine can take the chat at once. It runs on the // door's way out, after the conversations closed, so their last seal is in. func (b *driveBook) closeAll() { - b.closeAllWith(func(d *liveDrive, err error) { - log.Print("sync: closing a chat: ", err) - recordFinalSaveFailure(session.SavingError(d.transcript, err)) - }) -} - -// closeAllWith lets the one-message door report a failed publish without -// changing the turn's exit status. Both doors share the same closing budget. -func (b *driveBook) closeAllWith(report func(*liveDrive, error)) { drives := b.take() - if len(drives) == 0 { - return - } ctx, cancel := context.WithTimeout(context.Background(), driveCloseWithin) defer cancel() for _, d := range drives { if err := d.Close(ctx); err != nil { - report(d, err) + log.Print("sync: closing a chat: ", err) + recordFinalSaveFailure(session.SavingError(d.transcript, err)) } } } diff --git a/internal/cellstore/first_turn_test.go b/internal/cellstore/first_turn_test.go deleted file mode 100644 index bbbaface1a..0000000000 --- a/internal/cellstore/first_turn_test.go +++ /dev/null @@ -1,37 +0,0 @@ -package cellstore - -import ( - "encoding/json" - "os" - "path/filepath" - "strings" - "testing" - - "github.com/Agent-Field/codeaf/internal/cell" -) - -// SPEC-3 item 5: reading just the first turn preserves valid large first records. -func TestFixRound5FirstTurnHasNoNewSizeLimit(t *testing.T) { - c, err := cell.Create(t.TempDir(), cell.Options{Class: cell.HostBound}) - if err != nil { - t.Fatal(err) - } - first := Turn{ID: "first", Device: "original", ExcludedPaths: []string{strings.Repeat("long-path", 1<<14)}} - raw, err := json.Marshal(first) - if err != nil { - t.Fatal(err) - } - path := filepath.Join(c.Root, filepath.FromSlash(TurnsPath)) - if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { - t.Fatal(err) - } - for _, tail := range []string{"", "\nnot another turn"} { - if err := os.WriteFile(path, append(append([]byte{}, raw...), tail...), 0600); err != nil { - t.Fatal(err) - } - got, err := FirstTurn(c) - if err != nil || got == nil || got.ID != first.ID || got.Device != first.Device || len(got.ExcludedPaths) != 1 || got.ExcludedPaths[0] != first.ExcludedPaths[0] { - t.Fatalf("large first turn was not read whole: %v", err) - } - } -} diff --git a/internal/cellstore/log.go b/internal/cellstore/log.go index db07eabee7..ee9050092e 100644 --- a/internal/cellstore/log.go +++ b/internal/cellstore/log.go @@ -6,7 +6,6 @@ import ( "encoding/json" "errors" "fmt" - "io" "io/fs" "os" "slices" @@ -49,31 +48,6 @@ func Turns(c cell.Cell) ([]Turn, error) { return turns, sc.Err() } -// FirstTurn reads only the oldest seal when a caller needs its original writer. -// Later history cannot change that writer or add work to this decision. -func FirstTurn(c cell.Cell) (*Turn, error) { - f, err := os.Open(rel(c, TurnsPath)) - if errors.Is(err, fs.ErrNotExist) { - return nil, nil - } - if err != nil { - return nil, err - } - defer f.Close() - raw, err := bufio.NewReader(f).ReadBytes('\n') - if err != nil && !errors.Is(err, io.EOF) { - return nil, err - } - if len(raw) == 0 { - return nil, nil - } - var turn Turn - if err := json.Unmarshal(raw, &turn); err != nil { - return nil, fmt.Errorf("read first turn: %w", err) - } - return &turn, nil -} - // Log is the chain with receipts, newest first. func Log(c cell.Cell) ([]Entry, error) { turns, err := Turns(c) diff --git a/internal/cellsync/create_only_test.go b/internal/cellsync/create_only_test.go deleted file mode 100644 index 64427553a7..0000000000 --- a/internal/cellsync/create_only_test.go +++ /dev/null @@ -1,35 +0,0 @@ -package cellsync - -import ( - "errors" - "testing" - - "github.com/Agent-Field/codeaf/internal/directory" -) - -// C4: doctor repair cannot acquire a record that appeared after its listing, -// even when the other computer has already released an unfamiliar head. -func TestCreateOnlyPublishLeavesExistingHistoryAlone(t *testing.T) { - r := newRig(t) - head := r.seal(map[string]string{"local": "only here"}) - other := r.dir.For(devB) - v, err := other.Create(t.Context(), cellID, directory.CellInit{Head: otherHead, Class: "chat"}) - if err != nil { - t.Fatal(err) - } - if err := other.Release(t.Context(), cellID, v.Cell.Lease.Fence); err != nil { - t.Fatal(err) - } - before, err := other.Cell(t.Context(), cellID) - if err != nil { - t.Fatal(err) - } - r.pub.CreateOnly = true - if err := r.pub.Publish(t.Context(), r.drv, head, r.info()); !errors.Is(err, directory.ErrExists) { - t.Fatalf("publish = %v", err) - } - after, err := other.Cell(t.Context(), cellID) - if err != nil || after.Cell.Head != before.Cell.Head || after.Cell.Lease != before.Cell.Lease { - t.Fatalf("existing history changed: %+v, %v", after, err) - } -} diff --git a/internal/cellsync/publish.go b/internal/cellsync/publish.go index 8662eddef2..a33587db66 100644 --- a/internal/cellsync/publish.go +++ b/internal/cellsync/publish.go @@ -18,9 +18,6 @@ var ErrSuperseded = fmt.Errorf("cellsync: another device continued this chat: %w // Publisher makes turns durable: objects into the store first, the head in the // directory last. Dir is already bound to this device. type Publisher struct { - // CreateOnly refuses a record that appeared during a missing-chat repair. - // Ordinary chat publication still resumes a create whose reply was lost. - CreateOnly bool // WarmFrameHints opts into the bounded existing-frame prototype. WarmFrameHints bool Engine Engine @@ -152,7 +149,7 @@ func (p *Publisher) advance(ctx context.Context, d Driving, head string, in Publ return p.move(ctx, d, head, in, frames) } fence, err := p.create(ctx, d, head, in, frames) - if errors.Is(err, directory.ErrExists) && !p.CreateOnly { + if errors.Is(err, directory.ErrExists) { return p.resume(ctx, d, head, in, frames, err) } return fence, err diff --git a/internal/syncsetup/drive.go b/internal/syncsetup/drive.go index c76dc539aa..71cc7013a6 100644 --- a/internal/syncsetup/drive.go +++ b/internal/syncsetup/drive.go @@ -24,9 +24,6 @@ const unknownDevice = "another device" // DriveOptions is what a chat tells its drive side about itself. type DriveOptions struct { - // CreateOnly is used by repair after a missing directory record. It never - // acquires a record that appeared since the check or replaces its history. - CreateOnly bool // DeviceName is this machine's name, sealed into its directory record. DeviceName string // Title is the chat's title as it is now; nil publishes none. @@ -54,16 +51,12 @@ type Drive struct { directoryID string recordedFence uint64 - cancel context.CancelFunc - done chan struct{} - once sync.Once - closeErr error - publicationDone func() - publicationMu sync.Mutex - publicationChecked bool - publicationEligible bool - publicationSaved bool - ownership chan struct{} + cancel context.CancelFunc + done chan struct{} + once sync.Once + closeErr error + publicationDone func() + ownership chan struct{} sent string // fingerprint of the vault as last sent; touched by the flush loop and Close only @@ -91,7 +84,7 @@ func (s *Sync) Drive(ctx context.Context, eng cellstore.Engine, c cell.Cell, opt return nil, err } d.syncVault(ctx) - drv, held, err := s.driving(ctx, c, opt.CreateOnly) + drv, held, err := s.driving(ctx, c) if err != nil { return nil, err } @@ -167,9 +160,6 @@ func (d *Drive) note(sealed cellstore.Sealed) { spoken := d.spoken d.mu.Unlock() if spoken { - if err := d.expectPublication(); err != nil { - d.sync.logPublicationError(err) - } d.batcher.Note(sealed.Turn) } } @@ -248,16 +238,13 @@ func (s *Sync) Rename(ctx context.Context, name string) error { return s.putDevi // driving finds where this chat stands in the directory: no record yet (fence // 0), a record this device now holds, or a record another device holds, in // which case held names that device. A chat that was branched drives its branch. -func (s *Sync) driving(ctx context.Context, c cell.Cell, createOnly bool) (cellsync.Driving, string, error) { +func (s *Sync) driving(ctx context.Context, c cell.Cell) (cellsync.Driving, string, error) { branches, err := cellsync.OpenBranchMap(s.Home) if err != nil { return cellsync.Driving{}, "", err } drv := cellsync.Driving{Cell: c} if id := branches.Resolve(c.ID); id != c.ID { - if createOnly { - return drv, "", directory.ErrExists - } drv.Remote = id } view, err := s.Dir.Cell(ctx, drv.ID()) @@ -267,9 +254,6 @@ func (s *Sync) driving(ctx context.Context, c cell.Cell, createOnly bool) (cells if err != nil { return drv, "", err } - if createOnly { - return drv, "", directory.ErrExists - } if err := s.reconcileSendLease(c.ID, drv.ID(), view); err != nil { return drv, "", err } @@ -336,7 +320,7 @@ func (s *Sync) batcher(eng cellstore.Engine, c cell.Cell, drv cellsync.Driving, engine := eng.Sync(cellstore.SyncKeys{CellKey: s.Identity.CellKey(), Dedup: s.Identity.DedupSecret()}, s.Ledger) engine.MaxFrame = blobstore.BulkFrame // a flush's objects share frames up to the bulk size (contract §22.6); a small save still writes one small frame d.scope = s.scope(c.ID) - pub := &cellsync.Publisher{Engine: engine, Store: d.scope.Store, Dir: s.Dir, CreateOnly: d.opt.CreateOnly} + pub := &cellsync.Publisher{Engine: engine, Store: d.scope.Store, Dir: s.Dir} return &cellsync.Batcher{ Publisher: pub, Brancher: cellsync.Brancher{ diff --git a/internal/syncsetup/publication.go b/internal/syncsetup/publication.go deleted file mode 100644 index cc3e6d02fc..0000000000 --- a/internal/syncsetup/publication.go +++ /dev/null @@ -1,105 +0,0 @@ -package syncsetup - -import ( - "log" - "os" - "path/filepath" - - "github.com/Agent-Field/codeaf/internal/cell" - "github.com/Agent-Field/codeaf/internal/cellstore" - "github.com/Agent-Field/codeaf/internal/cellsync" - "github.com/Agent-Field/codeaf/internal/config" - "github.com/Agent-Field/codeaf/internal/fsreplace" - "github.com/Agent-Field/codeaf/internal/home" -) - -func (s *Sync) publicationPath(c cell.Cell) string { - return filepath.Join(s.Home, "v3", "sync", "publication", c.ID) -} - -func (s *Sync) publicationScope() string { return s.Ledger + "\n" + s.Device.ID() + "\n" } - -// PublicationExpected identifies chats this computer began publishing while -// paired. Upload counters are shared by chats, so they cannot establish this. -// A receipt from another identity, relay or device never grants repair permission. -func (s *Sync) PublicationExpected(c cell.Cell) bool { - if !s.publicationRecorded(c) { - return false - } - branches, err := cellsync.OpenBranchMap(s.Home) - return err == nil && branches.Resolve(c.ID) == c.ID -} - -func (s *Sync) publicationRecorded(c cell.Cell) bool { - if !cell.ValidID(c.ID) { - return false - } - raw, err := os.ReadFile(s.publicationPath(c)) - return err == nil && string(raw) == s.publicationScope() -} - -// expectPublication decides eligibility once per drive. An ineligible chat never -// reads its growing history again; a failed marker write can still be retried. -func (d *Drive) expectPublication() error { - d.publicationMu.Lock() - defer d.publicationMu.Unlock() - if !d.publicationChecked { - if d.sync.publicationRecorded(d.cell) { - d.publicationChecked, d.publicationSaved = true, true - } else { - first, err := cellstore.FirstTurn(d.cell) - if err != nil { - return err - } - d.publicationEligible = first != nil && first.Device == d.sync.Device.Cert.Device - d.publicationChecked = true - } - } - if d.publicationSaved || !d.publicationEligible { - return nil - } - if err := d.sync.recordPublication(d.cell); err != nil { - return err - } - d.publicationSaved = true - return nil -} - -// recordPublication keeps the paired writer's intent outside sealed chat content. -func (s *Sync) recordPublication(c cell.Cell) error { - path := s.publicationPath(c) - if err := home.MkdirAllUnder(s.Home, filepath.Dir(path), 0o700); err != nil { - return err - } - f, err := os.CreateTemp(filepath.Dir(path), ".publication-*") - if err != nil { - return err - } - defer os.Remove(f.Name()) - _, err = f.WriteString(s.publicationScope()) - if err == nil { - err = f.Sync() - } - closeErr := f.Close() - if err != nil { - return err - } - if closeErr != nil { - return closeErr - } - return fsreplace.Rename(f.Name(), path) -} - -// logPublicationError uses the chat's file log even at the one-message door, -// where the standard logger still points at the person's terminal. -func (s *Sync) logPublicationError(err error) { - if home.Removed(s.Home) { - return - } - f, openErr := os.OpenFile(config.ProfilePath(s.profile(), "chat.log"), os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o644) - if openErr != nil { - return - } - defer f.Close() - log.New(f, log.Prefix(), log.Flags()).Print("sync: recording chat publication: ", err) -} diff --git a/internal/syncsetup/publication_round_test.go b/internal/syncsetup/publication_round_test.go deleted file mode 100644 index daf61427c7..0000000000 --- a/internal/syncsetup/publication_round_test.go +++ /dev/null @@ -1,151 +0,0 @@ -package syncsetup - -import ( - "bytes" - "encoding/json" - "os" - "path/filepath" - "runtime" - "strings" - "testing" - - "github.com/Agent-Field/codeaf/internal/cell" - "github.com/Agent-Field/codeaf/internal/cellstore" - "github.com/Agent-Field/codeaf/internal/cellsync" - "github.com/Agent-Field/codeaf/internal/identity" -) - -// SPEC-3 item 5: eligibility needs only the first turn, then no per-seal reads. -func TestFixRound5PublicationEligibilityReadsFirstTurnOnce(t *testing.T) { - h := t.TempDir() - id, err := identity.Ensure(h) - if err != nil { - t.Fatal(err) - } - dev, err := identity.Device(h) - if err != nil { - t.Fatal(err) - } - s := &Sync{Home: h, Identity: id, Device: dev, Ledger: "local"} - for _, eligible := range []bool{false, true} { - t.Run(map[bool]string{false: "ineligible", true: "eligible"}[eligible], func(t *testing.T) { - c, err := cell.Create(h, cell.Options{Class: cell.HostBound}) - if err != nil { - t.Fatal(err) - } - device := "before pairing" - if eligible { - device = dev.Cert.Device - } - first, _ := json.Marshal(cellstore.Turn{Device: device}) - path := filepath.Join(c.Root, filepath.FromSlash(cellstore.TurnsPath)) - if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { - t.Fatal(err) - } - // Malformed trailing bytes must never be read to decide who made the first seal. - if err := os.WriteFile(path, append(append(first, '\n'), []byte(strings.Repeat("unneeded", 1<<17))...), 0600); err != nil { - t.Fatal(err) - } - d := &Drive{sync: s, cell: c} - if err := d.expectPublication(); err != nil { - t.Fatal(err) - } - if got := s.publicationRecorded(c); got != eligible { - t.Fatalf("publication recorded=%v, want %v", got, eligible) - } - if err := os.Remove(path); err != nil { - t.Fatal(err) - } - if err := d.expectPublication(); err != nil { - t.Fatalf("decision reread chain: %v", err) - } - allocs := testing.AllocsPerRun(100, func() { - if err := d.expectPublication(); err != nil { - panic(err) - } - }) - if allocs != 0 { - t.Fatalf("cached decision allocated %g times", allocs) - } - t.Logf("cached decision: %g allocations per seal; fixture tail=%d bytes", allocs, 8<<17) - }) - } -} - -// SPEC-3 item 5 and PERF.md: repeated decisions do no allocation work as history grows. -func TestFixRound5IneligiblePublicationSealWork(t *testing.T) { - h := t.TempDir() - s := &Sync{Home: h, Ledger: "local"} - c, err := cell.Create(h, cell.Options{Class: cell.HostBound}) - if err != nil { - t.Fatal(err) - } - first, _ := json.Marshal(cellstore.Turn{Device: "before pairing"}) - path := filepath.Join(c.Root, filepath.FromSlash(cellstore.TurnsPath)) - if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(path, bytes.Repeat(append(first, '\n'), 4096), 0600); err != nil { - t.Fatal(err) - } - d := &Drive{sync: s, cell: c} - if err := d.expectPublication(); err != nil { - t.Fatal(err) - } - allocs := testing.AllocsPerRun(100, func() { - if err := d.expectPublication(); err != nil { - panic(err) - } - }) - t.Logf("%s %s/%s: 4096 turns, cached decision allocations=%g", runtime.Version(), runtime.GOOS, runtime.GOARCH, allocs) - if allocs != 0 { - t.Fatalf("history added per-seal allocation work: %g", allocs) - } -} - -// SPEC-3 item 9: a failed recovery record is logged, silent in chat, and still queues the turn. -func TestFixRound9PublicationMarkerFailureIsSilent(t *testing.T) { - h := t.TempDir() - id, err := identity.Ensure(h) - if err != nil { - t.Fatal(err) - } - dev, err := identity.Device(h) - if err != nil { - t.Fatal(err) - } - s := &Sync{Home: h, Identity: id, Device: dev, Ledger: "local"} - c, err := cell.Create(h, cell.Options{Class: cell.HostBound}) - if err != nil { - t.Fatal(err) - } - path := filepath.Join(c.Root, filepath.FromSlash(cellstore.TurnsPath)) - if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { - t.Fatal(err) - } - first, _ := json.Marshal(cellstore.Turn{ID: "first", Device: dev.Cert.Device}) - if err := os.WriteFile(path, append(first, '\n'), 0600); err != nil { - t.Fatal(err) - } - parent := filepath.Dir(s.publicationPath(c)) - if err := os.MkdirAll(filepath.Dir(parent), 0700); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(parent, []byte("blocked"), 0600); err != nil { - t.Fatal(err) - } - var notices []string - batch := &cellsync.Batcher{Driving: &cellsync.Driving{Cell: c}} - d := &Drive{sync: s, cell: c, spoken: true, batcher: batch, opt: DriveOptions{OnNotice: func(line string) { notices = append(notices, line) }}} - d.note(cellstore.Sealed{Turn: cellstore.Turn{ID: "first"}}) - if len(notices) != 0 { - t.Fatalf("marker failure reached chat: %v", notices) - } - logs, err := os.ReadFile(filepath.Join(h, "chat.log")) - if err != nil || !strings.Contains(string(logs), "sync: recording chat publication:") { - t.Fatalf("marker failure not in chat log: %s %v", logs, err) - } - if batch.Pending() != 1 { - t.Fatal("marker failure stopped ordinary publication") - } -} From 2c90d8f0d9f6ffac077c02223f2b2e162ccb00b1 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Sat, 31 Oct 2026 00:35:41 -0400 Subject: [PATCH 15/18] manual: a deleted home is refused, and a stopping engine exits when its home is removed Co-Authored-By: Claude Opus 5.5 --- internal/manual/chat/running-from-the-terminal.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/internal/manual/chat/running-from-the-terminal.md b/internal/manual/chat/running-from-the-terminal.md index 20122d9a56..f23dc933fc 100644 --- a/internal/manual/chat/running-from-the-terminal.md +++ b/internal/manual/chat/running-from-the-terminal.md @@ -1624,6 +1624,16 @@ machine that counted, which is the opposite of the truth. That command is about this *install*. `/status` in the chat is about the conversation you are in. They are related and they are not the same reading. +## I deleted my codeaf home while the engine was stopping + +A running process remembers its original home directory. If you remove it or +replace it with a different directory, attempts to make an identity or device +refuse with `codeaf home was removed or replaced; restart codeaf to use the current folder`. +Late saving work does not make a new identity there. If the engine is already +stopping and still closing chats, it exits promptly when its original home is +removed. With the home intact, stopping keeps its usual saving order and limits. +A new process can still create a home on first use. + ## Repair a busy engine store or sync state — codeaf doctor --fix When an engine store lock or cross-computer sync issue exists on this computer, `codeaf doctor` adds `engine store` and `sync` rows naming a busy store holder (`busy · held by , pid `), an `unfinished continue · chat `, `duplicate rows · chat `, or a `lapsed hold on this computer · chat `. Healthy stores and clean sync state print nothing. From fff8847f83a17206be433e5f566aba4b71da6bc2 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Sat, 31 Oct 2026 00:35:41 -0400 Subject: [PATCH 16/18] changes: #2086 no longer carries the --once change Co-Authored-By: Claude Opus 5.5 --- docs/changes/unreleased/2086-deleted-home-and-once-chats.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/changes/unreleased/2086-deleted-home-and-once-chats.md b/docs/changes/unreleased/2086-deleted-home-and-once-chats.md index d70d7fa60b..4d01e1025f 100644 --- a/docs/changes/unreleased/2086-deleted-home-and-once-chats.md +++ b/docs/changes/unreleased/2086-deleted-home-and-once-chats.md @@ -1,11 +1,10 @@ --- kind: fixed -title: A deleted codeaf home is never rewritten, and a chat --once chat always reaches your other computers +title: A deleted codeaf home is never rewritten by an engine that is stopping pr: 2086 surface: [chat, engine] invalidates: - "An engine still saving after it was asked to stop could find its home deleted and write a brand-new identity.json, device.json, identity-solo and sync ledgers into it, as if it were a first run; codeaf now refuses with \"codeaf home was removed or replaced; restart codeaf to use the current folder\", never re-creates a home it started in, and a stopping engine exits at once when its home is removed. A first run is unchanged." - - "A paired codeaf chat --once could exit after uploading its chat but before registering it, so the other computer never listed it and codeaf doctor said nothing; --once now finishes its final send and release within the usual closing limit, codeaf doctor reports a chat this computer started that your other computers do not have, and codeaf doctor --fix registers it. A computer that never paired is unchanged." - "Naming the process that holds a busy engine store could take many seconds on macOS (one lsof per lock file, 500 ms each); one short budget now covers the whole lookup." --- From b534a6d97276ed7f45322e21096f3dc534d32423 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Sat, 31 Oct 2026 00:56:20 -0400 Subject: [PATCH 17/18] Private apply tolerates macOS's own com.apple.provenance attribute macOS 15 stamps com.apple.provenance on every file a process tree started from some apps creates, and the owner cannot remove it. Private apply refused any extended attribute, so on such a Mac it refused every changed file. That one attribute is now tolerated on macOS: the replacements apply writes come from the same process tree and are stamped the same way. Every other attribute, quarantine included, is still refused, and Linux is unchanged. Co-Authored-By: Claude Opus 5.5 --- internal/manual/chat/task-controls.md | 4 ++- internal/taskcopy/review_metadata_darwin.go | 9 +++++ internal/taskcopy/review_metadata_linux.go | 5 +++ internal/taskcopy/review_metadata_other.go | 5 +++ internal/taskcopy/review_metadata_unix.go | 38 ++++++++++++++++++++- 5 files changed, 59 insertions(+), 2 deletions(-) diff --git a/internal/manual/chat/task-controls.md b/internal/manual/chat/task-controls.md index 512e31c75c..0b1c6ad9dc 100644 --- a/internal/manual/chat/task-controls.md +++ b/internal/manual/chat/task-controls.md @@ -274,7 +274,9 @@ ref/index checks continue to protect the destination. The portable apply contract preserves raw file bytes and line endings, file permissions, empty directories and directory permissions. Changed paths with unsupported ownership, hardlinks, extended attributes, native flags or access -control metadata are refused before writing the original. Timestamps are outside +control metadata are refused before writing the original. The one exception is +`com.apple.provenance`, which macOS stamps on new files itself and which the +replacements receive the same way; it never causes a refusal. Timestamps are outside this portable contract. Older copies without a historical metadata baseline stay readable, but this apply door cannot infer their original metadata safely. diff --git a/internal/taskcopy/review_metadata_darwin.go b/internal/taskcopy/review_metadata_darwin.go index e1c2a1003c..2a071f2224 100644 --- a/internal/taskcopy/review_metadata_darwin.go +++ b/internal/taskcopy/review_metadata_darwin.go @@ -79,3 +79,12 @@ func SetPortableSymlinkModeAt(parentFD int, name string, mode fs.FileMode) error } return unix.Fchmodat(parentFD, name, uint32(mode.Perm()), unix.AT_SYMLINK_NOFOLLOW) } + +// OSManagedAttribute reports whether an extended attribute is one macOS +// stamps on its own and that portable apply may leave behind. macOS 15 adds +// com.apple.provenance to every file a process tree started from certain apps +// creates, and it cannot be removed by its owner; the replacements apply +// writes are created by the same process tree and are stamped the same way, +// so nothing a person set is lost. Every other attribute, quarantine included, +// is still refused. +func OSManagedAttribute(name string) bool { return name == "com.apple.provenance" } diff --git a/internal/taskcopy/review_metadata_linux.go b/internal/taskcopy/review_metadata_linux.go index ca37614f16..bedeb21763 100644 --- a/internal/taskcopy/review_metadata_linux.go +++ b/internal/taskcopy/review_metadata_linux.go @@ -50,3 +50,8 @@ func SetPortableSymlinkModeAt(parentFD int, name string, mode fs.FileMode) error } return nil } + +// OSManagedAttribute reports whether an extended attribute is one the +// operating system stamps on its own and that portable apply may leave +// behind. Linux stamps none, so every attribute is refused as before. +func OSManagedAttribute(string) bool { return false } diff --git a/internal/taskcopy/review_metadata_other.go b/internal/taskcopy/review_metadata_other.go index 015e963506..eea6184f13 100644 --- a/internal/taskcopy/review_metadata_other.go +++ b/internal/taskcopy/review_metadata_other.go @@ -21,3 +21,8 @@ func portableUnsupportedNativeMetadata(path string, info fs.FileInfo) error { func SetPortableSymlinkModeAt(parentFD int, name string, mode fs.FileMode) error { return errors.New("this platform cannot preserve symlink permissions") } + +// OSManagedAttribute reports whether an extended attribute is one the +// operating system stamps on its own and that portable apply may leave +// behind. This platform does not read extended attributes here, so none is. +func OSManagedAttribute(string) bool { return false } diff --git a/internal/taskcopy/review_metadata_unix.go b/internal/taskcopy/review_metadata_unix.go index ed21ed96cd..d93ec3b17b 100644 --- a/internal/taskcopy/review_metadata_unix.go +++ b/internal/taskcopy/review_metadata_unix.go @@ -6,6 +6,7 @@ import ( "errors" "io/fs" "os" + "strings" "syscall" "golang.org/x/sys/unix" @@ -31,7 +32,42 @@ func portableUnsupportedNativeMetadata(path string, info fs.FileInfo) error { return err } if size > 0 { - return errors.New("portable apply cannot preserve extended attributes or access-control metadata") + names, err := portableAttributeNames(path, size) + if err != nil { + return err + } + for _, name := range names { + if !OSManagedAttribute(name) { + return errors.New("portable apply cannot preserve extended attributes or access-control metadata") + } + } } return portableFilesystemFlags(path, info) } + +// portableAttributeNames reads the extended attribute names on path without +// following a symlink. The list can grow between the size query and the read, +// so a short buffer is retried once with the new size before it is an error. +func portableAttributeNames(path string, size int) ([]string, error) { + for attempt := 0; attempt < 2; attempt++ { + buffer := make([]byte, size) + n, err := unix.Llistxattr(path, buffer) + if errors.Is(err, unix.ERANGE) { + if size, err = unix.Llistxattr(path, nil); err != nil { + return nil, err + } + continue + } + if err != nil { + return nil, err + } + var names []string + for _, name := range strings.Split(string(buffer[:n]), "\x00") { + if name != "" { + names = append(names, name) + } + } + return names, nil + } + return nil, errors.New("extended attributes changed while they were being read") +} From 58e1327544b847b6b93376859c0ecade32b43269 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Sat, 31 Oct 2026 00:56:20 -0400 Subject: [PATCH 18/18] test(session): Apply fixtures leave macOS provenance in place Co-Authored-By: Claude Opus 5.5 --- .../task_workspace_fixture_unix_test.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/internal/session/task_workspace_fixture_unix_test.go b/internal/session/task_workspace_fixture_unix_test.go index 7a1aeddec3..12e23f20ac 100644 --- a/internal/session/task_workspace_fixture_unix_test.go +++ b/internal/session/task_workspace_fixture_unix_test.go @@ -12,21 +12,32 @@ import ( "testing" "golang.org/x/sys/unix" + + "github.com/Agent-Field/codeaf/internal/taskcopy" ) -// macOS inherits directory groups and can attach provenance to otherwise plain fixture files. +// macOS inherits directory groups and stamps provenance on otherwise plain fixture files. func normalizeWorkspaceApplyFixtureMetadata(t *testing.T, path string) { t.Helper() if err := os.Lchown(path, os.Getuid(), os.Getgid()); err != nil { t.Fatalf("set Apply fixture ownership for %s: %v", path, err) } + // macOS 15 stamps com.apple.provenance on files a process tree started + // from some apps creates, and its owner cannot remove it; the product + // tolerates exactly that attribute (taskcopy.OSManagedAttribute), so the + // fixture leaves it and clears only attributes a person could have set. for _, name := range workspaceApplyFixtureAttributes(t, path) { + if taskcopy.OSManagedAttribute(name) { + continue + } if err := unix.Lremovexattr(path, name); err != nil { t.Fatalf("remove extended attribute %q from Apply fixture %s: %v", name, path, err) } } - if names := workspaceApplyFixtureAttributes(t, path); len(names) != 0 { - t.Fatalf("extended attribute %q remains on Apply fixture %s after removal", names[0], path) + for _, name := range workspaceApplyFixtureAttributes(t, path) { + if !taskcopy.OSManagedAttribute(name) { + t.Fatalf("extended attribute %q remains on Apply fixture %s after removal", name, path) + } } } @@ -100,7 +111,7 @@ func TestTaskWorkspaceReviewRefusesInheritedGroupUntilFixtureIsNormalized(t *tes } func TestTaskWorkspaceReviewAcceptsFixtureAfterRemovingExtendedAttributes(t *testing.T) { - // macOS provenance must be removed from success fixtures without relaxing Review's metadata refusal. + // An attribute a person set is still refused by Review; only macOS provenance is tolerated. agent, id, _, source, copyDir := completedWorkspaceApplyFixture(t, false) attribute := "user.codeaf.fixture" if runtime.GOOS == "darwin" {