diff --git a/cmd/codeaf/chatv3_surface.go b/cmd/codeaf/chatv3_surface.go index 527869deef..d07066b077 100644 --- a/cmd/codeaf/chatv3_surface.go +++ b/cmd/codeaf/chatv3_surface.go @@ -82,7 +82,7 @@ func runSurface(ctx context.Context, options tui3.Options) error { options.Restart = restart } options.CellsUnavailable = cell.UnavailableReason() - if options.CellsUnavailable == "" { + if options.CellsUnavailable == "" && options.Host == "" { wireSync(&options) options.WaitForRelease = func() bool { s, err := syncOf() diff --git a/cmd/codeaf/doctor.go b/cmd/codeaf/doctor.go index 0a1993de43..9302638ad9 100644 --- a/cmd/codeaf/doctor.go +++ b/cmd/codeaf/doctor.go @@ -43,7 +43,10 @@ type doctorSnapshot struct { Key keyReport // Cells is what the cells' working files hold against the disk budget, "" when // they hold nothing. - Cells string + Cells string + EngineStore []string + SyncIssues []string + Fixes []string } // fallbackKeyEnv is the compatibility variable [config.Load] accepts for an @@ -87,11 +90,12 @@ func runDoctor(args []string) error { func runDoctorWith(args []string, output io.Writer, dailyBudget float64) error { flags := commandFlags("doctor") database := flags.String("db", defaultChatDB(), storeFlagHelp) + fix := flags.Bool("fix", false, "repair safe engine store and sync state") if err := parseCommandFlags(flags, reorder(flags, args)); err != nil { return err } if flags.NArg() != 0 { - return fmt.Errorf("usage: codeaf doctor [--db path]") + return fmt.Errorf("usage: codeaf doctor [--db path] [--fix]") } path, err := expandHome(strings.TrimSpace(*database)) if err != nil { @@ -128,6 +132,7 @@ func runDoctorWith(args []string, output io.Writer, dailyBudget float64) error { // moves the store and moves nothing about who this machine can talk to. snapshot.Key = readKeyReport(strings.TrimSpace(env.Get(config.ProfileDirEnv))) snapshot.Cells = cellsUsage() + snapshot.EngineStore, snapshot.SyncIssues, snapshot.Fixes = inspectAndRepairDoctor(*fix) _, err = io.WriteString(output, formatDoctor(snapshot)) return err } @@ -250,6 +255,15 @@ func formatDoctor(snapshot doctorSnapshot) string { if line := formatCallLog(snapshot.CallLog); line != "" { block += fmt.Sprintf("%-16s %s\n", "model calls", line) } + for _, line := range snapshot.EngineStore { + block += fmt.Sprintf("%-16s %s\n", "engine store", line) + } + for _, line := range snapshot.SyncIssues { + block += fmt.Sprintf("%-16s %s\n", "sync", line) + } + for _, line := range snapshot.Fixes { + block += line + "\n" + } return block } diff --git a/cmd/codeaf/doctor_repair.go b/cmd/codeaf/doctor_repair.go new file mode 100644 index 0000000000..ee0e723b4b --- /dev/null +++ b/cmd/codeaf/doctor_repair.go @@ -0,0 +1,477 @@ +package main + +import ( + "bytes" + "context" + "errors" + "fmt" + "maps" + "os" + "path/filepath" + "slices" + "strings" + "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/handoff" + "github.com/Agent-Field/codeaf/internal/home" + "github.com/Agent-Field/codeaf/internal/session" + "github.com/Agent-Field/codeaf/internal/syncsetup" +) + +const doctorSyncTimeout = 3 * time.Second + +// inspectAndRepairDoctor collects engine-store and cross-computer sync health +// diagnostics and, when fix is true, repairs only what cannot touch another +// computer's live hold or discard a unique turn. +func inspectAndRepairDoctor(fix bool) (engineStore []string, syncIssues []string, fixes []string) { + storeLines, storeFixes := inspectDoctorStores(fix) + engineStore = append(engineStore, storeLines...) + fixes = append(fixes, storeFixes...) + + syncLines, syncFixes := inspectDoctorSync(fix) + syncIssues = append(syncIssues, syncLines...) + fixes = append(fixes, syncFixes...) + return engineStore, syncIssues, fixes +} + +func doctorStoreDirs() []string { + seen := map[string]bool{} + var out []string + add := func(dir string) { + clean := filepath.Clean(strings.TrimSpace(dir)) + if clean == "" || clean == "." || strings.HasSuffix(clean, ".lock") || seen[clean] { + return + } + seen[clean] = true + out = append(out, clean) + } + add(cellstore.SharedStoreDir()) + matches, _ := filepath.Glob(filepath.Join(home.Join("v3", "stores"), "*")) + slices.Sort(matches) + for _, m := range matches { + info, err := os.Stat(m) + if err == nil && info.IsDir() { + add(m) + } + } + return out +} + +func inspectDoctorStores(fix bool) (issues []string, fixes []string) { + dirs := doctorStoreDirs() + seenBusy := map[string]bool{} + for _, dir := range dirs { + if holder, busy := cellstore.InspectStoreBusy(dir, ""); busy { + key := filepath.Clean(holder.Lock) + if key == "." || key == "" { + key = dir + } + if !seenBusy[key] { + seenBusy[key] = true + if who := holder.Describe(); who != "" { + issues = append(issues, "busy · held by "+who) + } else { + issues = append(issues, "busy") + } + } + } + } + return issues, fixes +} + +type localChatInventory struct { + foldersByID map[string][]string + inUseByID map[string]bool + orphanStaging map[string]string +} + +func scanLocalChatInventory() localChatInventory { + inv := localChatInventory{ + foldersByID: map[string][]string{}, + inUseByID: map[string]bool{}, + orphanStaging: map[string]string{}, + } + seenFolder := map[string]bool{} + recordFolder := func(id, dir string) { + id = strings.TrimSpace(id) + dir = filepath.Clean(strings.TrimSpace(dir)) + if id == "" || dir == "" || dir == "." { + return + } + transcript := session.Place{Dir: dir}.Transcript() + if session.InUse(transcript) || handoff.Arriving(dir) { + inv.inUseByID[id] = true + } + key := id + "\x00" + dir + if seenFolder[key] { + return + } + seenFolder[key] = true + inv.foldersByID[id] = append(inv.foldersByID[id], dir) + } + + for _, row := range session.ReadHome().Sessions() { + id := strings.TrimSpace(row.ID) + if id == "" { + if dir, ok := session.FolderOf(row.Transcript); ok { + id = filepath.Base(dir) + } + } + dir := session.DirOf(row.Transcript) + if row.Open { + inv.inUseByID[id] = true + } + recordFolder(id, dir) + } + + var candidates []string + for _, pattern := range []string{ + filepath.Join(home.Join("v3", "projects"), "*", "*"), + filepath.Join(home.Join("v3", "projects"), "*", ".moving", "*"), + filepath.Join(cell.Dir(home.Dir()), "*"), + } { + matches, _ := filepath.Glob(pattern) + candidates = append(candidates, matches...) + } + slices.Sort(candidates) + for _, entry := range candidates { + base := filepath.Base(entry) + if base == ".moving" || strings.HasSuffix(base, ".arriving") || strings.HasSuffix(base, ".lock") { + continue + } + info, err := os.Stat(entry) + if err != nil || !info.IsDir() { + continue + } + if strings.HasSuffix(base, handoff.StagingSuffix) { + baseRoot := strings.TrimSuffix(entry, handoff.StagingSuffix) + id := strings.TrimSpace(filepath.Base(baseRoot)) + if id == "" { + continue + } + if handoff.Arriving(baseRoot) || session.InUse(session.Place{Dir: baseRoot}.Transcript()) { + inv.inUseByID[id] = true + continue + } + if _, exists := inv.orphanStaging[id]; !exists { + inv.orphanStaging[id] = filepath.Clean(entry) + } + continue + } + if strings.HasSuffix(base, ".replaced") { + continue + } + hasTranscript := fileExists(session.Place{Dir: entry}.Transcript()) + hasCell := fileExists(filepath.Join(entry, filepath.FromSlash(cell.MetaPath))) + if hasTranscript || hasCell { + id := base + if meta, err := session.LoadMeta(entry); err == nil && strings.TrimSpace(meta.ID) != "" { + id = strings.TrimSpace(meta.ID) + } + recordFolder(id, entry) + } + } + return inv +} + +func inspectDoctorSync(fix bool) (issues []string, fixes []string) { + inv := scanLocalChatInventory() + handledStaging := map[string]bool{} + + if syncsetup.MayTalk(home.Dir()) { + if s, ok, err := syncsetup.Open(home.Dir()); err == nil && ok { + ctx, cancel := context.WithTimeout(context.Background(), doctorSyncTimeout) + defer cancel() + if l, listErr := s.Dir.List(ctx); listErr == nil { + self := s.Device.ID() + ids := slices.Sorted(maps.Keys(l.Cells)) + reportedUnfinished := map[string]bool{} + releasedTake := map[string]bool{} + + // 1. Unfinished continues started by this computer. + for _, id := range ids { + rec := l.Cells[id] + if rec.Archived || inv.inUseByID[id] || handoff.Arriving(takeRootFor(id)) { + continue + } + takingDir := inv.orphanStaging[id] + heldLiveBySelf := rec.Lease.Device == self && rec.Lease.Expires > l.Now + heldLiveByOther := rec.Lease.Device != "" && rec.Lease.Device != self && rec.Lease.Expires > l.Now + if heldLiveByOther { + // Never touch another computer's live hold. + continue + } + _, hasLocal := openCellByID(id) + unfinished := false + if heldLiveBySelf && (takingDir != "" || hasLocal != nil || !sendLeaseMatches(s, id, rec.Lease.Fence)) { + unfinished = true + } else if takingDir != "" { + unfinished = true + } + if !unfinished { + continue + } + handledStaging[id] = true + + if fix { + safeStaging := takingDir == "" || !stagingHasUniqueTurns(takingDir, id, rec.Head) + released := false + if heldLiveBySelf { + relErr := directory.ReleaseQuiet(ctx, s.Dir, id, rec.Lease.Fence) + if errors.Is(relErr, directory.ErrUnauthorized) { + relErr = s.Dir.Release(ctx, id, rec.Lease.Fence) + } + if relErr == nil { + released = true + rec.Lease.Expires, rec.Lease.Pending = 0, 0 + l.Cells[id] = rec + } + } + removedStaging := false + if (!heldLiveBySelf || released) && takingDir != "" && safeStaging { + if rmErr := os.RemoveAll(takingDir); rmErr == nil { + removedStaging = true + delete(inv.orphanStaging, id) + } + } + if (heldLiveBySelf && released) || (!heldLiveBySelf && removedStaging) { + fixes = append(fixes, fmt.Sprintf("fixed: released unfinished continue for chat %s", id)) + releasedTake[id] = true + if takingDir == "" || removedStaging { + continue + } + } + } + issues = append(issues, fmt.Sprintf("unfinished continue · chat %s", id)) + reportedUnfinished[id] = true + } + + // 2. Duplicate Home rows for one chat id: local duplicate folders and sync branch copies. + localIDs := slices.Sorted(maps.Keys(inv.foldersByID)) + for _, id := range localIDs { + if len(inv.foldersByID[id]) > 1 { + issues = append(issues, fmt.Sprintf("duplicate rows · chat %s (%d local folders)", id, len(inv.foldersByID[id]))) + } + } + for _, branchID := range ids { + branchRec := l.Cells[branchID] + if branchRec.Archived || branchRec.ParentCell == "" || branchRec.OrphanTurns == 0 { + continue + } + mainID := resolveRootCellID(l.Cells, branchRec.ParentCell) + mainRec, hasMain := l.Cells[mainID] + allInMain := hasMain && !mainRec.Archived && branchTurnsAllInMain(mainID, mainRec, branchID, branchRec) + heldLiveByOther := branchRec.Lease.Expires > l.Now && branchRec.Lease.Device != "" && branchRec.Lease.Device != self + if fix && allInMain && !heldLiveByOther { + if discErr := s.Discard(ctx, branchID); discErr == nil { + fixes = append(fixes, fmt.Sprintf("fixed: retired duplicate branch copy %s of chat %s", branchID, mainID)) + branchRec.Archived = true + l.Cells[branchID] = branchRec + continue + } + } + if allInMain { + issues = append(issues, fmt.Sprintf("duplicate rows · chat %s (branch copy %s, all turns in main chat)", mainID, branchID)) + } else { + turnsWord := fmt.Sprintf("%d turns", branchRec.OrphanTurns) + if branchRec.OrphanTurns == 1 { + turnsWord = "1 turn" + } + issues = append(issues, fmt.Sprintf("duplicate rows · chat %s (branch copy %s, %s not in main chat)", mainID, branchID, turnsWord)) + } + } + + // 3. This computer's own lapsed holds that would ask a question on open. + for _, id := range ids { + rec := l.Cells[id] + if rec.Archived || rec.OrphanTurns > 0 || reportedUnfinished[id] || releasedTake[id] || inv.inUseByID[id] { + continue + } + if rec.Lease.Device == self && rec.Lease.Expires > 0 && rec.Lease.Expires <= l.Now { + if lapsedHoldWouldAskOnOpen(s, id, rec) { + issues = append(issues, fmt.Sprintf("lapsed hold on this computer · chat %s", id)) + } + } + } + + // Any local orphan .taking folder for a chat id absent from the directory listing. + issues, fixes = finishOrphanStaging(inv.orphanStaging, handledStaging, fix, issues, fixes) + return issues, fixes + } + } + } + + // Sync is off or unreachable: still report local duplicate Home rows and orphaned .taking folders. + localIDs := slices.Sorted(maps.Keys(inv.foldersByID)) + for _, id := range localIDs { + if len(inv.foldersByID[id]) > 1 { + issues = append(issues, fmt.Sprintf("duplicate rows · chat %s (%d local folders)", id, len(inv.foldersByID[id]))) + } + } + issues, fixes = finishOrphanStaging(inv.orphanStaging, handledStaging, fix, issues, fixes) + return issues, fixes +} + +func finishOrphanStaging(orphanStaging map[string]string, handled map[string]bool, fix bool, issues, fixes []string) ([]string, []string) { + ids := slices.Sorted(maps.Keys(orphanStaging)) + for _, id := range ids { + if handled[id] { + continue + } + takingDir := orphanStaging[id] + if fix && !stagingHasUniqueTurns(takingDir, id, "") { + if err := os.RemoveAll(takingDir); err == nil { + fixes = append(fixes, fmt.Sprintf("fixed: released unfinished continue for chat %s", id)) + continue + } + } + issues = append(issues, fmt.Sprintf("unfinished continue · chat %s", id)) + } + return issues, fixes +} + +func sendLeaseMatches(s *syncsetup.Sync, id string, fence uint64) bool { + heldCell, heldFence, err := s.SendLease(id) + return err == nil && heldCell == id && heldFence == fence && fence != 0 +} + +func resolveRootCellID(cells map[string]directory.Cell, parent string) string { + cur := strings.TrimSpace(parent) + for hops := 0; cur != "" && hops < 16; hops++ { + rec, ok := cells[cur] + if !ok || rec.OrphanTurns == 0 || strings.TrimSpace(rec.ParentCell) == "" || rec.ParentCell == cur { + break + } + cur = strings.TrimSpace(rec.ParentCell) + } + return cur +} + +func branchTurnsAllInMain(mainID string, mainRec directory.Cell, branchID string, branchRec directory.Cell) bool { + if strings.TrimSpace(branchRec.Head) == "" { + return false + } + if mainRec.Head != "" && branchRec.Head == mainRec.Head { + return true + } + mainLocal, err := openCellByID(mainID) + if err != nil { + return false + } + if mainRec.Head != "" { + if mainKnown, err := cellstore.KnowsHead(mainLocal, mainRec.Head); err != nil || !mainKnown { + return false + } + } + if branchLocal, err := openCellByID(branchID); err == nil { + branchTurns, err := cellstore.Turns(branchLocal) + if err != nil || len(branchTurns) == 0 || branchTurns[len(branchTurns)-1].ID != branchRec.Head { + return false + } + for _, turn := range branchTurns { + if known, err := cellstore.KnowsHead(mainLocal, turn.ID); err != nil || !known { + return false + } + } + return true + } + known, err := cellstore.KnowsHead(mainLocal, branchRec.Head) + return err == nil && known +} + +func lapsedHoldWouldAskOnOpen(s *syncsetup.Sync, id string, rec directory.Cell) bool { + localCell, err := openCellByID(id) + if err != nil { + return true + } + if known, err := cellstore.KnowsHead(localCell, rec.Head); err != nil || !known { + return true + } + if extends, err := cellstore.Extends(localCell, rec.Head); err != nil || !extends { + return true + } + if head, err := cellstore.Head(localCell); err != nil || head == nil || head.Turn.ID != rec.Head { + return true + } + if rec.Lease.Pending > 0 { + return true + } + if heldCell, heldFence, err := s.SendLease(id); err == nil && heldFence != 0 && (heldCell != id || heldFence != rec.Lease.Fence) { + return true + } + return false +} + +// stagingHasUniqueTurns fails closed whenever a .taking scratch folder holds any +// sealed turn or unsealed transcript content that does not already exist in the +// local main chat or the directory's durable head. +func stagingHasUniqueTurns(takingDir, id, durableHead string) bool { + entries, err := os.ReadDir(takingDir) + if err != nil { + return !os.IsNotExist(err) + } + if len(entries) == 0 { + return false + } + + mainCell, mainErr := openCellByID(id) + stagedCell := cell.Cell{ID: id, Root: takingDir} + hasStagedCell := fileExists(filepath.Join(takingDir, filepath.FromSlash(cell.MetaPath))) + if hasStagedCell { + if _, busy := cellstore.InspectStoreBusy(cellstore.SharedStoreDir(), ""); busy { + return true + } + stagedTurns, turnsErr := cellstore.Turns(stagedCell) + if turnsErr != nil && !os.IsNotExist(turnsErr) { + return true + } + for _, turn := range stagedTurns { + if durableHead != "" && len(stagedTurns) > 0 && stagedTurns[len(stagedTurns)-1].ID == durableHead { + break + } + if mainErr != nil { + return true + } + if known, err := cellstore.KnowsHead(mainCell, turn.ID); err != nil || !known { + return true + } + } + } + + stagedTranscript := session.Place{Dir: takingDir}.Transcript() + stagedRaw, readErr := os.ReadFile(stagedTranscript) + if readErr != nil { + return !os.IsNotExist(readErr) + } + if len(bytes.TrimSpace(stagedRaw)) == 0 { + return false + } + if hasStagedCell { + if head, err := cellstore.Head(stagedCell); err == nil && head != nil && int64(len(stagedRaw)) <= head.Receipt.Transcript.End { + if durableHead != "" && head.Turn.ID == durableHead { + return false + } + if mainErr == nil { + if known, err := cellstore.KnowsHead(mainCell, head.Turn.ID); err == nil && known { + return false + } + } + } + } + if mainErr != nil { + return true + } + mainRaw, err := os.ReadFile(session.Place{Dir: mainCell.Root}.Transcript()) + if err != nil { + return true + } + return !bytes.HasPrefix(mainRaw, stagedRaw) +} + +func fileExists(path string) bool { + info, err := os.Stat(path) + return err == nil && !info.IsDir() +} diff --git a/cmd/codeaf/doctor_repair_test.go b/cmd/codeaf/doctor_repair_test.go new file mode 100644 index 0000000000..7cff4fcb8d --- /dev/null +++ b/cmd/codeaf/doctor_repair_test.go @@ -0,0 +1,374 @@ +package main + +import ( + "bufio" + "bytes" + "context" + "encoding/json" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/cell" + "github.com/Agent-Field/codeaf/internal/cell/celltest" + "github.com/Agent-Field/codeaf/internal/cellstore" + "github.com/Agent-Field/codeaf/internal/directory" + "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/relayserve" + "github.com/Agent-Field/codeaf/internal/session" + "github.com/Agent-Field/codeaf/internal/syncsetup" +) + +func TestDoctorHoldStoreHelperProcess(t *testing.T) { + storeDir := os.Getenv("CODEAF_TEST_DOCTOR_HOLD_STORE") + if storeDir == "" { + return + } + release, err := cellstore.HoldStoreWriter(storeDir, "codeaf chat") + if err != nil { + os.Exit(2) + } + _, _ = os.Stdout.WriteString("LOCKED\n") + _, _ = bufio.NewReader(os.Stdin).ReadBytes('\n') + _ = release() + os.Exit(0) +} + +func startDoctorStoreHolder(t *testing.T, storeDir string) func() { + t.Helper() + exe, err := os.Executable() + if err != nil { + t.Fatal(err) + } + cmd := exec.Command(exe, "-test.run=^TestDoctorHoldStoreHelperProcess$") //codeaf:plumbing spawns this test binary to hold an engine store lock in another process + cmd.Env = append(os.Environ(), "CODEAF_TEST_DOCTOR_HOLD_STORE="+storeDir) + stdin, err := cmd.StdinPipe() + if err != nil { + t.Fatal(err) + } + stdout, err := cmd.StdoutPipe() + if err != nil { + t.Fatal(err) + } + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + line, err := bufio.NewReader(stdout).ReadString('\n') + if err != nil || strings.TrimSpace(line) != "LOCKED" { + _ = cmd.Process.Kill() + _ = cmd.Wait() + t.Fatalf("helper failed to hold store %s: line=%q err=%v", storeDir, line, err) + } + var stopped bool + stop := func() { + if stopped { + return + } + stopped = true + _, _ = stdin.Write([]byte("release\n")) + _ = stdin.Close() + done := make(chan error, 1) + go func() { done <- cmd.Wait() }() + select { + case <-done: + case <-time.After(2 * time.Second): + _ = cmd.Process.Kill() + <-done + } + } + t.Cleanup(stop) + return stop +} + +type doctorTestClock struct { + mu sync.Mutex + offset time.Duration +} + +func (c *doctorTestClock) Now() time.Time { + c.mu.Lock() + defer c.mu.Unlock() + return time.Now().Add(c.offset) +} + +func (c *doctorTestClock) SetOffset(d time.Duration) { + c.mu.Lock() + defer c.mu.Unlock() + c.offset = d +} + +func writeLocalChatFolder(t *testing.T, dir, id, title, transcriptBody string) { + t.Helper() + if err := os.MkdirAll(dir, 0o700); err != nil { + t.Fatal(err) + } + if err := session.SaveMeta(dir, session.Meta{ + ID: id, + Title: title, + Workspace: dir, + }); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(session.Place{Dir: dir}.Transcript(), []byte(transcriptBody), 0o600); err != nil { + t.Fatal(err) + } +} + +// Contract 5: `codeaf doctor` reports a busy engine store and its holder, an +// unfinished take, duplicate Home rows for one chat id, and this computer's own +// lapsed holds that would ask a question on open; without `--fix` it mutates +// nothing; with `--fix` it repairs only what is safe, prints one line per +// change, never touches another computer's live hold, and never deletes a turn +// that exists only in the thing it removes. +func TestDoctorFindsAndRepairsSyncAndEngineStoreSafely(t *testing.T) { + celltest.Available(t) + clock := &doctorTestClock{offset: -2 * time.Minute} + svc := relayserve.New(relayserve.Config{Store: t.TempDir(), Now: clock.Now}) + srv := httptest.NewServer(svc.Handler) + t.Cleanup(func() { + srv.Close() + svc.Close() + }) + + homeA := t.TempDir() + homeB := t.TempDir() + t.Setenv(home.EnvVar, homeA) + t.Setenv(syncsetup.URLVar, srv.URL) + + id, err := identity.Ensure(homeA) + if err != nil { + t.Fatal(err) + } + if err := identity.Save(homeB, id); err != nil { + t.Fatal(err) + } + + syncA, ok, err := syncsetup.Open(homeA) + if err != nil || !ok { + t.Fatalf("syncsetup.Open(homeA) = (%v, %v)", ok, err) + } + syncB, ok, err := syncsetup.Open(homeB) + if err != nil || !ok { + t.Fatalf("syncsetup.Open(homeB) = (%v, %v)", ok, err) + } + ctx := context.Background() + if err := syncsetup.PutOwnDevice(ctx, syncA.Dir, syncA.Identity, syncA.Device, "desk"); err != nil { + t.Fatal(err) + } + if err := syncsetup.PutOwnDevice(ctx, syncB.Dir, syncB.Identity, syncB.Device, "laptop"); err != nil { + t.Fatal(err) + } + + const ( + unfinishedID = "01J00000000000000000000101" + uniqueStagingID = "01J00000000000000000000102" + otherLiveID = "01J00000000000000000000103" + mainChatID = "01J00000000000000000000104" + redundantBranchID = "01J00000000000000000000105" + uniqueBranchID = "01J00000000000000000000106" + lapsedSelfID = "01J00000000000000000000107" + localDupID = "01J00000000000000000000108" + + headMain = "1111111111111111111111111111111111111111111111111111111111111111" + headUnique = "2222222222222222222222222222222222222222222222222222222222222222" + ) + + makeInit := func(head, parent string, orphans uint32) directory.CellInit { + return directory.CellInit{ + Head: head, + Class: string(cell.HostBound), + ParentCell: parent, + OrphanTurns: orphans, + Keys: map[string]map[string]string{id.CellKeyID(): {id.ID(): ""}}, + } + } + + // 1. Lapsed hold on this computer (created 2 minutes ago on the relay clock, > 90s LeaseTTL, within 5m skew). + if _, err := syncA.Dir.Create(ctx, lapsedSelfID, makeInit(headMain, "", 0)); err != nil { + t.Fatal(err) + } + clock.SetOffset(0) + + // 2. Unfinished take on this computer: live lease on syncA with no SendLease receipt, plus empty .taking folder. + unfinishedView, err := syncA.Dir.Create(ctx, unfinishedID, makeInit(headMain, "", 0)) + if err != nil { + t.Fatal(err) + } + unfinishedTaking := filepath.Join(homeA, "v3", "projects", "copies", unfinishedID+handoff.StagingSuffix) + if err := os.MkdirAll(unfinishedTaking, 0o700); err != nil { + t.Fatal(err) + } + + // 3. Unfinished take whose .taking folder contains a unique unsealed turn absent from main chat (must NOT be deleted!). + uniqueStagingDir := filepath.Join(homeA, "v3", "projects", "copies", uniqueStagingID+handoff.StagingSuffix) + writeLocalChatFolder(t, uniqueStagingDir, uniqueStagingID, "Unique Staged Turn", "{\"type\":\"session\",\"id\":\""+uniqueStagingID+"\"}\n{\"role\":\"user\",\"content\":\"only in staging\"}\n") + + // 4. Live hold on another computer (syncB) — must NEVER be touched by doctor --fix! + otherView, err := syncB.Dir.Create(ctx, otherLiveID, makeInit(headMain, "", 0)) + if err != nil { + t.Fatal(err) + } + + // 5. Main chat + two branch copies: + // - redundantBranchID has Head == headMain (all turns in main chat -> safe to retire) + // - uniqueBranchID has Head == headUnique (turns NOT in main chat -> must NEVER be retired!) + mainView, err := syncB.Dir.Create(ctx, mainChatID, makeInit(headMain, "", 0)) + if err != nil { + t.Fatal(err) + } + if err := syncB.Dir.Release(ctx, mainChatID, mainView.Cell.Lease.Fence); err != nil { + t.Fatal(err) + } + redBranchView, err := syncA.Dir.Create(ctx, redundantBranchID, makeInit(headMain, mainChatID, 2)) + if err != nil { + t.Fatal(err) + } + if err := syncA.Dir.Release(ctx, redundantBranchID, redBranchView.Cell.Lease.Fence); err != nil { + t.Fatal(err) + } + uniqBranchView, err := syncB.Dir.Create(ctx, uniqueBranchID, makeInit(headUnique, mainChatID, 3)) + if err != nil { + t.Fatal(err) + } + if err := syncB.Dir.Release(ctx, uniqueBranchID, uniqBranchView.Cell.Lease.Fence); err != nil { + t.Fatal(err) + } + + // 6. Duplicate local Home folders for one chat id on disk. + header := "{\"type\":\"session\",\"id\":\"" + localDupID + "\"}\n" + writeLocalChatFolder(t, filepath.Join(homeA, "v3", "projects", "-work-proj", localDupID), localDupID, "Dup Local", header) + writeLocalChatFolder(t, filepath.Join(homeA, "v3", "projects", "copies", localDupID), localDupID, "Dup Local", header) + + // 7. Busy engine store (held by live foreign process) AND a lock file in another store + // whose recorded holder is dead. No live process holds that lock, so it is free: doctor + // neither reports it nor deletes it. + sharedStore := cellstore.SharedStoreDir() + stopHolder := startDoctorStoreHolder(t, sharedStore) + defer stopHolder() + + otherStore := filepath.Join(homeA, "v3", "stores", " stale-check") + otherStore = strings.ReplaceAll(otherStore, " ", "") + if err := os.MkdirAll(otherStore, 0o700); err != nil { + t.Fatal(err) + } + staleLockPath := filepath.Join(otherStore, "writer.lock") + stalePayload, _ := json.Marshal(cellstore.StoreHolder{PID: 99999999, Command: "codeaf chat"}) + if err := os.WriteFile(staleLockPath, append(stalePayload, '\n'), 0o600); err != nil { + t.Fatal(err) + } + + dbPath := filepath.Join(t.TempDir(), "graph.db") + + // ── STEP 1: `codeaf doctor` WITHOUT `--fix` reports everything and mutates NOTHING ── + var reportOut bytes.Buffer + if err := runDoctorWith([]string{"--db", dbPath}, &reportOut, 0); err != nil { + t.Fatal(err) + } + report := reportOut.String() + exeBase := filepath.Base(os.Args[0]) + for _, want := range []string{ + "engine store", + "busy · held by " + exeBase + ", pid ", + "unfinished continue · chat " + unfinishedID, + "unfinished continue · chat " + uniqueStagingID, + "duplicate rows · chat " + localDupID + " (2 local folders)", + "duplicate rows · chat " + mainChatID + " (branch copy " + redundantBranchID + ", all turns in main chat)", + "duplicate rows · chat " + mainChatID + " (branch copy " + uniqueBranchID + ", 3 turns not in main chat)", + "lapsed hold on this computer · chat " + lapsedSelfID, + } { + if !strings.Contains(report, want) { + t.Fatalf("doctor report missing %q:\n%s", want, report) + } + } + if strings.Contains(report, "fixed:") { + t.Fatalf("doctor without --fix printed fix lines:\n%s", report) + } + for _, forbidden := range []string{"daemon", "launchd", "systemd", "service", "brain", "standing watch", "auditor", "verdict", "verified", "refuted"} { + if strings.Contains(strings.ToLower(report), forbidden) { + t.Fatalf("doctor output contains forbidden word %q:\n%s", forbidden, report) + } + } + + if strings.Contains(report, staleLockPath) { + t.Fatalf("doctor reported a lock no live process holds:\n%s", report) + } + + // Verify zero state mutations occurred without --fix! + if _, err := os.Stat(unfinishedTaking); err != nil { + t.Fatalf("doctor without --fix deleted .taking dir: %v", err) + } + checkUnfinished, err := syncA.Dir.Cell(ctx, unfinishedID) + if err != nil || checkUnfinished.Cell.Lease.Expires <= checkUnfinished.Now || checkUnfinished.Cell.Lease.Fence != unfinishedView.Cell.Lease.Fence { + t.Fatalf("doctor without --fix altered unfinished take lease: %+v (err=%v)", checkUnfinished, err) + } + checkRedBranch, err := syncA.Dir.Cell(ctx, redundantBranchID) + if err != nil || checkRedBranch.Cell.Archived { + t.Fatalf("doctor without --fix archived redundant branch: %+v (err=%v)", checkRedBranch, err) + } + + // ── STEP 2: `codeaf doctor --fix` repairs what is safe and preserves every safety invariant ── + var fixOut bytes.Buffer + if err := runDoctorWith([]string{"--db", dbPath, "--fix"}, &fixOut, 0); err != nil { + t.Fatal(err) + } + fixedText := fixOut.String() + for _, wantFix := range []string{ + "fixed: released unfinished continue for chat " + unfinishedID, + "fixed: retired duplicate branch copy " + redundantBranchID + " of chat " + mainChatID, + } { + if !strings.Contains(fixedText, wantFix) { + t.Fatalf("doctor --fix missing %q:\n%s", wantFix, fixedText) + } + } + for _, forbidden := range []string{"daemon", "launchd", "systemd", "service", "brain", "standing watch", "auditor", "verdict", "verified", "refuted"} { + if strings.Contains(strings.ToLower(fixedText), forbidden) { + t.Fatalf("doctor --fix output contains forbidden word %q:\n%s", forbidden, fixedText) + } + } + + // Safe repairs happened: + if _, err := os.Stat(staleLockPath); err != nil { + t.Fatalf("doctor --fix deleted a free lock file: %v", err) + } + if _, err := os.Stat(unfinishedTaking); !os.IsNotExist(err) { + t.Fatalf("doctor --fix did not remove empty .taking dir: %v", err) + } + afterUnfinished, err := syncA.Dir.Cell(ctx, unfinishedID) + if err != nil || afterUnfinished.Cell.Lease.Expires != 0 { + t.Fatalf("doctor --fix did not release this computer's stale take: %+v (err=%v)", afterUnfinished, err) + } + afterRedBranch, err := syncA.Dir.Cell(ctx, redundantBranchID) + if err != nil || !afterRedBranch.Cell.Archived { + t.Fatalf("doctor --fix did not retire redundant branch copy: %+v (err=%v)", afterRedBranch, err) + } + + // Safety Invariant 1: NEVER touches another computer's live hold! + afterOther, err := syncB.Dir.Cell(ctx, otherLiveID) + if err != nil { + t.Fatal(err) + } + if afterOther.Cell.Lease.Device != syncB.Device.ID() || afterOther.Cell.Lease.Fence != otherView.Cell.Lease.Fence || afterOther.Cell.Lease.Expires <= afterOther.Now { + t.Fatalf("doctor --fix touched another computer's live hold: got %+v, want device %s fence %d live", afterOther.Cell.Lease, syncB.Device.ID(), otherView.Cell.Lease.Fence) + } + + // Safety Invariant 2: NEVER touches a live process's store lock! + if _, stillBusy := cellstore.InspectStoreBusy(sharedStore, ""); !stillBusy { + t.Fatal("doctor --fix disturbed a live foreign process's store lock") + } + + // Safety Invariant 3: NEVER deletes or retires a branch copy or .taking folder with unique turns! + afterUniqBranch, err := syncB.Dir.Cell(ctx, uniqueBranchID) + if err != nil || afterUniqBranch.Cell.Archived { + t.Fatalf("doctor --fix retired branch copy whose turns are NOT in main chat: %+v (err=%v)", afterUniqBranch, err) + } + if _, err := os.Stat(session.Place{Dir: uniqueStagingDir}.Transcript()); err != nil { + t.Fatalf("doctor --fix deleted .taking folder containing unique turns: %v", err) + } +} diff --git a/cmd/codeaf/main.go b/cmd/codeaf/main.go index 6015964a08..95d7fe80b1 100644 --- a/cmd/codeaf/main.go +++ b/cmd/codeaf/main.go @@ -579,7 +579,7 @@ Look at what happened — read-only, no key, nothing spent the models this machine will use, and what each has been measured at codeaf pool [show|status|verify] [--json] [--key key] the Model Pool: what is resolved and cached; verify fetches with a key - codeaf doctor [--db path] + codeaf doctor [--db path] [--fix] is this install healthy, and where does it keep things codeaf manual every page of codeaf's own manual, one per line diff --git a/cmd/codeaf/synctake.go b/cmd/codeaf/synctake.go index 6ac646e7af..9b6f75fd81 100644 --- a/cmd/codeaf/synctake.go +++ b/cmd/codeaf/synctake.go @@ -32,6 +32,9 @@ var surfaceNotices = tui3.NewNotices() // when it cannot start, it adds nothing and the home screen must not fail to // start over a relay setting: every seam stays nil, and a nil seam is absent. func wireSync(o *tui3.Options) { + if o.Host != "" { + return + } if o.Notices == nil { o.Notices = surfaceNotices } @@ -68,6 +71,70 @@ func wireSync(o *tui3.Options) { // surfaceMachines keeps display hints out of the decision to send a message. type surfaceMachines struct{ *syncsetup.Sync } +// RowsAt enriches branch rows with whether their turns already exist in the +// local main chat, off the surface loop, so Home can tell when discarding a +// collapsed copy loses nothing. +func (s surfaceMachines) RowsAt(ctx context.Context) ([]chatlist.Row, uint64, error) { + rows, version, err := s.Sync.RowsAt(ctx) + if err != nil || len(rows) == 0 { + return rows, version, err + } + markLocalRedundantBranches(rows) + return rows, version, nil +} + +func (s surfaceMachines) Rows(ctx context.Context) ([]chatlist.Row, error) { + rows, _, err := s.RowsAt(ctx) + return rows, err +} + +func markLocalRedundantBranches(rows []chatlist.Row) { + byCell := make(map[string]chatlist.Row, len(rows)) + hasBranch := false + for _, r := range rows { + byCell[r.Cell] = r + if r.Status == chatlist.Branch && r.Parent != "" && r.Head != "" { + hasBranch = true + } + } + if !hasBranch { + return + } + turnSets := map[string]map[string]bool{} + turnsOf := func(id string) map[string]bool { + if set, seen := turnSets[id]; seen { + return set + } + var set map[string]bool + if c, err := openCellByID(id); err == nil { + if logRecs, err := cellstore.Log(c); err == nil { + set = make(map[string]bool, len(logRecs)) + for _, rec := range logRecs { + set[rec.Turn.ID] = true + } + } + } + turnSets[id] = set + return set + } + for i := range rows { + if rows[i].Status != chatlist.Branch || rows[i].Parent == "" || rows[i].Head == "" { + continue + } + root := rows[i].Parent + for hops := 0; root != "" && hops < 16; hops++ { + parent, ok := byCell[root] + if !ok || parent.Status != chatlist.Branch || parent.Parent == "" || parent.Parent == root { + break + } + root = parent.Parent + } + if turns := turnsOf(root); turns[rows[i].Head] { + rows[i].BranchRedundant = true + } + } +} + // RowsForChatSend uses a live writer's receipt or a never-shared chat before asking the relay. // A known outage keeps unknown copies unsent without repeating the network wait. func (s surfaceMachines) RowsForChatSend(ctx context.Context, file string, offline, holding bool) ([]chatlist.Row, uint64, bool, error) { diff --git a/docs/changes/unreleased/2078-continue-every-time.md b/docs/changes/unreleased/2078-continue-every-time.md new file mode 100644 index 0000000000..3f9bb4ce81 --- /dev/null +++ b/docs/changes/unreleased/2078-continue-every-time.md @@ -0,0 +1,19 @@ +--- +kind: fixed +title: Continuing a chat from another computer works every time, and codeaf doctor can repair sync +pr: 2078 +surface: [chat, engine] +invalidates: + - "After one continue in a window, a second or third continue here could do nothing, because the running-take mark was not cleared on every ending; it is now cleared when a take opens, is brought forward, fails or is cancelled, and a press during a take says a chat is already continuing here." + - "A failed continue closed its card and said why only on Home's say line; the failure now stands on the card with 1 retry and 2 leave it there, or in the chat." + - "A busy engine store failed a continue with the generic could-not-be-put-here sentence; it now names the program holding the store and its pid, and offers retry." + - "Under codeaf chat --host, Home offered continue here and showed chats the other computer held as held elsewhere, acting on this computer's disk; under --host continue here is not offered and those chats are not held elsewhere." + - "One chat could show twice on Home, once from this computer and once through sync, and each branch copy had its own row; rows are keyed by chat id and copies fold into the chat's row with a count." + - "codeaf doctor said nothing about sync; it now reports a busy store, unfinished continues, duplicate rows and lapsed holds, and codeaf doctor --fix repairs what is safe." +--- + +`codeaf doctor --fix` releases unfinished continues this computer started and +retires branch copies whose turns are all already in the main chat, printing a +`fixed:` line for each. It never touches another computer's live hold and never +removes a turn that exists only in what it would delete. A store lock whose +process has ended is already free, so doctor neither reports nor removes one. diff --git a/internal/cellstore/daemon.go b/internal/cellstore/daemon.go index 54c1f9d398..87fc185e22 100644 --- a/internal/cellstore/daemon.go +++ b/internal/cellstore/daemon.go @@ -72,7 +72,7 @@ func (d Daemon) Do(ctx context.Context, t Target, op Op) ([]byte, error) { return nil, fmt.Errorf("engine %s answer interrupted: %w", op.Verb(), err) } if response.Err != "" { - return nil, fmt.Errorf("engine %s: %s", op.Verb(), response.Err) + return nil, classifyStoreBusyError(t.DataDir, d.Socket, fmt.Errorf("engine %s: %s", op.Verb(), response.Err)) } return response.Ok, nil } diff --git a/internal/cellstore/daemon_stuck.go b/internal/cellstore/daemon_stuck.go index 1eaf0a57f4..13a819cb41 100644 --- a/internal/cellstore/daemon_stuck.go +++ b/internal/cellstore/daemon_stuck.go @@ -14,27 +14,32 @@ func (e *SavingStuckError) Error() string { return fmt.Sprintf("saving is stuck; your turns stay on this computer, and saving resumes on its own within %s minutes", idleMinutes) } -func daemonServesSocket(uid, ownUID string, argv []string, socket string) bool { +func daemonSocketArg(uid, ownUID string, argv []string) (string, bool) { if uid != ownUID || len(argv) < 4 || argv[1] != "serve" { - return false + return "", false } name := filepath.Base(argv[0]) if !storageProgramName.MatchString(name) { - return false + return "", false } - found := false + socket := "" for i := 2; i < len(argv); i++ { if argv[i] == "--socket" { - if found || i+1 >= len(argv) || argv[i+1] != socket { - return false + if socket != "" || i+1 >= len(argv) || argv[i+1] == "" { + return "", false } - found = true + socket = argv[i+1] i++ } else if strings.HasPrefix(argv[i], "--socket=") { - return false + return "", false } } - return found + return socket, socket != "" +} + +func daemonServesSocket(uid, ownUID string, argv []string, socket string) bool { + got, ok := daemonSocketArg(uid, ownUID, argv) + return ok && got == socket } var storageProgramName = regexp.MustCompile(`^furrow(?:-[0-9]+\.[0-9]+\.[0-9]+-[0-9a-f]+)?$`) diff --git a/internal/cellstore/store_busy.go b/internal/cellstore/store_busy.go new file mode 100644 index 0000000000..d4cd9772ab --- /dev/null +++ b/internal/cellstore/store_busy.go @@ -0,0 +1,299 @@ +package cellstore + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "strconv" + "strings" + "sync" + + "github.com/Agent-Field/codeaf/internal/filelock" + "github.com/Agent-Field/codeaf/internal/home" +) + +// ErrStoreBusy means another process holds the engine store's writer lock. +var ErrStoreBusy = errors.New("engine store is busy") + +const ( + storeWriterLockName = "writer.lock" + maxLockPayloadBytes = 4 << 10 +) + +// StoreHolder identifies the process holding a store writer lock. +type StoreHolder struct { + PID int `json:"pid,omitempty"` + Command string `json:"command,omitempty"` + Lock string `json:"lock,omitempty"` +} + +// Describe formats the known parts of a holder and answers empty when neither +// command nor pid is known. +func (h StoreHolder) Describe() string { + cmd := strings.TrimSpace(h.Command) + switch { + case cmd != "" && h.PID > 0: + return fmt.Sprintf("%s, pid %d", cmd, h.PID) + case cmd != "": + return cmd + case h.PID > 0: + return fmt.Sprintf("pid %d", h.PID) + default: + return "" + } +} + +// StoreBusyError reports that DataDir's writer lock is held by another process. +type StoreBusyError struct { + DataDir string + Holder StoreHolder + Cause error +} + +func (e *StoreBusyError) Error() string { + if who := e.Holder.Describe(); who != "" { + return fmt.Sprintf("engine store is busy (held by %s)", who) + } + return ErrStoreBusy.Error() +} + +func (e *StoreBusyError) Unwrap() []error { + if e.Cause != nil { + return []error{ErrStoreBusy, e.Cause} + } + return []error{ErrStoreBusy} +} + +// SharedStoreDir returns the default shared engine store directory for the +// active state root. +func SharedStoreDir() string { + return home.Join("v3", "stores", "shared") +} + +// HoldStoreWriter acquires DataDir's writer lock and stamps this process as its +// holder until release runs. +func HoldStoreWriter(dataDir, command string) (func() error, error) { + if strings.TrimSpace(dataDir) == "" { + return nil, errors.New("empty store data directory") + } + if err := os.MkdirAll(dataDir, 0o700); err != nil { + return nil, err + } + lockPath := filepath.Join(dataDir, storeWriterLockName) + file, err := os.OpenFile(lockPath, os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + return nil, err + } + if err := filelock.Lock(file, true, true); err != nil { + _ = file.Close() + if filelock.IsBusy(err) { + holder, _ := InspectStoreBusy(dataDir, "") + return nil, &StoreBusyError{DataDir: dataDir, Holder: holder, Cause: err} + } + return nil, err + } + cmd := strings.TrimSpace(command) + if cmd == "" { + cmd = formatHolderCommand(os.Args) + } + payload, _ := json.Marshal(StoreHolder{PID: os.Getpid(), Command: cmd}) + payload = append(payload, '\n') + _, _ = file.Seek(0, io.SeekStart) + _, _ = file.Write(payload) + _ = file.Truncate(int64(len(payload))) + _ = file.Sync() + + var once sync.Once + var releaseErr error + return func() error { + once.Do(func() { + removeErr := os.Remove(lockPath) + if errors.Is(removeErr, os.ErrNotExist) { + removeErr = nil + } + unlockErr := filelock.Unlock(file) + closeErr := file.Close() + releaseErr = errors.Join(removeErr, unlockErr, closeErr) + }) + return releaseErr + }, nil +} + +// 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) { + if strings.TrimSpace(dataDir) == "" { + return StoreHolder{}, false + } + for _, lockPath := range storeLockCandidates(dataDir, ownSocket) { + holder, busy := probeStoreLockBusy(lockPath, ownSocket) + if busy { + return holder, true + } + } + return StoreHolder{}, false +} + +func classifyStoreBusyError(dataDir, ownSocket string, err error) error { + if err == nil { + return nil + } + var existing *StoreBusyError + if errors.As(err, &existing) { + return err + } + if isEngineStoreBusyText(err.Error()) { + holder, _ := InspectStoreBusy(dataDir, ownSocket) + return &StoreBusyError{DataDir: dataDir, Holder: holder, Cause: err} + } + return err +} + +func isEngineStoreBusyText(text string) bool { + lower := strings.ToLower(text) + for _, needle := range []string{ + "store writer is still busy", + "engine store is busy", + } { + if strings.Contains(lower, needle) { + return true + } + } + return false +} + +func storeLockCandidates(dataDir, ownSocket string) []string { + var paths []string + add := func(matches ...string) { + for _, m := range matches { + if m != "" { + paths = append(paths, m) + } + } + } + add(dataDir + ".lock") + for _, pattern := range []string{ + filepath.Join(dataDir, "*.lock"), + filepath.Join(dataDir, "store-v1", "*.lock"), + filepath.Join(dataDir, "store-v1", "*", "*.lock"), + } { + matches, _ := filepath.Glob(pattern) + add(matches...) + } + v3Dir := filepath.Dir(filepath.Dir(filepath.Clean(dataDir))) + if filepath.Base(filepath.Dir(filepath.Clean(dataDir))) == "stores" { + socks, _ := filepath.Glob(filepath.Join(v3Dir, "engine-*.sock.lock")) + for _, sockLock := range socks { + if ownSocket != "" && filepath.Clean(sockLock) == filepath.Clean(ownSocket+".lock") { + continue + } + // Another engine daemon socket lock only blocks this store when no + // socket is owned here or when inspecting stale locks across v3. + if ownSocket == "" { + add(sockLock) + } + } + } + return paths +} + +func probeStoreLockBusy(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. + sock := strings.TrimSuffix(lockPath, ".lock") + if !heldWithoutSocket(sock) { + return StoreHolder{}, false + } + } + file, err := os.OpenFile(lockPath, os.O_RDWR, 0) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return StoreHolder{}, false + } + file, err = os.Open(lockPath) + if err != nil { + return StoreHolder{}, false + } + } + defer file.Close() + + if err := filelock.Lock(file, true, true); err == nil { + _ = filelock.Unlock(file) + return StoreHolder{}, false + } else if !filelock.IsBusy(err) { + return StoreHolder{}, false + } + + holder := lockHolderFromOS(lockPath) + if holder.PID == os.Getpid() { + return StoreHolder{}, false + } + if holder.PID > 0 && pidServesSocket(holder.PID, ownSocket) { + return StoreHolder{}, false + } + if holder.PID == 0 || holder.Command == "" { + recorded := readLockPayload(file) + if recorded.PID == os.Getpid() { + return StoreHolder{}, false + } + if recorded.PID > 0 && pidServesSocket(recorded.PID, ownSocket) { + return StoreHolder{}, false + } + if holder.PID == 0 { + holder.PID = recorded.PID + } + if holder.Command == "" { + holder.Command = recorded.Command + } + } + if holder.Command == "" && holder.PID > 0 { + holder.Command = describeProcess(holder.PID) + } + holder.Lock = lockPath + return holder, true +} + +func readLockPayload(file *os.File) StoreHolder { + raw, err := io.ReadAll(io.NewSectionReader(file, 0, maxLockPayloadBytes)) + if err != nil { + return StoreHolder{} + } + text := strings.TrimSpace(string(raw)) + if text == "" { + return StoreHolder{} + } + var holder StoreHolder + if json.Unmarshal([]byte(text), &holder) == nil && (holder.PID > 0 || holder.Command != "") { + return holder + } + if pid, err := strconv.Atoi(text); err == nil && pid > 0 { + return StoreHolder{PID: pid} + } + return StoreHolder{} +} + +func formatHolderCommand(argv []string) string { + var clean []string + for _, arg := range argv { + arg = strings.TrimSpace(arg) + if arg != "" { + clean = append(clean, arg) + } + } + if len(clean) == 0 { + return "" + } + prog := filepath.Base(clean[0]) + if storageProgramName.MatchString(prog) { + prog = "furrow" + } + if len(clean) > 1 && !strings.HasPrefix(clean[1], "-") && !strings.Contains(clean[1], string(filepath.Separator)) { + return prog + " " + clean[1] + } + return prog +} diff --git a/internal/cellstore/store_busy_test.go b/internal/cellstore/store_busy_test.go new file mode 100644 index 0000000000..be53065a7b --- /dev/null +++ b/internal/cellstore/store_busy_test.go @@ -0,0 +1,207 @@ +package cellstore + +import ( + "bufio" + "context" + "encoding/json" + "errors" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/cell" + "github.com/Agent-Field/codeaf/internal/filelock" + "github.com/Agent-Field/codeaf/internal/home" +) + +type testBusyOp struct{} + +func (testBusyOp) Verb() string { return "want" } +func (testBusyOp) Args() []string { return []string{"want"} } + +func TestStoreBusyHelperProcess(t *testing.T) { + lockPath := os.Getenv("CODEAF_TEST_HOLD_STORE_LOCK") + if lockPath == "" { + return + } + if err := os.MkdirAll(filepath.Dir(lockPath), 0o700); err != nil { + os.Exit(2) + } + file, err := os.OpenFile(lockPath, os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + os.Exit(3) + } + if err := filelock.Lock(file, true, true); err != nil { + os.Exit(4) + } + if payload := os.Getenv("CODEAF_TEST_HOLD_PAYLOAD"); payload != "" { + _, _ = file.WriteString(payload) + _ = file.Sync() + } + _, _ = os.Stdout.WriteString("LOCKED\n") + _, _ = bufio.NewReader(os.Stdin).ReadBytes('\n') + _ = filelock.Unlock(file) + _ = file.Close() + os.Exit(0) +} + +func startLockHolderSubprocess(t *testing.T, lockPath, payload string) (*exec.Cmd, func()) { + t.Helper() + exe, err := os.Executable() + if err != nil { + t.Fatal(err) + } + cmd := exec.Command(exe, "-test.run=^TestStoreBusyHelperProcess$", "chat") //codeaf:plumbing spawns this test binary to hold an advisory store lock in another process + cmd.Env = append(os.Environ(), + "CODEAF_TEST_HOLD_STORE_LOCK="+lockPath, + "CODEAF_TEST_HOLD_PAYLOAD="+payload, + ) + stdin, err := cmd.StdinPipe() + if err != nil { + t.Fatal(err) + } + stdout, err := cmd.StdoutPipe() + if err != nil { + t.Fatal(err) + } + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + line, err := bufio.NewReader(stdout).ReadString('\n') + if err != nil || strings.TrimSpace(line) != "LOCKED" { + _ = cmd.Process.Kill() + _ = cmd.Wait() + t.Fatalf("helper failed to lock %s: line=%q err=%v", lockPath, line, err) + } + var stopped bool + stop := func() { + if stopped { + return + } + stopped = true + _, _ = stdin.Write([]byte("release\n")) + _ = stdin.Close() + done := make(chan error, 1) + go func() { done <- cmd.Wait() }() + select { + case <-done: + case <-time.After(2 * time.Second): + _ = cmd.Process.Kill() + <-done + } + } + t.Cleanup(stop) + return cmd, stop +} + +func TestStoreBusyReportsHolderWithinSeconds(t *testing.T) { + dataDir := filepath.Join(t.TempDir(), "v3", "stores", "shared") + lockPath := filepath.Join(dataDir, "store-v1", "locks", "pack.lock") + payload := "" + if runtime.GOOS == "windows" { + raw, _ := json.Marshal(StoreHolder{Command: "codeaf chat"}) + payload = string(raw) + } + child, stopChild := startLockHolderSubprocess(t, lockPath, payload) + defer stopChild() + + // A held lock never pre-empts a request that the engine itself handles. + ran := 0 + var engineErr error + tr := Spawn{ + Binary: "/bin/echo", + Run: func(context.Context, string, []string, ...string) ([]byte, error) { + ran++ + if engineErr != nil { + return nil, engineErr + } + return []byte("{}"), nil + }, + } + + if _, err := tr.Do(context.Background(), Target{Tree: t.TempDir(), DataDir: dataDir}, testBusyOp{}); err != nil { + t.Fatalf("normal request failed while lock was held: %v", err) + } + if ran != 1 { + t.Fatalf("engine invocations = %d, want 1", ran) + } + + // Non-busy engine errors are left untouched even when a lock file exists. + engineErr = errors.New("furrow: exit status 1: remote writer lock is already held") + if _, err := tr.Do(context.Background(), Target{Tree: t.TempDir(), DataDir: dataDir}, testBusyOp{}); errors.Is(err, ErrStoreBusy) { + t.Fatalf("remote lock error misclassified as ErrStoreBusy: %v", err) + } + + // When the engine itself returns its busy error, the transport classifies it + // as ErrStoreBusy and names the foreign holder's command and pid. + engineErr = errors.New("furrow: exit status 1: store writer is still busy after waiting 3 seconds; retry the operation") + began := time.Now() + _, err := tr.Do(context.Background(), Target{Tree: t.TempDir(), DataDir: dataDir}, testBusyOp{}) + elapsed := time.Since(began) + + if !errors.Is(err, ErrStoreBusy) { + t.Fatalf("err = %v, want errors.Is(..., ErrStoreBusy)", err) + } + var busy *StoreBusyError + if !errors.As(err, &busy) { + t.Fatalf("err = %T (%v), want *StoreBusyError", err, err) + } + if elapsed > 3*time.Second { + t.Fatalf("busy store classification took %v, want within a few seconds", elapsed) + } + if runtime.GOOS != "windows" { + if busy.Holder.PID != child.Process.Pid { + t.Fatalf("holder pid = %d, want child pid %d", busy.Holder.PID, child.Process.Pid) + } + if busy.Holder.Command == "" { + t.Fatalf("holder command is empty; want child command") + } + } + + // Releasing the foreign writer lock lets the next operation proceed. + stopChild() + engineErr = nil + if _, err := tr.Do(context.Background(), Target{Tree: t.TempDir(), DataDir: dataDir}, testBusyOp{}); err != nil { + t.Fatalf("after lock release, Do returned %v", err) + } +} + +func TestStoreBusyIgnoresHomesOwnDaemon(t *testing.T) { + bin, err := sealingEngine() + if err != nil { + t.Skipf("no owned engine: %v", err) + } + stateRoot := t.TempDir() + t.Setenv(home.EnvVar, stateRoot) + t.Cleanup(StopSpawned) + + work := t.TempDir() + if err := os.WriteFile(filepath.Join(work, "a.txt"), []byte("hello\n"), 0o600); err != nil { + t.Fatal(err) + } + c, err := cell.CreateIn(t.TempDir(), cell.Options{Class: cell.HostBound}) + if err != nil { + t.Fatal(err) + } + e := Engine{Binary: bin, Workspace: work, Transport: Spawn{Binary: bin}} + sealed, err := e.Seal(t.Context(), c, TurnInfo{}) + if err != nil { + t.Fatal(err) + } + + // The home's own furrow serve daemon is the normal writer path and must + // never be reported as a busy-store blocker or pre-empt a spawned verb. + if holder, busy := InspectStoreBusy(e.StoreDir(c), ""); busy { + t.Fatalf("home's own daemon reported as busy holder: %+v", holder) + } + if err := os.WriteFile(filepath.Join(work, "a.txt"), []byte("changed\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := e.Sync(SyncKeys{}, "test").Materialize(t.Context(), c, sealed.Turn.ID); err != nil { + t.Fatalf("Materialize failed while home's own daemon was active: %v", err) + } +} diff --git a/internal/cellstore/store_busy_unix.go b/internal/cellstore/store_busy_unix.go new file mode 100644 index 0000000000..62cea8e336 --- /dev/null +++ b/internal/cellstore/store_busy_unix.go @@ -0,0 +1,227 @@ +//go:build linux || darwin + +package cellstore + +import ( + "bufio" + "context" + "errors" + "os" + "os/exec" + "path/filepath" + "runtime" + "strconv" + "strings" + "syscall" + "time" + + "golang.org/x/sys/unix" + + "github.com/Agent-Field/codeaf/internal/home" +) + +func storeProcessAlive(pid int) bool { + if pid <= 0 { + return false + } + err := syscall.Kill(pid, 0) + return err == nil || errors.Is(err, syscall.EPERM) +} + +func pidServesSocket(pid int, socket string) bool { + uid, argv := storageProcess(context.Background(), pid) + ownUID := strconv.Itoa(os.Getuid()) + if socket != "" && daemonServesSocket(uid, ownUID, argv, socket) { + return true + } + // Each codeaf home has its own private saving daemon; it is the normal + // writer path for this home even when a caller inspects without naming a socket. + served, ok := daemonSocketArg(uid, ownUID, argv) + if !ok { + return false + } + if filepath.Clean(served) == filepath.Clean(socketFor(argv[0])) { + return true + } + return filepath.Dir(filepath.Clean(served)) == filepath.Clean(home.Join("v3")) +} + +func describeProcess(pid int) string { + _, argv := storageProcess(context.Background(), pid) + return formatHolderCommand(argv) +} + +func lockHolderFromOS(lockPath string) StoreHolder { + if runtime.GOOS == "linux" { + return lockHolderLinux(lockPath) + } + return lockHolderDarwin(lockPath) +} + +func lockHolderLinux(lockPath string) StoreHolder { + info, err := os.Stat(lockPath) + if err != nil { + return StoreHolder{} + } + stat, ok := info.Sys().(*syscall.Stat_t) + if !ok { + return StoreHolder{} + } + pid := pidFromProcLocks(uint64(stat.Dev), stat.Ino) + if pid <= 0 { + pid = pidFromProcFDs(info) + } + if pid <= 0 { + return StoreHolder{} + } + return resolveUnixHolder(pid) +} + +func pidFromProcLocks(dev, ino uint64) int { + file, err := os.Open("/proc/locks") + if err != nil { + return 0 + } + defer file.Close() + + wantMajor := unix.Major(dev) + wantMinor := unix.Minor(dev) + ownPID := os.Getpid() + + scanner := bufio.NewScanner(file) + for scanner.Scan() { + fields := strings.Fields(scanner.Text()) + // Standard format: "1: FLOCK ADVISORY WRITE :: 0 EOF" + // Blocked waiters carry "->" after the index ("1: -> FLOCK ..."); skip them. + if len(fields) < 6 || fields[1] == "->" { + continue + } + pid, err := strconv.Atoi(fields[4]) + if err != nil || pid <= 0 || pid == ownPID { + continue + } + parts := strings.Split(fields[5], ":") + if len(parts) != 3 { + continue + } + gotIno, err := strconv.ParseUint(parts[2], 10, 64) + if err != nil || gotIno != ino { + continue + } + maj, err1 := strconv.ParseUint(parts[0], 16, 32) + min, err2 := strconv.ParseUint(parts[1], 16, 32) + if err1 != nil || err2 != nil { + continue + } + if uint32(maj) == wantMajor && uint32(min) == wantMinor { + return pid + } + } + return 0 +} + +func pidFromProcFDs(target os.FileInfo) int { + entries, err := os.ReadDir("/proc") + if err != nil { + return 0 + } + ownUID := uint32(os.Getuid()) + ownPID := os.Getpid() + for _, entry := range entries { + if !entry.IsDir() { + continue + } + pid, err := strconv.Atoi(entry.Name()) + if err != nil || pid <= 0 || pid == ownPID { + continue + } + procDir := filepath.Join("/proc", entry.Name()) + pInfo, err := os.Stat(procDir) + if err != nil { + continue + } + if st, ok := pInfo.Sys().(*syscall.Stat_t); !ok || st.Uid != ownUID { + continue + } + fds, err := os.ReadDir(filepath.Join(procDir, "fd")) + if err != nil { + continue + } + for _, fd := range fds { + fdInfo, err := os.Stat(filepath.Join(procDir, "fd", fd.Name())) + if err == nil && os.SameFile(target, fdInfo) { + return pid + } + } + } + return 0 +} + +func resolveUnixHolder(pid int) StoreHolder { + _, argv := storageProcess(context.Background(), 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) + if len(parentArgv) > 0 { + base := filepath.Base(parentArgv[0]) + if base == "codeaf" || strings.HasPrefix(base, "codeaf-") { + return StoreHolder{PID: ppid, Command: formatHolderCommand(parentArgv)} + } + } + } + } + return StoreHolder{PID: pid, Command: formatHolderCommand(argv)} +} + +func linuxParentPID(pid int) int { + if runtime.GOOS != "linux" { + return 0 + } + raw, err := os.ReadFile(filepath.Join("/proc", strconv.Itoa(pid), "status")) + if err != nil { + return 0 + } + for _, line := range strings.Split(string(raw), "\n") { + if after, ok := strings.CutPrefix(line, "PPid:"); ok { + ppid, _ := strconv.Atoi(strings.TrimSpace(after)) + return ppid + } + } + return 0 +} + +func lockHolderDarwin(lockPath string) StoreHolder { + ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) + defer cancel() + 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{} + } + ownPID := os.Getpid() + var pid int + var cmd string + for _, line := range strings.Split(string(raw), "\n") { + switch { + case strings.HasPrefix(line, "p"): + parsed, err := strconv.Atoi(line[1:]) + if err == nil && parsed > 0 && parsed != ownPID { + pid = parsed + } else { + pid = 0 + } + case strings.HasPrefix(line, "c") && pid > 0 && cmd == "": + cmd = strings.TrimSpace(line[1:]) + } + } + if pid <= 0 { + return StoreHolder{} + } + holder := resolveUnixHolder(pid) + if holder.Command == "" { + holder.Command = cmd + } + return holder +} diff --git a/internal/cellstore/store_busy_windows.go b/internal/cellstore/store_busy_windows.go new file mode 100644 index 0000000000..093a0c1258 --- /dev/null +++ b/internal/cellstore/store_busy_windows.go @@ -0,0 +1,29 @@ +//go:build windows + +package cellstore + +import "golang.org/x/sys/windows" + +const windowsStillActive = 259 + +func storeProcessAlive(pid int) bool { + if pid <= 0 { + return false + } + handle, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid)) + if err != nil { + return false + } + defer windows.CloseHandle(handle) + var code uint32 + if err := windows.GetExitCodeProcess(handle, &code); err != nil { + return false + } + return code == windowsStillActive +} + +func pidServesSocket(int, string) bool { return false } + +func describeProcess(int) string { return "" } + +func lockHolderFromOS(string) StoreHolder { return StoreHolder{} } diff --git a/internal/cellstore/transport.go b/internal/cellstore/transport.go index 947790ae2c..010fb066bb 100644 --- a/internal/cellstore/transport.go +++ b/internal/cellstore/transport.go @@ -66,14 +66,14 @@ func (s Spawn) Do(ctx context.Context, t Target, op Op) ([]byte, error) { if err != nil { return nil, err } + socket := s.Socket + if socket == "" { + socket = socketFor(bin) + } // Seals and durability fences always use codeaf's one daemon. The CLI's // implicit daemon is per data directory and escapes codeaf's child lifecycle; // invoking its direct sync would also miss the retained writer's pending group. if s.Run == nil && (op.Verb() == "seal" || op.Verb() == "sync") { - socket := s.Socket - if socket == "" { - socket = socketFor(bin) - } return (Daemon{Socket: socket, Binary: bin}).Do(ctx, t, op) } argv := append([]string{bin}, op.Args()...) @@ -84,7 +84,8 @@ func (s Spawn) Do(ctx context.Context, t Target, op Op) ([]byte, error) { argv = append(argv, "--timeline", t.Timeline) } environ := append(os.Environ(), dataDirEnv+"="+t.DataDir) - return s.run(ctx, t.Tree, append(environ, opEnv(op)...), argv...) + out, err := s.run(ctx, t.Tree, append(environ, opEnv(op)...), argv...) + return out, classifyStoreBusyError(t.DataDir, socket, err) } // EnvOp is an Op that carries secrets. A spawn hands them to the engine diff --git a/internal/chatlist/collapse_copies_test.go b/internal/chatlist/collapse_copies_test.go new file mode 100644 index 0000000000..fa44ff60b9 --- /dev/null +++ b/internal/chatlist/collapse_copies_test.go @@ -0,0 +1,108 @@ +package chatlist + +import ( + "strings" + "testing" + "time" +) + +func TestCollapseCopiesIntoMainRowAndExplainLoss(t *testing.T) { + rows := []Row{ + { + Cell: "chat-main", + Title: "Refactor router", + Device: "tower", + Status: Running, + Head: "turn-3", + DurableAgo: 2 * time.Minute, + }, + { + Cell: "chat-copy-same", + Title: "Refactor router", + Device: "laptop", + Parent: "chat-main", + Status: Branch, + Head: "turn-3", + OrphanTurns: 1, + DurableAgo: 5 * time.Minute, + }, + { + Cell: "chat-copy-extra", + Title: "Refactor router", + Device: "desk", + Parent: "chat-main", + Status: Branch, + Head: "turn-other", + OrphanTurns: 2, + DurableAgo: 6 * time.Minute, + }, + { + Cell: "orphan-no-parent", + Title: "Standalone branch", + Device: "mac", + Status: Branch, + OrphanTurns: 1, + DurableAgo: 10 * time.Minute, + }, + } + + collapsed, byLocal := CollapseCopies(rows, nil) + if len(byLocal) != 0 { + t.Fatalf("unexpected local copies: %+v", byLocal) + } + if len(collapsed) != 2 { + t.Fatalf("collapsed len = %d, want 2: %+v", len(collapsed), collapsed) + } + main := collapsed[0] + if main.Cell != "chat-main" || len(main.Copies) != 2 { + t.Fatalf("main row copies = %+v, want 2 collapsed copies", main) + } + if !main.Copies[0].Redundant || main.Copies[1].Redundant { + t.Fatalf("copy redundancy = %+v, want [true, false]", main.Copies) + } + + line := CopiesLine(main.Copies) + if !strings.Contains(line, "2 copies") || !strings.Contains(line, "unsent turns set aside") || !strings.Contains(line, "discarding archives 2 turns not in this chat") { + t.Fatalf("non-redundant CopiesLine = %q", line) + } + + onlyRedundant := CopiesLine(main.Copies[:1]) + if !strings.Contains(onlyRedundant, "1 copy") || !strings.Contains(onlyRedundant, "unsent turns set aside") || !strings.Contains(onlyRedundant, "discarding loses nothing") { + t.Fatalf("redundant CopiesLine = %q", onlyRedundant) + } + + takeover := TakeoverLine(main) + if !strings.Contains(takeover, line) { + t.Fatalf("TakeoverLine = %q, want collapsed copy explanation %q", takeover, line) + } +} + +func TestCollapseCopiesAttachesToLocalOnlyChatAndComparesInSame(t *testing.T) { + rows := []Row{ + { + Cell: "branch-1", + Title: "Local main", + Device: "laptop", + Parent: "local-main", + Status: Branch, + OrphanTurns: 3, + BranchRedundant: true, + }, + } + collapsed, byLocal := CollapseCopies(rows, map[string]bool{"local-main": true}) + if len(collapsed) != 0 { + t.Fatalf("collapsed = %+v, want branch folded into local row", collapsed) + } + if got := byLocal["local-main"]; len(got) != 1 || !got[0].Redundant { + t.Fatalf("byLocal = %+v, want 1 redundant copy", byLocal) + } + + a := []Row{{Cell: "c1", Copies: []BranchCopy{{Cell: "b1", OrphanTurns: 1, Redundant: true}}}} + b := []Row{{Cell: "c1", Copies: []BranchCopy{{Cell: "b1", OrphanTurns: 1, Redundant: false}}}} + if Same(a, b) { + t.Fatal("Same ignored Copies difference") + } + if !Same(a, []Row{{Cell: "c1", DurableAgo: time.Minute, Copies: []BranchCopy{{Cell: "b1", OrphanTurns: 1, Redundant: true}}}}) { + t.Fatal("Same rejected identical Copies with different DurableAgo") + } +} diff --git a/internal/chatlist/copy.go b/internal/chatlist/copy.go index cfe5ae3005..76f3e307a9 100644 --- a/internal/chatlist/copy.go +++ b/internal/chatlist/copy.go @@ -179,15 +179,82 @@ func BranchShort(r Row) string { return fmt.Sprintf(branchShort, turns(r.OrphanT func TakeoverLine(r Row) string { line := fmt.Sprintf(takeoverLine, agoWord(r.DurableAgo)) if r.Mine { - return lastHeldHere + "; " + line + line = lastHeldHere + "; " + line + } else { + if r.Pending > 0 { + line += fmt.Sprintf(takeoverMore, turns(r.Pending), r.Device) + } + if r.Status == Off && r.Device != "" { + line = fmt.Sprintf("%s offline; %s", r.Device, line) + } else { + line = holderLead(r) + line + } + } + if extra := CopiesLine(r.Copies); extra != "" { + line += "; " + extra + } + return line +} + +// CopiesLine summarizes branch copies collapsed into a main chat row: how many +// copies exist, what a copy is, and whether discarding loses anything. Empty +// when there are no copies (the emptiness law). +func CopiesLine(copies []BranchCopy) string { + if len(copies) == 0 { + return "" + } + count := copiesWord(len(copies)) + allRedundant, extraTurns := copiesLoss(copies) + if allRedundant { + return fmt.Sprintf("%s (unsent turns set aside; already in this chat, so discarding loses nothing)", count) + } + if extraTurns > 0 { + return fmt.Sprintf("%s (unsent turns set aside; discarding archives %s not in this chat)", count, turns(extraTurns)) + } + return fmt.Sprintf("%s (unsent turns set aside; discarding archives turns not in this chat)", count) +} + +// CopiesShort is the narrow-column spelling for collapsed branch copies. +func CopiesShort(copies []BranchCopy) string { + if len(copies) == 0 { + return "" + } + count := copiesWord(len(copies)) + allRedundant, extraTurns := copiesLoss(copies) + if allRedundant { + return count + " · unsent, loses nothing" } - if r.Pending > 0 { - line += fmt.Sprintf(takeoverMore, turns(r.Pending), r.Device) + if extraTurns > 0 { + return fmt.Sprintf("%s · unsent, %s not here", count, turns(extraTurns)) } - if r.Status == Off && r.Device != "" { - return fmt.Sprintf("%s offline; %s", r.Device, line) + return count + " · unsent turns" +} + +// CopiesCount is the count alone, for a row whose own state already fills the +// narrow spelling and has room for nothing more about its copies. +func CopiesCount(copies []BranchCopy) string { + if len(copies) == 0 { + return "" + } + return copiesWord(len(copies)) +} + +func copiesWord(n int) string { + if n == 1 { + return "1 copy" + } + return fmt.Sprintf("%d copies", n) +} + +func copiesLoss(copies []BranchCopy) (allRedundant bool, extraTurns uint32) { + allRedundant = true + for _, c := range copies { + if !c.Redundant { + allRedundant = false + extraTurns += c.OrphanTurns + } } - return holderLead(r) + line + return allRedundant, extraTurns } // agoWord spells an age in its largest whole unit, so an hour reads `1h` and diff --git a/internal/chatlist/rows.go b/internal/chatlist/rows.go index 49a49b2d8a..1086de8c94 100644 --- a/internal/chatlist/rows.go +++ b/internal/chatlist/rows.go @@ -6,6 +6,7 @@ package chatlist import ( "context" + "slices" "sort" "strings" "time" @@ -25,6 +26,16 @@ const ( Branch Status = "branch" // OrphanTurns > 0, not archived ) +// BranchCopy is one unsent branch copy set aside for a chat during a handoff. +// Redundant reports that every turn in this copy is already in the main chat, +// so archiving it loses nothing. +type BranchCopy struct { + Cell string + Device string + OrphanTurns uint32 + Redundant bool +} + // Row is one chat as the list shows it. type Row struct { Cell, Title, Device, DeviceID, Parent string @@ -44,6 +55,11 @@ type Row struct { // HolderRemoved says the last holder was removed from the person's // devices: its lease reads as released and it is never offered (#2072). HolderRemoved bool + // BranchRedundant reports that a branch row's saved head is already in its + // parent chat's turn history, so archiving the branch loses nothing. + BranchRedundant bool + // Copies are branch copies of this chat collapsed onto its single Home row. + Copies []BranchCopy } // Quiet is the row of a chat whose live holder has gone: a lease that still @@ -210,3 +226,116 @@ func nameOr(open Opener, sealed, fallback string) string { } return fallback } + +// CollapseCopies folds branch copies with a known parent chat onto that main +// chat's single row (or returns them under byLocal when the main chat is known +// only locally on Home). Standalone branch rows with no parent stay on their +// own row so orphan branches remain visible and actionable. +func CollapseCopies(rows []Row, localIDs map[string]bool) ([]Row, map[string][]BranchCopy) { + if len(rows) == 0 { + return nil, nil + } + byCell := make(map[string]int, len(rows)) + mainIdx := make(map[string]int, len(rows)) + for i, r := range rows { + byCell[r.Cell] = i + if r.Status != Branch { + mainIdx[r.Cell] = i + } + } + rootOf := func(r Row) string { + cur := strings.TrimSpace(r.Parent) + for hops := 0; cur != "" && hops < 16; hops++ { + idx, ok := byCell[cur] + if !ok || rows[idx].Status != Branch { + break + } + next := strings.TrimSpace(rows[idx].Parent) + if next == "" || next == cur { + break + } + cur = next + } + return cur + } + orphanCounts := map[string]int{} + firstBranchIdx := map[string]int{} + for i, r := range rows { + if r.Status != Branch { + continue + } + root := rootOf(r) + if root == "" { + continue + } + if _, ok := mainIdx[root]; ok || localIDs[root] { + continue + } + orphanCounts[root]++ + if _, seen := firstBranchIdx[root]; !seen { + firstBranchIdx[root] = i + } + } + + skip := make([]bool, len(rows)) + attached := make(map[int][]BranchCopy) + var byLocal map[string][]BranchCopy + + for i, r := range rows { + if r.Status != Branch { + continue + } + root := rootOf(r) + if root == "" { + continue + } + if idx, ok := mainIdx[root]; ok { + redundant := r.BranchRedundant || (r.Head != "" && r.Head == rows[idx].Head) + attached[idx] = append(attached[idx], BranchCopy{ + Cell: r.Cell, + Device: r.Device, + OrphanTurns: r.OrphanTurns, + Redundant: redundant, + }) + skip[i] = true + continue + } + if localIDs[root] { + if byLocal == nil { + byLocal = make(map[string][]BranchCopy) + } + byLocal[root] = append(byLocal[root], BranchCopy{ + Cell: r.Cell, + Device: r.Device, + OrphanTurns: r.OrphanTurns, + Redundant: r.BranchRedundant, + }) + skip[i] = true + continue + } + if orphanCounts[root] > 1 { + if idx := firstBranchIdx[root]; idx != i { + redundant := r.BranchRedundant || (r.Head != "" && r.Head == rows[idx].Head) + attached[idx] = append(attached[idx], BranchCopy{ + Cell: r.Cell, + Device: r.Device, + OrphanTurns: r.OrphanTurns, + Redundant: redundant, + }) + skip[i] = true + } + } + } + + out := make([]Row, 0, len(rows)) + for i, r := range rows { + if skip[i] { + continue + } + if more := attached[i]; len(more) > 0 { + r.Copies = append(slices.Clone(r.Copies), more...) + } + out = append(out, r) + } + return out, byLocal +} diff --git a/internal/chatlist/same.go b/internal/chatlist/same.go index daf824bf74..f2adff5ca8 100644 --- a/internal/chatlist/same.go +++ b/internal/chatlist/same.go @@ -1,5 +1,10 @@ package chatlist +import ( + "slices" + "time" +) + // Same reports whether two listings say the same thing about the chats. // // DurableAgo is left out on purpose: it is measured against the directory's @@ -13,15 +18,43 @@ func Same(a, b []Row) bool { return false } for i := range a { - if timeless(a[i]) != timeless(b[i]) { + if timeless(a[i]) != timeless(b[i]) || !slices.Equal(a[i].Copies, b[i].Copies) { return false } } return true } -// timeless is a row with the read-time-dependent age taken out. -func timeless(r Row) Row { - r.DurableAgo = 0 - return r +type timelessRow struct { + cell, title, device, deviceID, parent string + head string + status Status + durableAt int64 + pending, orphanTurns uint32 + fence uint64 + mine, released bool + releasedAgo time.Duration + holderRemoved, branchRedundant bool +} + +// timeless is a row with the read-time-dependent age and slice fields taken out. +func timeless(r Row) timelessRow { + return timelessRow{ + cell: r.Cell, + title: r.Title, + device: r.Device, + deviceID: r.DeviceID, + parent: r.Parent, + head: r.Head, + status: r.Status, + durableAt: r.DurableAt, + pending: r.Pending, + orphanTurns: r.OrphanTurns, + fence: r.Fence, + mine: r.Mine, + released: r.Released, + releasedAgo: r.ReleasedAgo, + holderRemoved: r.HolderRemoved, + branchRedundant: r.BranchRedundant, + } } diff --git a/internal/manual/chat/continuing-a-chat-on-another-computer.md b/internal/manual/chat/continuing-a-chat-on-another-computer.md index 99302b64b2..52c1876b1f 100644 --- a/internal/manual/chat/continuing-a-chat-on-another-computer.md +++ b/internal/manual/chat/continuing-a-chat-on-another-computer.md @@ -2,7 +2,7 @@ To continue on this machine, select the chat on home and choose `continue here`. When your paired computer is offline or has let go of its latest chat, home can ask `Continue tower's latest chat here?`: `alt+c` continues immediately when there is no local choice to make, and `alt+x` dismisses the offer. Moving chats is available by default on Linux, macOS and native Windows with a compatible engine. Older chats become movable through independent copies after pairing. -## Continue on another computer — why can't I type in this chat, my message did not send and home opened with a question +## Continue on another computer — continuing a chat on another computer, why can't I type in this chat, my message did not send and home opened with a question Moving chats and pairing are available by default on Linux, macOS and native Windows on every build with a compatible engine. Pair both computers first. @@ -184,18 +184,19 @@ only shows it` notice still applies. When unsent turns were kept as a separate copy, reopening that copy checks who holds the copy itself. Another computer may still hold the original chat. The previous window finishes saving and stops syncing before the reopened copy starts its own sync, so a late save cannot change which copy it opens. Taking that copy elsewhere still stops new tool work in its older window. -## Continue here did nothing — another device continued first or the sync service is unreachable +## Continue here does nothing — continue here did nothing, the engine store is busy, or a chat is already continuing here Moving chats and pairing are available by default on Linux, macOS and native Windows on every build with a compatible engine. Pair both computers first. -If nothing happens when you press `Continue here`, the cause is one of these: +While a chat is moving here, the card says `continuing here…`. Pressing `continue here` again before it finishes says `a chat is already continuing here; wait for it to open` on the card instead of doing nothing. Every failure is shown where you are looking — on Home's card with `1 retry` and `2 leave it there`, or inside the open chat window — never only on Home's bottom line after the card closes: - `another device continued this chat first`: another device was faster. The row goes back to `running on `. Nothing changed. - `other machines unreachable`: this device cannot reach the service. Nothing can be moved until it can. -- `could not continue this chat here`: the move failed. The row stays where it was; try again. +- `could not continue this chat here because the engine store is busy` or `could not continue this chat here because the engine store is busy (held by codeaf chat, pid 4321)`: another `codeaf` process holds this computer's engine store writer. Close or finish that process, then choose `retry`. +- `could not continue this chat here`: the move failed. The row stays where it was; choose `retry` or try again. - `could not continue this chat here because other machines are unreachable; your words are still in the box`: the service went away during the move. Nothing was sent and the chat stays where it was; send again once the other machines are reachable. - `taking an open chat from another computer waits for a sync service update; all else works, so close it there and continue here` means the chat is open on another computer and the sync service is too old to take it from there safely. Everything else works, including continuing a chat the other computer has closed. Close the chat on that computer, then choose `continue here` again. -- With no way to continue set up, an off row has no `continue here`: enter says why. +- Under `codeaf chat --host`, `continue here` is not offered and a chat the remote host already holds never shows as held elsewhere. With no way to continue set up, an off row has no `continue here`: enter says why. ## Taking a chat back, and two computers racing for the same chat — who wins, and what the loser sees @@ -398,3 +399,22 @@ A new chat can send its first message normally. A sleeping computer can also res If something changes the chat's files while a move is putting them in place, codeaf stops the move and lets go of the chat. It says the move could not finish and that your words are still in the box. The window keeps showing the chat or returns to Home, and it cannot send on the old copy. Try **continue here** again. Every failed move keeps the captured draft, including its whitespace, caret, pasted text and attachments, even if the old session closed during the move. Losing a race, a connection failure or a refused move does the same. A failed move never sends those words by itself. + +## Repair sync — codeaf doctor and codeaf doctor --fix for unfinished continues, duplicate rows and a busy engine store + +Moving chats and pairing are available by default on Linux, macOS and native Windows on every build with a compatible engine. Pair both computers first. + +Run `codeaf doctor` from a terminal to inspect this computer's engine store and cross-computer sync state without changing anything. When an issue is present, `engine store` and `sync` rows report: + +- `busy · held by , pid `: another live process holds the engine store writer. +- `unfinished continue · chat `: a continue started on this computer never completed or let go. +- `duplicate rows · chat (2 local folders)`: more than one local folder on disk shares one chat id. +- `duplicate rows · chat (branch copy , all turns in main chat)` or `duplicate rows · chat (branch copy , 3 turns not in main chat)`: branch copies exist for a chat. +- `lapsed hold on this computer · chat `: an expired hold from this computer would raise a question on open. + +Run `codeaf doctor --fix` to repair what is safe. It prints one line per repair: + +- `fixed: released unfinished continue for chat ` +- `fixed: retired duplicate branch copy of chat ` + +`codeaf doctor --fix` never touches another computer's live hold, never disturbs a live store writer (quit the program `busy` names, or wait for it, and continue again), and never removes a branch copy or `.taking` scratch folder that holds a turn not already in the main chat. diff --git a/internal/manual/chat/home.md b/internal/manual/chat/home.md index e37a649025..e0d612ac1b 100644 --- a/internal/manual/chat/home.md +++ b/internal/manual/chat/home.md @@ -528,6 +528,15 @@ not offered on this build**. The row says ` turns from : open separately / discard`. If the other machines cannot be reached, enter says `other machines unreachable` and does nothing. +## Why do I see the same chat twice on Home — the same chat twice on Home, one row per chat id and collapsed branch copies + +Home keys every conversation row by the chat's id rather than its transcript path, both directly on this computer and over `codeaf chat --host`. A chat known both locally on disk and through sync appears once on Home instead of twice. Under `codeaf chat --host`, `continue here` is not offered and a chat the remote host already holds never appears a second time as held elsewhere. + +When a chat also has kept branch copies, Home collapses those copies into the main chat's row instead of listing separate duplicate rows for the same chat. The row and its continue card count the copies, say what a copy is, and state whether discarding loses anything: + +- `1 copy (unsent turns set aside; already in this chat, so discarding loses nothing)` — or `1 copy · unsent, loses nothing` in a narrow column — when every turn in the copy is already in the main chat. +- `2 copies (unsent turns set aside; discarding archives 3 turns not in this chat)` — or `2 copies · unsent, 3 turns not here` in a narrow column — when a copy holds turns that are not in the main chat. + ## Why does home show a session id for an untitled chat, untitled conversation, new conversation **A conversation nothing has named yet reads `new conversation`** on home's sessions diff --git a/internal/manual/chat/running-from-the-terminal.md b/internal/manual/chat/running-from-the-terminal.md index bfe2e32354..20122d9a56 100644 --- a/internal/manual/chat/running-from-the-terminal.md +++ b/internal/manual/chat/running-from-the-terminal.md @@ -1592,7 +1592,7 @@ a negative count is refused too. The command's own usage follows the line, and t short block of labelled rows. It needs no key and spends nothing. ``` -codeaf doctor [--db path] +codeaf doctor [--db path] [--fix] ``` Every row is labelled in the words a developer would search for: @@ -1624,6 +1624,12 @@ 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. +## 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. + +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 `codeaf help` is a third spelling of `codeaf --help`, beside `-h`, and all three print the diff --git a/internal/manual/chat_test.go b/internal/manual/chat_test.go index 593915fb89..d0ad674c6b 100644 --- a/internal/manual/chat_test.go +++ b/internal/manual/chat_test.go @@ -50,6 +50,12 @@ func TestTheChatManualAnswersTheQuestionsPeopleAsk(t *testing.T) { // R25/R29 keep older-client refusals and failed quit saves reachable. {"why can an older computer list chats but not continue a chat that is still running", "continuing-a-chat-on-another-computer"}, {"what happens if the final save fails during quit", "continuing-a-chat-on-another-computer"}, + // #2070 keeps continue-here troubleshooting, duplicate Home rows and sync repair reachable. + {"continue here does nothing", "continuing-a-chat-on-another-computer"}, + {"the same chat twice on Home", "home"}, + {"why do I see the same chat twice on Home", "home"}, + {"repair sync", "continuing-a-chat-on-another-computer"}, + {"how do I repair sync", "continuing-a-chat-on-another-computer"}, // C29.4/C30.2/C31.5 must be reachable in the person's own words. {"can I rewind an old chat on my new laptop", "continuing-a-chat-on-another-computer"}, {"I quit on my laptop and opened my desktop", "continuing-a-chat-on-another-computer"}, diff --git a/internal/tui3/app.go b/internal/tui3/app.go index 48f7b00c09..cb8a4de520 100644 --- a/internal/tui3/app.go +++ b/internal/tui3/app.go @@ -1485,8 +1485,15 @@ type app struct { homeTakenSaid func() tea.Cmd // homeTaking keeps a retiring session on its own continuation until arrival. homeTaking string + // homeTakeSeq numbers takeovers so a cancelled attempt's late answer is ignored. + homeTakeSeq uint64 + // homeTakeCancel stops the in-flight takeover context when the person cancels. + homeTakeCancel context.CancelFunc // homeTakeDraft moves the source composer only after the arriving door opens. homeTakeDraft *homeTakenMsg + // continueAskSeq gives each surface card its own token so re-raising inside + // an answer closure is not wiped by closeQuestion. + continueAskSeq uint64 // teamsDisk is where the teams are kept and the edits not yet written // there (teamseam.go). teamsDisk teamsDisk diff --git a/internal/tui3/continue_2070_test.go b/internal/tui3/continue_2070_test.go new file mode 100644 index 0000000000..69b21a38b2 --- /dev/null +++ b/internal/tui3/continue_2070_test.go @@ -0,0 +1,597 @@ +package tui3 + +import ( + "bufio" + "context" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + tea "charm.land/bubbletea/v2" + "github.com/Agent-Field/codeaf/internal/cellstore" + "github.com/Agent-Field/codeaf/internal/chatlist" + machine "github.com/Agent-Field/codeaf/internal/preflight" + "github.com/Agent-Field/codeaf/internal/session" +) + +type busyProbeOp struct{} + +func (busyProbeOp) Verb() string { return "want" } +func (busyProbeOp) Args() []string { return []string{"want"} } + +func TestHoldStoreWriterHelperProcess(t *testing.T) { + storeDir := os.Getenv("CODEAF_TEST_HOLD_STORE_DIR") + if storeDir == "" { + return + } + release, err := cellstore.HoldStoreWriter(storeDir, "codeaf chat") + if err != nil { + os.Exit(2) + } + _, _ = os.Stdout.WriteString("LOCKED\n") + _, _ = bufio.NewReader(os.Stdin).ReadBytes('\n') + _ = release() + os.Exit(0) +} + +func startForeignStoreWriter(t *testing.T, storeDir string) func() { + t.Helper() + exe, err := os.Executable() + if err != nil { + t.Fatal(err) + } + cmd := exec.Command(exe, "-test.run=^TestHoldStoreWriterHelperProcess$") //codeaf:plumbing spawns this test binary to hold the engine store writer lock in another process + cmd.Env = append(os.Environ(), "CODEAF_TEST_HOLD_STORE_DIR="+storeDir) + stdin, err := cmd.StdinPipe() + if err != nil { + t.Fatal(err) + } + stdout, err := cmd.StdoutPipe() + if err != nil { + t.Fatal(err) + } + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + line, err := bufio.NewReader(stdout).ReadString('\n') + if err != nil || strings.TrimSpace(line) != "LOCKED" { + _ = cmd.Process.Kill() + _ = cmd.Wait() + t.Fatalf("helper failed to hold store %s: line=%q err=%v", storeDir, line, err) + } + var stopped bool + stop := func() { + if stopped { + return + } + stopped = true + _, _ = stdin.Write([]byte("release\n")) + _ = stdin.Close() + done := make(chan error, 1) + go func() { done <- cmd.Wait() }() + select { + case <-done: + case <-time.After(2 * time.Second): + _ = cmd.Process.Kill() + <-done + } + } + t.Cleanup(stop) + return stop +} + +type scriptedRemoteSource struct { + rows []chatlist.Row +} + +func (s *scriptedRemoteSource) Rows(context.Context) ([]chatlist.Row, error) { + out := make([]chatlist.Row, len(s.rows)) + copy(out, s.rows) + return out, nil +} + +func stepKeyNoDrain(t *testing.T, a *app, k string) tea.Cmd { + t.Helper() + _, cmd := a.Update(key(k)) + return cmd +} + +func collectBatchMsgs(cmd tea.Cmd) []tea.Msg { + if cmd == nil { + return nil + } + msg := cmd() + if msg == nil { + return nil + } + if batch, ok := msg.(tea.BatchMsg); ok { + var out []tea.Msg + for _, sub := range batch { + out = append(out, collectBatchMsgs(sub)...) + } + return out + } + return []tea.Msg{msg} +} + +func countHomeRowsForID(a *app, id string) int { + n := 0 + for _, line := range a.home.lines { + switch { + case line.kind == homeMachineRow && line.remote != nil && (line.remote.Cell == id || line.remote.Parent == id): + n++ + case line.kind == homeSession && homeSessionID(line.row) == id: + n++ + } + } + return n +} + +// Contract 1: In one window, continuing three different chats last written on +// another computer, one after another, each shows `continuing here…` and opens +// that chat with its full history. +func TestContinueThreeRemoteChatsInOneWindowShowsContinuingAndFullHistory(t *testing.T) { + lab := newHomeLab(t) + workspace := lab.workspace("project") + initial := lab.session("project", "0000000000000000", "Local start", workspace, time.Now().Add(-time.Hour)) + + type remoteChat struct { + id string + title string + device string + history []session.DisplayEntry + file string + } + remotes := []remoteChat{ + { + id: "0000000000000101", + title: "Alpha migration", + device: "tower", + history: []session.DisplayEntry{ + {Role: "user", Text: "alpha user question"}, + {Role: "assistant", Text: "alpha assistant answer", Answer: true}, + }, + }, + { + id: "0000000000000102", + title: "Beta indexing", + device: "laptop", + history: []session.DisplayEntry{ + {Role: "user", Text: "beta first turn"}, + {Role: "assistant", Text: "beta full response", Answer: true}, + }, + }, + { + id: "0000000000000103", + title: "Gamma release", + device: "studio", + history: []session.DisplayEntry{ + {Role: "user", Text: "gamma ship checklist"}, + {Role: "assistant", Text: "gamma complete verification", Answer: true}, + }, + }, + } + + src := &scriptedRemoteSource{} + byID := map[string]*remoteChat{} + for i := range remotes { + rc := &remotes[i] + byID[rc.id] = rc + src.rows = append(src.rows, chatlist.Row{ + Cell: rc.id, + Title: rc.title, + Device: rc.device, + DeviceID: "dev_" + rc.device, + Status: chatlist.Off, + DurableAgo: time.Duration(i+1) * time.Minute, + }) + } + + a := lab.app(initial) + a.width, a.height = 160, 48 + byFile := map[string][]session.DisplayEntry{} + a.open = func(where, transcript string) (Conversation, error) { + return Conversation{ + Agent: &switchAgent{fakeAgent: &fakeAgent{model: "m", past: byFile[transcript]}}, + SessionFile: transcript, + Workspace: where, + Resumed: true, + }, nil + } + + taker := &fakeTaker{} + taker.on = func(cellID string) { + rc := byID[cellID] + if rc == nil { + t.Fatalf("unexpected cell %q", cellID) + } + rc.file = lab.session("project", rc.id, rc.title, workspace, time.Now()) + byFile[rc.file] = rc.history + taker.taken = Taken{ + Fence: uint64(len(taker.cells)), + Transcript: rc.file, + Resume: machine.Resume{From: rc.device, Now: workspace}, + } + for i := range src.rows { + if src.rows[i].Cell == cellID { + src.rows[i].Status = chatlist.Here + src.rows[i].Mine = true + } + } + } + a.machines = src + a.taker = taker + + for i := range remotes { + rc := &remotes[i] + drain(t, a, a.openHome()) + a.machinesAsking = false + drain(t, a, a.askMachines()) + + standOn(t, a, rc.id) + _ = stepKeyNoDrain(t, a, "1") + takeCmd := stepKeyNoDrain(t, a, "enter") + + if a.homeTaking != rc.id { + t.Fatalf("%s: homeTaking = %q immediately after confirm, want %q", rc.id, a.homeTaking, rc.id) + } + if text := homeText(a); !strings.Contains(text, continuing) { + t.Fatalf("%s: Home did not show %q while take ran:\n%s", rc.id, continuing, text) + } + + drain(t, a, takeCmd) + + if a.homeTaking != "" { + t.Fatalf("%s: homeTaking = %q after chat opened, want empty", rc.id, a.homeTaking) + } + if a.at(pageHome) { + t.Fatalf("%s: stayed on Home instead of opening arrived chat:\n%s", rc.id, homeText(a)) + } + if a.file != rc.file { + t.Fatalf("%s: opened %q, want %q", rc.id, a.file, rc.file) + } + chatLines, _, _ := a.chatFrameLines(a.width, a.height) + rendered := plain(strings.Join(chatLines, "\n")) + for _, entry := range rc.history { + if !strings.Contains(rendered, entry.Text) { + t.Fatalf("%s: opened chat missing history line %q:\n%s", rc.id, entry.Text, rendered) + } + } + } +} + +// Contract 1: homeTaking is cleared on EVERY ending of a take (homeOpenLater, +// bringForward, failure, cancel), a press while a take is already running says +// so in one line on the card instead of doing nothing, and a take's failure is +// shown where the person is looking (the card on Home or notes in the chat). +func TestHomeTakingClearedOnEveryEndingAndBusyPressExplainsOnCard(t *testing.T) { + t.Run("homeOpenLater and bringForward and cancel", func(t *testing.T) { + lab := newHomeLab(t) + workspace := lab.workspace("project") + first := lab.session("project", "0000000000000201", "First", workspace, time.Now().Add(-2*time.Minute)) + second := lab.session("project", "0000000000000202", "Second", workspace, time.Now().Add(-time.Minute)) + + src := &scriptedRemoteSource{ + rows: []chatlist.Row{ + {Cell: "0000000000000202", Title: "Second", Device: "desk", Status: chatlist.Off, DurableAgo: time.Minute}, + {Cell: "0000000000000201", Title: "First", Device: "desk", Status: chatlist.Off, DurableAgo: 2 * time.Minute}, + }, + } + a := lab.app(first) + a.width, a.height = 160, 48 + a.machines = src + taker := &fakeTaker{taken: Taken{Fence: 1, Transcript: second, Resume: machine.Resume{From: "desk", Now: workspace}}} + a.taker = taker + + // 1. Success through homeOpenLater parks `first` in a.behind and opens `second`. + drain(t, a, a.openHome()) + drain(t, a, a.askMachines()) + standOn(t, a, "0000000000000202") + confirm(t, a) + if a.homeTaking != "" || a.file != second { + t.Fatalf("after homeOpenLater: homeTaking=%q file=%q", a.homeTaking, a.file) + } + + // 2. Success through bringForward (`first` is held in a.behind and AlreadyHere is true). + taker.taken = Taken{Fence: 2, AlreadyHere: true, Transcript: first, Resume: machine.Resume{Now: workspace}} + drain(t, a, a.openHome()) + a.machinesAsking = false + drain(t, a, a.askMachines()) + standOn(t, a, "0000000000000201") + confirm(t, a) + if a.homeTaking != "" || a.file != first { + t.Fatalf("after bringForward: homeTaking=%q file=%q", a.homeTaking, a.file) + } + + // 3. While a take is already running, pressing continue on a card says so on the card, + // and cancelling clears homeTaking and ignores the late message. + src.rows = []chatlist.Row{ + {Cell: "0000000000000203", Title: "Third", Device: "desk", Status: chatlist.Off, DurableAgo: time.Minute}, + {Cell: "0000000000000204", Title: "Fourth", Device: "desk", Status: chatlist.Off, DurableAgo: 2 * time.Minute}, + } + drain(t, a, a.openHome()) + a.machinesAsking = false + drain(t, a, a.askMachines()) + + standOn(t, a, "0000000000000203") + _ = stepKeyNoDrain(t, a, "1") + inFlight := stepKeyNoDrain(t, a, "enter") + if a.homeTaking != "0000000000000203" { + t.Fatalf("expected in-flight take, got %q", a.homeTaking) + } + + // Press enter on another row while the first take is still running. + standOn(t, a, "0000000000000204") + card := strings.Join(cardRows(a), "\n") + if !strings.Contains(card, continueAlreadyRunning) { + t.Fatalf("busy press did not explain on the card:\n%s\nfull Home:\n%s", card, homeText(a)) + } + + // First esc closes the card; second esc cancels the in-flight take. + drive(t, a, key("esc")) + drive(t, a, key("esc")) + if a.homeTaking != "" { + t.Fatalf("esc did not clear homeTaking: %q", a.homeTaking) + } + // Late arrival from cancelled command is ignored. + for _, msg := range collectBatchMsgs(inFlight) { + _, _ = a.Update(msg) + } + if a.homeTaking != "" { + t.Fatalf("late cancelled message dirtied homeTaking: %q", a.homeTaking) + } + + // 4. Cancel during Phase 2 (conversationOpening) also clears homeTaking. + thirdFile := lab.session("project", "0000000000000203", "Third", workspace, time.Now()) + taker.taken = Taken{Fence: 3, Transcript: thirdFile, Resume: machine.Resume{From: "desk", Now: workspace}} + standOn(t, a, "0000000000000203") + _ = stepKeyNoDrain(t, a, "1") + phase1 := stepKeyNoDrain(t, a, "enter") + takenMsg := phase1().(homeTakenMsg) + _ = a.tookTakeover(takenMsg) + if !a.conversationOpening { + t.Fatal("expected Phase 2 conversationOpening") + } + a.cancelConversationOpening() + if a.homeTaking != "" { + t.Fatalf("cancelConversationOpening left homeTaking=%q", a.homeTaking) + } + }) + + t.Run("failure shown on card on Home and in notes in chat", func(t *testing.T) { + taker := &fakeTaker{err: errors.New("disk write failed")} + a := continueHome(t, taker, continueRows, 160) + standOn(t, a, "c-off") + confirm(t, a) + if a.homeTaking != "" { + t.Fatalf("failed take left homeTaking=%q", a.homeTaking) + } + // The card itself must remain open with the failure reason and retry. + card := strings.Join(cardRows(a), "\n") + if !strings.Contains(card, continueFails) || !strings.Contains(card, continueRetry) { + t.Fatalf("failure not shown on card with retry:\n%s", card) + } + + // Failure while looking at the chat lands in chat notes and clears homeTaking. + a.closeHome() + cmd := a.takeRemote(chatlist.Row{Cell: "c-off", Title: "Port the picker", Device: "host-a", Status: chatlist.Off}) + drain(t, a, cmd) + if a.homeTaking != "" { + t.Fatalf("chat-page failed take left homeTaking=%q", a.homeTaking) + } + if notes := strings.Join(noteTexts(a), "\n"); !strings.Contains(notes, continueFails) { + t.Fatalf("chat-page failed take missing from chat notes: %q", notes) + } + }) +} + +// Contract 2: When the engine store's writer is busy, continuing says within a +// few seconds that the store is busy and which program holds it (pid and +// command), offers retry, and never shows the generic "could not be put in this +// folder" sentence. +func TestBusyStoreNamedWithHolderAndOffersRetry(t *testing.T) { + storeDir := filepath.Join(t.TempDir(), "v3", "stores", "shared") + stopWriter := startForeignStoreWriter(t, storeDir) + defer stopWriter() + + transport := cellstore.Spawn{ + Binary: "/bin/echo", + Run: func(context.Context, string, []string, ...string) ([]byte, error) { + return nil, errors.New("furrow: exit status 1: store writer is still busy after waiting 3 seconds; retry the operation") + }, + } + _, storeErr := transport.Do(context.Background(), cellstore.Target{Tree: t.TempDir(), DataDir: storeDir}, busyProbeOp{}) + if !errors.Is(storeErr, cellstore.ErrStoreBusy) { + t.Fatalf("transport.Call err = %v, want errors.Is(_, cellstore.ErrStoreBusy)", storeErr) + } + + lab := newHomeLab(t) + workspace := lab.workspace("project") + initial := lab.session("project", "0000000000000300", "Start", workspace, time.Now().Add(-time.Hour)) + arrived := lab.session("project", "c-off", "Port the picker", workspace, time.Now()) + + a := lab.app(initial) + a.width, a.height = 160, 48 + a.machines = continueRows + taker := &fakeTaker{err: fmt.Errorf("handoff: %w", storeErr)} + a.taker = taker + + start := time.Now() + drain(t, a, a.openHome()) + drain(t, a, a.askMachines()) + standOn(t, a, "c-off") + confirm(t, a) + if elapsed := time.Since(start); elapsed > 5*time.Second { + t.Fatalf("busy store refusal took %s, want within a few seconds", elapsed) + } + + card := strings.Join(strings.Fields(strings.Join(cardRows(a), " ")), " ") + screen := homeText(a) + if strings.Contains(screen, "could not be put in this folder") { + t.Fatalf("generic folder error shown for busy store:\n%s", screen) + } + exeBase := filepath.Base(os.Args[0]) + for _, want := range []string{ + "could not continue this chat here because the engine store is busy (held by " + exeBase + ", pid ", + continueRetry, + } { + if !strings.Contains(card, want) { + t.Fatalf("busy store card missing %q:\n%s", want, card) + } + } + + // Release the foreign store lock and choose retry on the card. + stopWriter() + taker.err = nil + taker.taken = Taken{Fence: 1, Transcript: arrived, Resume: machine.Resume{From: "host-a", Now: workspace}} + + confirm(t, a) + if a.homeTaking != "" || a.at(pageHome) || a.file != arrived { + t.Fatalf("retry after releasing busy store did not open chat: homeTaking=%q atHome=%v file=%q", a.homeTaking, a.at(pageHome), a.file) + } +} + +// Contract 3: Under `--host`, `continue here` is not offered and a chat the +// remote host already holds never shows as held elsewhere or duplicated. +func TestHostedSurfaceNeverOffersContinueOrShowsHeldElsewhere(t *testing.T) { + lab := newHomeLab(t) + workspace := lab.workspace("project") + bChat := lab.session("project", heldCell, "Host B Chat", workspace, time.Now()) + + a := lab.app(bChat) + a.host = "host-b" + a.world = func() (session.World, bool) { return session.ReadWorld(a.placesRoot()), true } + a.width, a.height = 160, 48 + taker := &fakeTaker{} + src := &flipping{status: chatlist.Running} + a.machines = src + a.taker = taker + // Even if stale machine rows exist, hosted Home ignores them consistently. + a.machineRead.rows = []chatlist.Row{ + {Cell: heldCell, Title: "Host B Chat", Device: "host-b", Status: chatlist.Running, DurableAgo: time.Minute}, + {Cell: "other-remote", Title: "Other Remote", Device: "laptop", Status: chatlist.Off, DurableAgo: 2 * time.Minute}, + } + + drain(t, a, a.openHome()) + if cmd := a.pollMachines(a.now()); cmd != nil { + t.Fatalf("hosted pollMachines returned non-nil command") + } + if cmd := a.askMachines(); cmd != nil { + drain(t, a, cmd) + } + if _, elsewhere := a.heldElsewhere(); elsewhere { + t.Fatal("hosted chat reported as heldElsewhere") + } + if a.leftOff.row != nil || a.devicesWanted() || a.addMachineWanted() { + t.Fatal("hosted Home offered local device/resume bands") + } + + screen := homeText(a) + if strings.Contains(screen, "running on host-b") || strings.Contains(screen, chatlist.OfferContinue) || strings.Contains(screen, "Other Remote") { + t.Fatalf("hosted Home showed held-elsewhere or continue offer:\n%s", screen) + } + if got := countHomeRowsForID(a, heldCell); got != 1 { + t.Fatalf("hosted Home showed %d rows for %s, want 1:\n%s", got, heldCell, screen) + } +} + +// Contract 4: A chat known both locally and through sync appears once on Home, +// rows are keyed by chat id rather than transcript path, and branch copies of +// one chat collapse into the main row with a count and loss explanation. +func TestOneChatOneRowKeyedByIDAndCollapsedBranchCopies(t *testing.T) { + lab := newHomeLab(t) + workspace := lab.workspace("project") + now := time.Now() + + // Same chat ID stored at two different transcript paths (original and .moving copy). + const sharedID = "0000000000000401" + orig := lab.session("project", sharedID, "Shared Architecture", workspace, now.Add(-10*time.Minute)) + moving := lab.session(filepath.Join("project", ".moving"), sharedID, "Shared Architecture", workspace, now.Add(-2*time.Minute)) + _ = orig + + src := &scriptedRemoteSource{ + rows: []chatlist.Row{ + { + Cell: sharedID, + Title: "Shared Architecture", + Device: "tower", + Status: chatlist.Running, + Head: "turn-5", + DurableAgo: time.Minute, + }, + { + Cell: "branch-redundant", + Title: "Shared Architecture", + Device: "laptop", + Parent: sharedID, + Status: chatlist.Branch, + Head: "turn-5", + OrphanTurns: 2, + DurableAgo: 3 * time.Minute, + }, + }, + } + + a := lab.app(moving) + a.width, a.height = 180, 48 + a.machines = src + a.taker = &fakeTaker{} + + drain(t, a, a.openHome()) + drain(t, a, a.askMachines()) + + if got := countHomeRowsForID(a, sharedID); got != 1 { + t.Fatalf("rows for %s when held elsewhere = %d, want 1:\n%s", sharedID, got, homeText(a)) + } + if got := countHomeRowsForID(a, "branch-redundant"); got != 0 { + t.Fatalf("branch copy rendered as separate row (%d):\n%s", got, homeText(a)) + } + screen := homeText(a) + flat := strings.Join(strings.Fields(screen), " ") + for _, want := range []string{ + "1 copy", + "unsent turns set aside", + "discarding loses nothing", + } { + if !strings.Contains(flat, want) { + t.Fatalf("Home missing collapsed copy explanation %q:\n%s", want, screen) + } + } + + // When held locally (Status == Here) and a second non-redundant branch copy exists, + // there is still exactly one row for sharedID, now saying discarding archives turns not in this chat. + src.rows[0].Status = chatlist.Here + src.rows[0].Mine = true + src.rows = append(src.rows, chatlist.Row{ + Cell: "branch-extra", + Title: "Shared Architecture", + Device: "desk", + Parent: sharedID, + Status: chatlist.Branch, + Head: "turn-diverged", + OrphanTurns: 3, + DurableAgo: 4 * time.Minute, + }) + a.machinesAsking = false + drain(t, a, a.askMachines()) + + if got := countHomeRowsForID(a, sharedID); got != 1 { + t.Fatalf("rows for %s when held here = %d, want 1:\n%s", sharedID, got, homeText(a)) + } + screen = homeText(a) + for _, want := range []string{ + "2 copies", + "unsent turns set aside", + "discarding archives", + "3 turns not in this chat", + } { + if !strings.Contains(screen, want) { + t.Fatalf("Home missing non-redundant copy explanation %q:\n%s", want, screen) + } + } +} diff --git a/internal/tui3/continue_card.go b/internal/tui3/continue_card.go index e6b18d822f..6fd533a1c1 100644 --- a/internal/tui3/continue_card.go +++ b/internal/tui3/continue_card.go @@ -28,6 +28,18 @@ func (a *app) continueCardFor(row chatlist.Row, fromWindow bool) *continueCard { if line.row.Origin != "" { card.project = filepath.Base(line.row.Origin) + session.MovedCopyWord } + return card + } + for _, project := range a.home.world.Projects { + for _, s := range project.Sessions { + if homeSessionID(s) == row.Cell { + card.project = a.hostedPath(tildePath(s.Workspace, a.tilde)) + if s.Origin != "" { + card.project = filepath.Base(s.Origin) + session.MovedCopyWord + } + return card + } + } } return card } diff --git a/internal/tui3/conversation_open.go b/internal/tui3/conversation_open.go index dcf2d2de09..3e5672d31f 100644 --- a/internal/tui3/conversation_open.go +++ b/internal/tui3/conversation_open.go @@ -137,6 +137,9 @@ func (a *app) conversationLater(ask func() (Conversation, error), say func(strin a.conversationPreview = nil } if !current || !here || !a.at(page) { + if current { + a.finishHomeTake(true) + } if conv.Agent != nil { leaveOffFrame(conv.Agent) } @@ -173,10 +176,7 @@ func (a *app) conversationLater(ask func() (Conversation, error), say func(strin } func (a *app) cancelConversationOpening() { - if a.homeTakeDraft != nil { - a.restoreTakeDraft(*a.homeTakeDraft) - a.homeTaking, a.homeTakeDraft, a.homeTakenSaid = "", nil, nil - } + a.finishHomeTake(true) hadDraft := a.conversationPreview != nil && (!a.conversationPreview.view.input.empty() || len(a.conversationPreview.view.pasted) > 0) if a.conversationPreview == nil { defer a.tabSettleTeam() @@ -328,12 +328,10 @@ func (a *app) homeOpenLater(line homeLine, takeover bool) tea.Cmd { if a.homeTakenSaid != nil { said, a.homeTakenSaid = a.homeTakenSaid(), nil } + a.finishHomeTake(false) return tea.Batch(opened, a.homeLandOnTask(line), said) }, func(err error) tea.Cmd { - if a.homeTakeDraft != nil { - a.restoreTakeDraft(*a.homeTakeDraft) - a.homeTaking, a.homeTakeDraft, a.homeTakenSaid = "", nil, nil - } + a.finishHomeTake(true) if errors.Is(err, session.ErrSessionLocked) || err.Error() == sessionBusyWord { if takeover { return a.homeTakeoverEnter(line) diff --git a/internal/tui3/home.go b/internal/tui3/home.go index acf7fcb938..461d0e0a0b 100644 --- a/internal/tui3/home.go +++ b/internal/tui3/home.go @@ -2528,6 +2528,9 @@ func (a *app) homeKey(msg tea.KeyPressMsg) tea.Cmd { if a.cancelTakeover() { return nil } + if a.cancelHomeTake() { + return nil + } // THE @ LIST IS INNER TO THE BOX, and so it goes first. It is drawn // under home's box and belongs to the half-typed token in it // (homeat.go), so esc over an open list means "not that one" and must @@ -3109,7 +3112,8 @@ func (a *app) homeOpenLine(line homeLine) tea.Cmd { if line.row.Transcript == a.file { a.rememberOpen(a.frontTabKey()) a.closeHome() - return a.homeLandOnTask(line) + said := a.spendHomeTakeFor(line) + return tea.Batch(a.homeLandOnTask(line), said) } // THE ORDER OF THESE CHECKS IS THE FEATURE. Identity comes first, because a // transcript THIS PROCESS holds answers [session.InUse] true about itself — @@ -3130,7 +3134,8 @@ func (a *app) homeOpenLine(line homeLine) tea.Cmd { // surface narrating a door it just walked through. cmd, _ := a.bringForward(line.row.Transcript) a.closeHome() - return tea.Batch(cmd, a.homeLandOnTask(line)) + said := a.spendHomeTakeFor(line) + return tea.Batch(cmd, a.homeLandOnTask(line), said) case !a.canOpen(): h.say(resumeUnavailableWord, "") return nil diff --git a/internal/tui3/homeband_addmachine.go b/internal/tui3/homeband_addmachine.go index 860ca1346a..e51dcdde60 100644 --- a/internal/tui3/homeband_addmachine.go +++ b/internal/tui3/homeband_addmachine.go @@ -91,6 +91,9 @@ type addMachine struct { // wanted says the card is on: a door that can approve, and a fleet known to be one. func (a *app) addMachineWanted() bool { + if a.hosted() { + return false + } m := a.addMachine return a.approvals != nil && m.known && m.size < fleetEnough } @@ -105,7 +108,7 @@ type fleetMsg struct { // card has no more job and nothing is asked again. func (a *app) askFleet() tea.Cmd { m := &a.addMachine - if a.fleet == nil || m.asking || (m.known && m.size >= fleetEnough) { + if a.hosted() || a.fleet == nil || m.asking || (m.known && m.size >= fleetEnough) { return nil } m.asking = true diff --git a/internal/tui3/homeband_devices.go b/internal/tui3/homeband_devices.go index ac1f166cfd..56e5ba4043 100644 --- a/internal/tui3/homeband_devices.go +++ b/internal/tui3/homeband_devices.go @@ -67,6 +67,9 @@ type rosterMsg struct { // askRoster lists the devices, off the update loop. One ask is out at a time. func (a *app) askRoster() tea.Cmd { + if a.hosted() { + return nil + } src, ok := a.fleet.(Roster) if !ok || a.devRow.asking { return nil @@ -107,6 +110,9 @@ func (a *app) presence() (online map[string]bool, known bool) { // devicesWanted says the row is drawn: a roster of more than one device and a // feed that can say who is online. func (a *app) devicesWanted() bool { + if a.hosted() { + return false + } _, known := a.presence() return known && len(a.devRow.devices) > 1 } @@ -170,6 +176,9 @@ func (a *app) newestBringable(device string) (chatlist.Row, bool) { // bringWork is the chord: nothing to bring says so, one device goes straight to // the takeover card, and several ask which. func (a *app) bringWork() tea.Cmd { + if a.hosted() { + return nil + } picks := a.devicePicks() switch len(picks) { case 0: diff --git a/internal/tui3/homeband_resume.go b/internal/tui3/homeband_resume.go index e566935e02..1447e811e2 100644 --- a/internal/tui3/homeband_resume.go +++ b/internal/tui3/homeband_resume.go @@ -121,7 +121,7 @@ func (s resumeState) away(row chatlist.Row) bool { func (a *app) considerResume() { r := &a.leftOff var row *chatlist.Row - if a.taker != nil && !a.machineRead.computerRemoved { + if !a.hosted() && a.taker != nil && !a.machineRead.computerRemoved { row = a.resumeState().pick() } if !sameOffer(r.row, row) { @@ -157,7 +157,7 @@ func sameOffer(x, y *chatlist.Row) bool { // resumeKeyHandler claims the card's two chords where the card stands. func (a *app) resumeKeyHandler(key string) (tea.Cmd, bool) { - if a.machineRead.computerRemoved { + if a.hosted() || a.machineRead.computerRemoved { return nil, false } row := a.leftOff.row @@ -255,7 +255,7 @@ func (a *app) continueAfterQuit(err error) tea.Cmd { } func (a *app) continuationReady(msg continuationChecked) tea.Cmd { - if a.machineRead.computerRemoved { + if a.hosted() || a.machineRead.computerRemoved { return nil } if msg.automatic && msg.generation != a.continueGeneration { diff --git a/internal/tui3/homegrid.go b/internal/tui3/homegrid.go index 1f8ab1856b..0238437b21 100644 --- a/internal/tui3/homegrid.go +++ b/internal/tui3/homegrid.go @@ -327,6 +327,7 @@ type homeGridInput struct { // ([homeGridPanel.growsARow]), and paying that on a narrow terminal for a // column that is not on the screen costs a row a person could have read. desc bool + far bool } // gridInput gathers it. EVERY FIELD IS A CACHE HOME ALREADY HOLDS, and the one @@ -348,7 +349,7 @@ func (h *homeView) gridInput() homeGridInput { open, closed := h.conversationRows() return homeGridInput{openChats: open, closedChats: closed, rows: reading.rows, ledger: reading.ledger, calls: calls, callsOlder: older, opened: h.opened, openedOn: h.openedOn, - desc: homeDescOn(h.cols), world: world, autos: h.autos, active: h.active, + desc: homeDescOn(h.cols), far: h.far, world: world, autos: h.autos, active: h.active, errands: h.switchExchanges(), bucket: h.bucket, launch: h.launch, tilde: h.tilde, last: h.last, repos: h.repos, others: h.others, spend: h.spend, seen: h.seen, now: h.world.Read} } diff --git a/internal/tui3/homelive_test.go b/internal/tui3/homelive_test.go index 12181c12ac..c193ece2f9 100644 --- a/internal/tui3/homelive_test.go +++ b/internal/tui3/homelive_test.go @@ -73,8 +73,10 @@ func TestHomeLiveSource(t *testing.T) { t.Fatal(err) } drain(t, a, a.askMachines()) - if text := homeText(a); !strings.Contains(text, "2 turns from host-a") { - t.Fatalf("a branch that appeared is not listed:\n%s", text) + // A branch of the chat folds into the chat's own row (#2070): the row keeps + // its state and counts the copy rather than standing a second row beside it. + if text := homeText(a); !strings.Contains(text, "host-a offline · 1 copy") || strings.Count(text, "Port the picker") != 1 { + t.Fatalf("a branch that appeared is not counted on its chat's row:\n%s", text) } live.down = true diff --git a/internal/tui3/homepanel_continue.go b/internal/tui3/homepanel_continue.go index eba7fc4d39..2ce4e4a490 100644 --- a/internal/tui3/homepanel_continue.go +++ b/internal/tui3/homepanel_continue.go @@ -84,11 +84,14 @@ type BranchActions struct { // The words of the two cards. The sentences that are contract copy come from // [chatlist]; these are the questions around them. const ( - continueAsk = "Continue this chat here?" - continueStay = "leave it there" - lostRaceWord = chatlist.LostRace - continuing = "continuing here…" - continueFails = "could not continue this chat here" + continueAsk = "Continue this chat here?" + continueStay = "leave it there" + continueRetry = "retry" + continueAlreadyRunning = "a chat is already continuing here; wait for it to open" + lostRaceWord = chatlist.LostRace + continuing = "continuing here…" + continueFails = "could not continue this chat here" + continueStoreBusy = continueFails + " because the engine store is busy" // continueUnreachable names what did not happen, not only why, because // the box still holds words the person meant to send in the moved chat. continueUnreachable = continueFails + " because other machines are unreachable; your words are still in the box" @@ -161,6 +164,9 @@ var remoteOffers = map[chatlist.OfferKind]func(*app, chatlist.Row, chatlist.Offe // homeMachineEnter is enter on a chat from another machine. func (a *app) homeMachineEnter(line homeLine) tea.Cmd { + if a.hosted() || line.remote == nil { + return nil + } if a.machineRead.computerRemoved { a.home.say(chatlist.Removed, "") return nil @@ -178,6 +184,11 @@ func (a *app) homeMachineEnter(line homeLine) tea.Cmd { a.home.say(a.machineRead.unavailableWord(), "") return nil } + if a.homeTaking != "" && (offer.Kind == chatlist.Open || offer.Kind == chatlist.ContinueHere) && a.taker != nil { + a.raiseMachineAsk(row, a.continueBusyShown(row)) + a.home.say(continueAlreadyRunning, "") + return nil + } return run(a, row, offer) } @@ -195,6 +206,51 @@ const ( continueStayAt = 1 ) +func (a *app) nextContinueAskID() uint64 { + a.continueAskSeq++ + return a.continueAskSeq +} + +// finishHomeTake clears every in-flight continuation marker on this window. +// When restoreDraft is true, any unsent words captured before opening are put back. +func (a *app) finishHomeTake(restoreDraft bool) { + if a.homeTakeCancel != nil { + a.homeTakeCancel() + a.homeTakeCancel = nil + } + if restoreDraft && a.homeTakeDraft != nil { + a.restoreTakeDraft(*a.homeTakeDraft) + } + a.homeTaking, a.homeTakeDraft, a.homeTakenSaid = "", nil, nil +} + +// cancelHomeTake stops an in-flight takeover before its answer arrives. +func (a *app) cancelHomeTake() bool { + if a.homeTaking == "" { + return false + } + a.homeTakeSeq++ + a.finishHomeTake(true) + if a.at(pageHome) && a.home.msg == continuing { + a.home.say("", "") + } + a.touch() + return true +} + +func (a *app) spendHomeTakeFor(line homeLine) tea.Cmd { + if a.homeTakenSaid != nil { + said := a.homeTakenSaid + a.homeTakenSaid = nil + a.finishHomeTake(false) + return said() + } + if a.homeTaking != "" && (homeSessionID(line.row) == a.homeTaking || filepath.Base(session.DirOf(line.row.Transcript)) == a.homeTaking) { + a.finishHomeTake(false) + } + return nil +} + // offerContinue raises the takeover screen for a person who chose to continue // (enter on the row, the left-off chord, the device chord): the cursor starts // on `continue here`, so enter completes the move they just asked for. @@ -209,6 +265,9 @@ func (a *app) askContinue(row chatlist.Row) tea.Cmd { } func (a *app) raiseContinue(row chatlist.Row, at int) tea.Cmd { + if a.hosted() { + return nil + } if a.machineRead.computerRemoved { a.home.say(chatlist.Removed, "") return nil @@ -219,6 +278,11 @@ func (a *app) raiseContinue(row chatlist.Row, at int) tea.Cmd { if a.taker == nil { return a.sayOffer(row, chatlist.Offer{Line: chatlist.StatusLine(row)}) } + if a.homeTaking != "" { + a.raiseMachineAsk(row, a.continueBusyShown(row)) + a.home.say(continueAlreadyRunning, "") + return nil + } a.raiseMachineAsk(row, a.continueShown(row, at)) return nil } @@ -231,6 +295,7 @@ func (a *app) continueShown(row chatlist.Row, at int) questionShown { return questionShown{ continuation: card, question: session.Question{ + ID: a.nextContinueAskID(), Kind: homeContinueKind, Ask: session.AskConfirmation, Form: session.FormCard, @@ -253,11 +318,108 @@ func (a *app) continueShown(row chatlist.Row, at int) questionShown { } return nil } + if a.homeTaking != "" { + a.raiseMachineAsk(row, a.continueBusyShown(row)) + a.home.say(continueAlreadyRunning, "") + return nil + } + return a.takeRemote(row) + }, + } +} + +// continueBusyShown tells the person on the card itself that another take is +// already in progress, rather than closing the card and doing nothing. +func (a *app) continueBusyShown(row chatlist.Row) questionShown { + row = a.presentRow(row) + card := a.continueCardFor(row, false) + return questionShown{ + continuation: card, + question: session.Question{ + ID: a.nextContinueAskID(), + Kind: homeContinueKind, + Ask: session.AskConfirmation, + Form: session.FormCard, + Asker: session.Asker{Kind: session.AskerSurface}, + Head: continueAsk, + Reason: continueAlreadyRunning, + Subject: session.SubjectRef{Name: row.Title}, + Options: []session.AnswerOption{ + {Key: "1", Label: chatlist.OfferContinue}, + {Key: "2", Label: continueStay, Safe: true}, + }, + Stakes: session.StakesReversible, + Asked: a.now(), + }, + pick: continueYesAt, + local: func(answer session.Answer) tea.Cmd { + if answer.FirstKey() != "1" { + if card.returnFile != "" && a.file == card.returnFile { + a.closeHome() + } + return nil + } + if a.homeTaking != "" { + a.raiseMachineAsk(row, a.continueBusyShown(row)) + a.home.say(continueAlreadyRunning, "") + return nil + } return a.takeRemote(row) }, } } +// continueRetryShown keeps a failed take's reason on the card where the person +// is looking and offers retry without losing captured workspace roots or drafts. +func (a *app) continueRetryShown(msg homeTakenMsg, failure string) questionShown { + row := a.presentRow(msg.row) + card := a.continueCardFor(row, msg.fromFile != "") + if msg.draft != nil { + card.draft = *msg.draft + } + if msg.fromFile != "" { + card.returnFile = msg.fromFile + } + return questionShown{ + continuation: card, + question: session.Question{ + ID: a.nextContinueAskID(), + Kind: homeContinueKind, + Ask: session.AskConfirmation, + Form: session.FormCard, + Asker: session.Asker{Kind: session.AskerSurface}, + Head: continueAsk, + Reason: failure, + Subject: session.SubjectRef{Name: row.Title}, + Options: []session.AnswerOption{ + {Key: "1", Label: continueRetry}, + {Key: "2", Label: continueStay, Safe: true}, + }, + Stakes: session.StakesReversible, + Asked: a.now(), + }, + pick: continueYesAt, + local: func(answer session.Answer) tea.Cmd { + if answer.FirstKey() != "1" { + if card.returnFile != "" && a.file == card.returnFile { + a.closeHome() + } + return nil + } + if a.homeTaking != "" { + a.raiseMachineAsk(row, a.continueBusyShown(row)) + a.home.say(continueAlreadyRunning, "") + return nil + } + a.machineRead.down = false + retry := msg + retry.err = nil + retry.taken = Taken{} + return a.takeRemoteState(retry) + }, + } +} + // presentRow is the row as the device's presence reads now: a chat whose holder // is not online is one that stopped, so the card does not say it runs there. func (a *app) presentRow(row chatlist.Row) chatlist.Row { @@ -271,6 +433,7 @@ func (a *app) presentRow(row chatlist.Row) chatlist.Row { // homeTakenMsg is the takeover's answer, coming back off the update loop. type homeTakenMsg struct { + seq uint64 row chatlist.Row taken Taken err error @@ -290,17 +453,36 @@ func (a *app) takeRemoteWithRoots(row chatlist.Row, roots map[string]string) tea } func (a *app) takeRemoteState(msg homeTakenMsg) tea.Cmd { + if a.hosted() || a.taker == nil { + a.restoreTakeDraft(msg) + return nil + } if a.machineRead.computerRemoved { a.restoreTakeDraft(msg) a.home.say(chatlist.Removed, "") return nil } if a.homeTaking != "" { + a.restoreTakeDraft(msg) + if a.at(pageHome) { + a.raiseMachineAsk(msg.row, a.continueBusyShown(msg.row)) + a.home.say(continueAlreadyRunning, "") + } else { + a.note(continueAlreadyRunning) + } return nil } + a.homeTakeSeq++ + msg.seq = a.homeTakeSeq a.homeTaking = msg.row.Cell a.home.say(continuing, "") - taker, ctx := a.taker, a.ctx + baseCtx := a.ctx + if baseCtx == nil { + baseCtx = context.Background() + } + ctx, cancel := context.WithCancel(baseCtx) + a.homeTakeCancel = cancel + taker := a.taker if a.file != "" && filepath.Base(session.DirOf(a.file)) == msg.row.Cell { msg.fromFile = a.file // The displaced session may retire during the move, so its unsent words travel with the answer. @@ -324,8 +506,15 @@ func (a *app) takeRemoteState(msg homeTakenMsg) tea.Cmd { // tookTakeover restores unsent words on failure and opens the arrived chat on success. func (a *app) tookTakeover(msg homeTakenMsg) tea.Cmd { + if msg.seq != 0 && msg.seq != a.homeTakeSeq { + return nil + } + if a.homeTakeCancel != nil { + a.homeTakeCancel() + a.homeTakeCancel = nil + } if msg.err != nil { - a.homeTaking = "" + a.finishHomeTake(false) } var missing *session.WorkspaceBindingsRequired if errors.As(msg.err, &missing) { @@ -346,11 +535,11 @@ func (a *app) tookTakeover(msg homeTakenMsg) tea.Cmd { // Home is where the failed move lands, so it carries the whole sentence: // the reason alone does not say the chat stayed and the words were kept. a.machineRead.down = true - a.home.say(failure, "") + a.showTakeFailure(msg, failure) case errors.Is(msg.err, directory.ErrConditionalTake): - a.home.say(directory.ErrConditionalTake.Error(), "") + a.showTakeFailure(msg, directory.ErrConditionalTake.Error()) case msg.err != nil: - a.home.say(failure, "") + a.showTakeFailure(msg, failure) default: return a.openTaken(msg) } @@ -358,6 +547,16 @@ func (a *app) tookTakeover(msg homeTakenMsg) tea.Cmd { return nil } +// showTakeFailure keeps the failure visible where the person is looking: on a +// retry card beside the row when Home is open, and in the conversation notes +// when looking at the chat. +func (a *app) showTakeFailure(msg homeTakenMsg, failure string) { + a.home.say(failure, "") + if a.at(pageHome) && a.taker != nil && !a.machineRead.computerRemoved { + a.raiseMachineAsk(msg.row, a.continueRetryShown(msg, failure)) + } +} + // chooseTakeRoots keeps the original holder and unsent words while collecting local folders. func (a *app) chooseTakeRoots(msg homeTakenMsg, missing []session.WorkspaceRootBinding) tea.Cmd { if len(missing) == 0 { @@ -367,7 +566,10 @@ func (a *app) chooseTakeRoots(msg homeTakenMsg, missing []session.WorkspaceRootB a.folder.forWorkspace = true a.folder.held = nil a.folder.chooseTitle = "local folder for " + missing[0].Label - a.folder.cancelChoose = func() { a.restoreTakeDraft(msg) } + a.folder.cancelChoose = func() { + a.finishHomeTake(false) + a.restoreTakeDraft(msg) + } a.folder.chooseFolder = func(path string) tea.Cmd { roots := make(map[string]string, len(msg.roots)+1) for uri, folder := range msg.roots { @@ -440,6 +642,7 @@ func (a *app) openTaken(msg homeTakenMsg) tea.Cmd { a.machineRead.rows = rows a.machinesRetaken = a.machinesAsking a.considerResume() + a.home.others = a.othersNow() a.refreshHome() kept := takenSaid(msg.taken) moved := movedSaid(msg.taken) @@ -448,11 +651,16 @@ func (a *app) openTaken(msg homeTakenMsg) tea.Cmd { } line, ok := a.takenLine(msg) if !ok { - a.homeTaking = "" + a.finishHomeTake(false) a.restoreTakeDraft(msg) a.home.say(joinSaid(moved, kept), "") return a.askMachines() } + if a.conversationOpening { + a.conversationOpening = false + a.conversationPreview = nil + a.conversationRequest++ + } a.homeTakeDraft = &msg a.homeTakenSaid = func() tea.Cmd { // A local session of this same chat was retired before replacement. @@ -467,7 +675,7 @@ func (a *app) openTaken(msg homeTakenMsg) tea.Cmd { a.forget(key) } } - a.homeTaking, a.homeTakeDraft = "", nil + a.finishHomeTake(false) a.note(joinSaid(moved, kept)) if msg.draft != nil { if msg.taken.AlreadyHere { @@ -500,22 +708,26 @@ func (a *app) openTaken(msg homeTakenMsg) tea.Cmd { } switch { case cmd == nil && a.at(pageHome): - a.homeTaking, a.homeTakeDraft = "", nil - a.restoreTakeDraft(msg) + a.finishHomeTake(true) // THE OPEN REFUSED and home still stands: the sentence has no // conversation to arrive in, so home itself says the takeover // happened, the way it says every row's refusal. a.home.say(joinSaid(moved, kept), "") - a.homeTakenSaid = nil return a.askMachines() case !a.at(pageHome): // THE OPEN LANDED IN PLACE — a shared window walks in synchronously — // so the ride is spent here rather than at a landing that already // happened. - said := a.homeTakenSaid - a.homeTakenSaid = nil - return tea.Batch(cmd, said(), a.askMachines()) + var saidCmd tea.Cmd + if a.homeTakenSaid != nil { + said := a.homeTakenSaid + a.homeTakenSaid = nil + saidCmd = said() + } + a.finishHomeTake(false) + return tea.Batch(cmd, saidCmd, a.askMachines()) } + a.finishHomeTake(true) return tea.Batch(cmd, a.askMachines()) } @@ -604,7 +816,7 @@ func takenSaid(t Taken) string { // homeLineOf is the conversation line of a chat this machine lists. func (a *app) homeLineOf(cell string) (homeLine, bool) { for _, line := range a.home.lines { - if line.kind == homeSession && line.row.ID == cell { + if line.kind == homeSession && homeSessionID(line.row) == cell { return line, true } } @@ -638,6 +850,9 @@ func (a *app) branchVerbs() []branchVerb { // offerBranch raises the branch card, or says the row's sentence when nothing // can be done with it from here. func (a *app) offerBranch(row chatlist.Row, _ chatlist.Offer) tea.Cmd { + if a.hosted() { + return nil + } verbs := a.branchVerbs() if len(verbs) == 0 { a.home.say(chatlist.BranchLine(row, false), "") @@ -657,6 +872,7 @@ func (a *app) branchShown(row chatlist.Row, verbs []branchVerb) questionShown { options = append(options, session.AnswerOption{Key: string(rune('1' + len(verbs))), Label: branchLeave, Safe: true}) return questionShown{ question: session.Question{ + ID: a.nextContinueAskID(), Kind: homeBranchKind, Ask: session.AskConfirmation, Form: session.FormCard, @@ -686,14 +902,32 @@ type homeBranchMsg struct{ err error } // runBranchVerb runs one verb off the update loop, for takeRemote's reason. func (a *app) runBranchVerb(v branchVerb, row chatlist.Row) tea.Cmd { - ctx := a.ctx + if a.hosted() { + return nil + } if v.label == "open separately" { + if a.homeTaking != "" { + a.raiseMachineAsk(row, a.continueBusyShown(row)) + a.home.say(continueAlreadyRunning, "") + return nil + } + a.homeTakeSeq++ + seq := a.homeTakeSeq + a.homeTaking = row.Cell + a.home.say(continuing, "") + baseCtx := a.ctx + if baseCtx == nil { + baseCtx = context.Background() + } + ctx, cancel := context.WithCancel(baseCtx) + a.homeTakeCancel = cancel open := a.branches.Open return func() tea.Msg { taken, err := open(ctx, row.Cell) - return homeTakenMsg{row: row, taken: taken, err: err} + return homeTakenMsg{seq: seq, row: row, taken: taken, err: err} } } + ctx := a.ctx return func() tea.Msg { return homeBranchMsg{err: v.do(ctx, row.Cell)} } } @@ -708,6 +942,9 @@ func (a *app) tookBranch(msg homeBranchMsg) tea.Cmd { // othersNow is the held reading with presence as the feed says it now. func (a *app) othersNow() machineReading { + if a.hosted() { + return machineReading{} + } reading := a.machineRead reading.away = a.resumeState().away return reading diff --git a/internal/tui3/homepanel_heldelsewhere.go b/internal/tui3/homepanel_heldelsewhere.go index 41c5e3b4cc..2249633a10 100644 --- a/internal/tui3/homepanel_heldelsewhere.go +++ b/internal/tui3/homepanel_heldelsewhere.go @@ -369,6 +369,14 @@ func (a *app) failedTake(msg homeTakenMsg) string { case errors.Is(msg.err, directory.ErrRevoked): a.removedComputer() return chatlist.Removed + case errors.Is(msg.err, cellstore.ErrStoreBusy): + var busy *cellstore.StoreBusyError + if errors.As(msg.err, &busy) { + if who := busy.Holder.Describe(); who != "" { + return continueFails + " because the engine store is busy (held by " + who + ")" + } + } + return continueStoreBusy case strings.Contains(msg.err.Error(), "rewind cancelled"), strings.Contains(msg.err.Error(), "changed during rewind"): return continueFails + " because the folder changed during the move; your words are still in the box" default: diff --git a/internal/tui3/homepanel_machines.go b/internal/tui3/homepanel_machines.go index 438ac5e8c4..be00336540 100644 --- a/internal/tui3/homepanel_machines.go +++ b/internal/tui3/homepanel_machines.go @@ -76,7 +76,7 @@ func (a *app) askMachines() tea.Cmd { // askChats is the listing half of [app.askMachines]. func (a *app) askChats() tea.Cmd { - if a.machines == nil || a.machinesAsking { + if a.hosted() || a.machines == nil || a.machinesAsking { return nil } a.machinesAsking = true @@ -140,8 +140,61 @@ func (a *app) recordMachines(msg homeMachinesMsg) tea.Cmd { // into merges the other machines' chats into the panel's own lines, newest // first, cuts to the panel's cap, and puts the unreachable sentence at the foot. +// Each chat id appears at most once: a chat held elsewhere replaces its stale +// local row with the continuation door, while a chat held or released here +// keeps its single local row (carrying any collapsed branch copies). func (m machineReading) into(in *homeGridInput, own []homeLine) homePanelRows { - lines := append(own, m.lines(in, own)...) + if in != nil && in.far { + return homePanelCut(in, panelSessions, own) + } + localIDs := make(map[string]bool, len(own)) + localTitle := make(map[string]string, len(own)) + for _, line := range own { + if id := homeSessionID(line.row); id != "" { + localIDs[id] = true + if line.cell != nil && line.cell.title != "" { + localTitle[id] = line.cell.title + } + } + } + collapsed, byLocal := chatlist.CollapseCopies(m.rows, localIDs) + replacedByRemote := map[string]bool{} + var remoteLines []homeLine + for _, row := range collapsed { + if m.away != nil && m.away(row) { + row = row.Quiet() + } + if localIDs[row.Cell] { + if heldElsewhere(row) { + if (row.Title == "" || row.Title == "untitled") && localTitle[row.Cell] != "" { + row.Title = localTitle[row.Cell] + } + replacedByRemote[row.Cell] = true + remoteLines = append(remoteLines, machineLineIn(row, m.at, in, m.merge, m.separate)) + } else if len(row.Copies) > 0 { + if byLocal == nil { + byLocal = make(map[string][]chatlist.BranchCopy) + } + byLocal[row.Cell] = append(byLocal[row.Cell], row.Copies...) + } + continue + } + if drawnAsRemote(row, false) { + remoteLines = append(remoteLines, machineLineIn(row, m.at, in, m.merge, m.separate)) + } + } + lines := make([]homeLine, 0, len(own)+len(remoteLines)) + for _, line := range own { + id := homeSessionID(line.row) + if id != "" && replacedByRemote[id] { + continue + } + if copies := byLocal[id]; len(copies) > 0 && line.cell != nil { + decorateCellWithCopies(line.cell, copies, in != nil && in.desc) + } + lines = append(lines, line) + } + lines = append(lines, remoteLines...) sort.SliceStable(lines, func(i, j int) bool { return lines[i].stamp().After(lines[j].stamp()) }) rows := homePanelCut(in, panelSessions, lines) if m.down { @@ -160,31 +213,33 @@ func (m machineReading) unavailableWord() string { return chatlist.Unreachable } -// lines are the rows to draw: none of the kind that only this machine can hold, -// and none for a chat this machine already lists unless another machine holds it -// live. That one is drawn WITH its door (`running on desktop`, enter takes it), -// because the local row of a chat taken elsewhere is a stale window, and the way -// back to it is the same take the cold path runs. +// lines are the rows to draw from other machines when merged with own. func (m machineReading) lines(in *homeGridInput, own []homeLine) []homeLine { + if in != nil && in.far { + return nil + } local := map[string]bool{} for _, line := range own { - local[line.row.ID] = true + if id := homeSessionID(line.row); id != "" { + local[id] = true + } } + collapsed, _ := chatlist.CollapseCopies(m.rows, local) var out []homeLine - for _, row := range m.rows { + for _, row := range collapsed { if m.away != nil && m.away(row) { row = row.Quiet() } if drawnAsRemote(row, local[row.Cell]) { - out = append(out, machineLine(row, m.at, in.now, m.merge, m.separate)) + out = append(out, machineLineIn(row, m.at, in, m.merge, m.separate)) } } return out } // drawnAsRemote says a row from the directory is a row of the panel. A chat -// held here is never drawn twice; a chat listed here is drawn again only while -// another machine holds it, live or gone quiet. +// held here is never drawn twice; a chat listed here replaces its local row +// only while another machine holds it, live or gone quiet. func drawnAsRemote(row chatlist.Row, listedHere bool) bool { if row.Status == chatlist.Here { return false @@ -200,10 +255,51 @@ func heldElsewhere(row chatlist.Row) bool { return row.Status == chatlist.Running || (row.Status == chatlist.Off && !row.Mine) } +func decorateCellWithCopies(cell *homeCell, copies []chatlist.BranchCopy, desc bool) { + full := chatlist.CopiesLine(copies) + short := chatlist.CopiesShort(copies) + if full == "" { + return + } + if cell.note != "" { + baseShort := cell.noteShort + if baseShort == "" { + baseShort = cell.note + } + cell.note = cell.note + " · " + full + // THE CHAT'S OWN STATE OUTRANKS THE STORY OF ITS COPIES. Where the whole + // sentence does not fit, the narrow spelling keeps the state and only counts + // the copies, so a row never loses `host-a offline` to the sentence about + // what a copy is; the painter drops a narrow note that is still too wide. + cell.noteShort = baseShort + " · " + chatlist.CopiesCount(copies) + } else { + cell.note = full + cell.noteShort = short + } + cell.keepNote = true + if desc { + cell.sub = rowClauses(cell.sub, full) + } +} + // machineLine is one chat on another machine as a row of the sessions panel. // The turn's time is fixed once, from the moment of the read; the age drawn is // then counted to now, so it grows with the clock however long ago the ask was. func machineLine(row chatlist.Row, readAt, now time.Time, merge bool, separate ...bool) homeLine { + return machineLineWithDesc(row, readAt, now, false, merge, separate...) +} + +func machineLineIn(row chatlist.Row, readAt time.Time, in *homeGridInput, merge bool, separate ...bool) homeLine { + var now time.Time + var desc bool + if in != nil { + now = in.now + desc = in.desc + } + return machineLineWithDesc(row, readAt, now, desc, merge, separate...) +} + +func machineLineWithDesc(row chatlist.Row, readAt, now time.Time, desc, merge bool, separate ...bool) homeLine { // THE DRAW CLOCK IS THE WORLD'S READING AND THE READ TIME IS THE ASK'S, two // samples of one wall clock a few milliseconds apart. A draw that seems to // come before the read would make a turn look younger than the directory said @@ -219,11 +315,15 @@ func machineLine(row chatlist.Row, readAt, now time.Time, merge bool, separate . // spelling for frames that cannot hold the whole. note, short = chatlist.BranchLine(row, merge, separate...), chatlist.BranchShort(row) } - return homeLine{kind: homeMachineRow, since: at, remote: &row, cell: &homeCell{ + cell := &homeCell{ kind: cellRow, panel: panelSessions, title: row.Title, note: note, noteShort: short, keepNote: true, right: sinceAt(at, now), key: "machine:" + row.Cell, - }} + } + if len(row.Copies) > 0 { + decorateCellWithCopies(cell, row.Copies, desc) + } + return homeLine{kind: homeMachineRow, since: at, remote: &row, cell: cell} } // stamp is when a line's chat last moved, for ordering the merged panel. diff --git a/internal/tui3/homepanel_sessions.go b/internal/tui3/homepanel_sessions.go index 888c2d814b..911fd9ef7a 100644 --- a/internal/tui3/homepanel_sessions.go +++ b/internal/tui3/homepanel_sessions.go @@ -3,6 +3,9 @@ package tui3 import ( "path/filepath" "sort" + "strings" + + "github.com/Agent-Field/codeaf/internal/session" ) // Home previews recent conversations; the Sessions tab holds their full trees. @@ -10,38 +13,103 @@ const homeSessionsLimit = 15 type sessionsPanel struct{ homePanelBase } +// homeSessionID extracts a conversation's stable chat id, recovering it from a +// structured session or cell folder when the row has not populated ID yet. +func homeSessionID(row session.SessionRow) string { + if id := strings.TrimSpace(row.ID); id != "" { + return id + } + if dir, ok := session.FolderOf(row.Transcript); ok { + if id := strings.TrimSpace(filepath.Base(dir)); id != "" && id != "." && id != string(filepath.Separator) { + return id + } + } + return "" +} + +// homeSessionKey keys Home's conversation rows by chat id rather than by +// transcript path, falling back to the cleaned file path only for bare test +// transcripts that live outside a session folder and carry no id. +func homeSessionKey(row session.SessionRow) string { + if id := homeSessionID(row); id != "" { + return "id:" + id + } + return "file:" + filepath.Clean(row.Transcript) +} + +func preferHomeSessionRow(candidate, existing switcherRow) bool { + if candidate.here != existing.here { + return candidate.here + } + if candidate.session.Archived != existing.session.Archived { + return !candidate.session.Archived + } + if candidate.session.Open != existing.session.Open { + return candidate.session.Open + } + if !candidate.session.At.Equal(existing.session.At) { + return candidate.session.At.After(existing.session.At) + } + candMoving := strings.Contains(filepath.ToSlash(candidate.session.Transcript), "/.moving/") + existMoving := strings.Contains(filepath.ToSlash(existing.session.Transcript), "/.moving/") + if candMoving != existMoving { + return candMoving + } + return candidate.session.Transcript < existing.session.Transcript +} + func (sessionsPanel) rows(in *homeGridInput) homePanelRows { - byFile := make(map[string]switcherRow) + byID := make(map[string]switcherRow) + put := func(row switcherRow) { + if row.session.ID == "" { + row.session.ID = homeSessionID(row.session) + } + key := homeSessionKey(row.session) + if prev, exists := byID[key]; exists && !preferHomeSessionRow(row, prev) { + return + } + byID[key] = row + } for _, row := range in.rows { if row.kind == switcherConversation { - byFile[filepath.Clean(row.session.Transcript)] = row + put(row) } } // Closed conversations remain part of history and retain their dim styling. for _, project := range in.world.Projects { for _, row := range project.Sessions { if row.Archived { - byFile[filepath.Clean(row.Transcript)] = switcherRow{kind: switcherConversation, + put(switcherRow{kind: switcherConversation, session: row, project: project.Name, title: homeName(row), - place: switcherWhere(row, project)} + place: switcherWhere(row, project)}) } } } // The tab keeper has fresher titles and ownership than the disk snapshot. for _, row := range in.closedChats { + if row.session.ID == "" { + row.session.ID = homeSessionID(row.session) + } row.session.Archived = true - byFile[filepath.Clean(row.session.Transcript)] = row + key := homeSessionKey(row.session) + if prev, exists := byID[key]; exists && (prev.here || prev.session.Open) { + continue + } + byID[key] = row } for _, row := range in.openChats { - byFile[filepath.Clean(row.session.Transcript)] = row + if row.session.ID == "" { + row.session.ID = homeSessionID(row.session) + } + byID[homeSessionKey(row.session)] = row } - rows := make([]switcherRow, 0, len(byFile)) - for _, row := range byFile { + rows := make([]switcherRow, 0, len(byID)) + for _, row := range byID { rows = append(rows, row) } sort.Slice(rows, func(i, j int) bool { if rows[i].session.At.Equal(rows[j].session.At) { - return rows[i].session.Transcript < rows[j].session.Transcript + return homeSessionKey(rows[i].session) < homeSessionKey(rows[j].session) } return rows[i].session.At.After(rows[j].session.At) }) @@ -54,7 +122,7 @@ func (sessionsPanel) rows(in *homeGridInput) homePanelRows { cell = recentOwnCell(row, in) } cell.panel, cell.chatKey, cell.closed = panelSessions, row.chatKey, row.session.Archived - cell.key = "sessions:" + row.session.Transcript + cell.key = "sessions:" + homeSessionKey(row.session) if row.needs && !needsLandingsSpeakFor(row.session, needsCallTitlesOn(in, row.session.ID)) { cell.mark = cellMarkNeeds } diff --git a/internal/tui3/homepanel_takedoor_test.go b/internal/tui3/homepanel_takedoor_test.go index 75ff81b6fc..77e1f8ee07 100644 --- a/internal/tui3/homepanel_takedoor_test.go +++ b/internal/tui3/homepanel_takedoor_test.go @@ -59,8 +59,8 @@ func rowsOf(a *app, cell string) (local, remote int) { func TestListedHereAndHeldElsewhereDrawsTheDoor(t *testing.T) { src := &flipping{status: chatlist.Running} a, _ := heldLab(t, src, &fakeTaker{}) - if local, remote := rowsOf(a, heldCell); local != 1 || remote != 1 { - t.Fatalf("local=%d remote=%d rows, want one of each:\n%s", local, remote, homeText(a)) + if local, remote := rowsOf(a, heldCell); local != 0 || remote != 1 { + t.Fatalf("local=%d remote=%d rows, want one single row with the door:\n%s", local, remote, homeText(a)) } if !strings.Contains(homeText(a), "running on host-a") { t.Fatalf("the held chat does not say where it runs:\n%s", homeText(a)) diff --git a/internal/tui3/hometabs.go b/internal/tui3/hometabs.go index 179e81cb2a..c33083573f 100644 --- a/internal/tui3/hometabs.go +++ b/internal/tui3/hometabs.go @@ -3,6 +3,8 @@ package tui3 import ( "path/filepath" "sort" + + "github.com/Agent-Field/codeaf/internal/session" ) // homeClosedLimit keeps recently closed conversations within easy reach without @@ -17,13 +19,22 @@ func (h *homeView) conversationRows() (open, closed []switcherRow) { return nil, nil } byFile := map[string]switcherRow{} + byID := map[string]switcherRow{} var archived []switcherRow for _, project := range h.world.Projects { for _, row := range project.Sessions { + if row.ID == "" { + row.ID = homeSessionID(row) + } read := switcherRow{kind: switcherConversation, session: row, project: project.Name, title: homeName(row), age: sinceAt(row.At, h.world.Read), at: switcherSortAt(row), gone: h.gone[switcherWhere(row, project)], place: switcherWhere(row, project)} byFile[filepath.Clean(row.Transcript)] = read + if row.ID != "" { + if prev, exists := byID[row.ID]; !exists || preferHomeSessionRow(read, prev) { + byID[row.ID] = read + } + } if row.Archived { archived = append(archived, read) } @@ -31,10 +42,18 @@ func (h *homeView) conversationRows() (open, closed []switcherRow) { } seen := map[string]bool{} fromTab := func(tab chatTab, isOpen bool) switcherRow { - row := byFile[filepath.Clean(tab.file)] + row, ok := byFile[filepath.Clean(tab.file)] + if !ok { + if id := homeSessionID(session.SessionRow{Transcript: tab.file}); id != "" { + row = byID[id] + } + } row.kind, row.title, row.here = switcherConversation, tab.word, isOpen && tab.here row.chatKey = tab.key row.session.Transcript = tab.file + if row.session.ID == "" { + row.session.ID = homeSessionID(row.session) + } owned := tab.held || tab.here || h.holding != nil && h.holding(tab.file) row.held = (row.session.Open || row.session.Live) && !owned row.door = row.held && !h.far && row.session.Dir != "" @@ -52,8 +71,10 @@ func (h *homeView) conversationRows() (open, closed []switcherRow) { if tab.work || tab.start || tab.file == "" { continue } + built := fromTab(tab, true) + seen[homeSessionKey(built.session)] = true seen[filepath.Clean(tab.file)] = true - open = append(open, fromTab(tab, true)) + open = append(open, built) } if h.closedTabs != nil { tabs := h.closedTabs() @@ -62,22 +83,27 @@ func (h *homeView) conversationRows() (open, closed []switcherRow) { if tab.work || tab.start || tab.file == "" { continue } + built := fromTab(tab, false) + key := homeSessionKey(built.session) file := filepath.Clean(tab.file) - if seen[file] { + if seen[key] || seen[file] { continue } + seen[key] = true seen[file] = true - closed = append(closed, fromTab(tab, false)) + closed = append(closed, built) } } // The close stack gives this window's exact close order. Persisted archived // conversations fill any remaining slots after a restart, newest activity first. sort.SliceStable(archived, func(i, j int) bool { return archived[i].at.After(archived[j].at) }) for _, row := range archived { + key := homeSessionKey(row.session) file := filepath.Clean(row.session.Transcript) - if seen[file] || len(closed) >= homeClosedLimit { + if seen[key] || seen[file] || len(closed) >= homeClosedLimit { continue } + seen[key] = true seen[file] = true closed = append(closed, row) } diff --git a/internal/tui3/machinepoll.go b/internal/tui3/machinepoll.go index c9136220ea..90177ea6fd 100644 --- a/internal/tui3/machinepoll.go +++ b/internal/tui3/machinepoll.go @@ -98,7 +98,7 @@ func (a *app) attended(now time.Time) bool { // pollMachines asks the directory if, and only if, the schedule allows: home is // showing, somebody is at the window, and the wait is over. func (a *app) pollMachines(now time.Time) tea.Cmd { - wanted := (a.at(pageHome) || a.watchesConversation()) && a.attended(now) + wanted := !a.hosted() && (a.at(pageHome) || a.watchesConversation()) && a.attended(now) followed := a.tendWatch(wanted) if !wanted || !a.machinePoll.ready(now) { return followed diff --git a/internal/tui3/manual_devices_test.go b/internal/tui3/manual_devices_test.go index 4fc99c590a..71cdb298d2 100644 --- a/internal/tui3/manual_devices_test.go +++ b/internal/tui3/manual_devices_test.go @@ -29,7 +29,7 @@ func TestEverySentenceOfTheDeviceScreensIsOnItsPage(t *testing.T) { "naming-a-device": {devicesNameKeys, devicesNameAsk + "atlas▏", devicesNameOwn}, "continuing-a-chat-on-another-computer": { chatlist.SyncPaused, chatlist.SyncRestored, - continueAsk, continueStay, continueFails, continueUnreachable, lostRaceWord, chatlist.Unreachable, + continueAsk, continueStay, continueRetry, continueAlreadyRunning, continuing, continueFails, continueStoreBusy, continueUnreachable, lostRaceWord, chatlist.Unreachable, fmt.Sprintf(resumeAskFormat, "tower"), resumeYes, resumeNo, chatlist.Moved("tower", 3200e6, true), chatlist.Moved("", 3200e6, false), chatlist.ArrivedHead("tower"), chatlist.OfferSetUp, chatlist.OfferNotNow, diff --git a/internal/tui3/wormhole_contract_test.go b/internal/tui3/wormhole_contract_test.go index 4b6a0e90a3..9837d3f40c 100644 --- a/internal/tui3/wormhole_contract_test.go +++ b/internal/tui3/wormhole_contract_test.go @@ -22,6 +22,9 @@ func TestC5OnlineContinueOpensTheArrivedCopy(t *testing.T) { a := lab.app(mine) a.width, a.height = 200, 70 drain(t, a, a.openHome()) + if _, ok := a.homeLineOf("c-run"); !ok { + t.Fatal("fixture did not expose the local stale row") + } at := filepath.Join(lab.root, "copies", "c-run") work := filepath.Join(at, "work") transcript := filepath.Join(at, ".cell", "transcript.jsonl") @@ -45,8 +48,8 @@ func TestC5OnlineContinueOpensTheArrivedCopy(t *testing.T) { } }} drain(t, a, a.askMachines()) - if _, ok := a.homeLineOf("c-run"); !ok { - t.Fatal("fixture did not expose the local stale row") + if local, remote := rowsOf(a, "c-run"); local != 0 || remote != 1 { + t.Fatalf("local stale row and sync row did not merge into one row: local=%d remote=%d", local, remote) } standOn(t, a, "c-run") drive(t, a, key("enter"))