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/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/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/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/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..4d01e1025f --- /dev/null +++ b/docs/changes/unreleased/2086-deleted-home-and-once-chats.md @@ -0,0 +1,15 @@ +--- +kind: fixed +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." + - "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. 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/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/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{} } 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)) 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/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/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. 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/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) 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..12e23f20ac --- /dev/null +++ b/internal/session/task_workspace_fixture_unix_test.go @@ -0,0 +1,138 @@ +//go:build linux || darwin + +package session + +import ( + "errors" + "os" + "path/filepath" + "runtime" + "strings" + "syscall" + "testing" + + "golang.org/x/sys/unix" + + "github.com/Agent-Field/codeaf/internal/taskcopy" +) + +// 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) + } + } + for _, name := range workspaceApplyFixtureAttributes(t, path) { + if !taskcopy.OSManagedAttribute(name) { + t.Fatalf("extended attribute %q remains on Apply fixture %s after removal", name, 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) { + // 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" { + 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" { 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_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/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/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/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": 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 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 { 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) } 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") +} 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"))