cellsync: let a chat go on quit only after its last save uploaded

docs/changes/unreleased/1918-quit-keeps-a-chat-whose-last-upload-failed.md

@@ -0,0 +1,8 @@
+---
+kind: fixed
+title: quitting while the last upload fails no longer lets another computer open an older save
+pr: 1918
+surface: [chat, remote]
+invalidates:
+  - "Quitting released a moved chat's lease even when its final upload failed, so the relay announced a clean quit and another paired computer could continue automatically from the older saved head. Quit now keeps the lease until the last save uploads; the unsent turns stay on this computer."
+---

internal/cellsync/batch.go

@@ -359,8 +359,12 @@ func realSleep(ctx context.Context, d time.Duration) error {
 func (b *Batcher) Close(ctx context.Context) error {
 	b.flushMu.Lock()
 	defer b.flushMu.Unlock()
-	err := b.drain(ctx)
-	return errors.Join(err, b.release(ctx))
+	// A release is the cross-computer clean-quit signal. Publishing must finish
+	// first; otherwise another home could automatically open an older head.
+	if err := b.drain(ctx); err != nil {
+		return err
+	}
+	return b.release(ctx)
 }
 
 // release gives the lease up so another device can take over at once. A lease

internal/cellsync/quit_contract_test.go

@@ -0,0 +1,39 @@
+package cellsync
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/Agent-Field/codeaf/internal/blobstore"
+	"github.com/Agent-Field/codeaf/internal/directory"
+)
+
+// C28/C31 require the final saved head before a successful quit marker.
+// A reachable directory must not advertise an incomplete upload as a clean quit.
+func TestC28C31FailedFinalUploadDoesNotRelease(t *testing.T) {
+	r := newRig(t)
+	before := r.publishFirst(map[string]string{"work.txt": "published"})
+	b := r.batcher()
+	latest := r.seal(map[string]string{"work.txt": "local final turn"})
+	note(b, latest)
+	r.store.set(blobstore.ErrUnreachable, nil)
+	if err := b.Close(context.Background()); !errors.Is(err, blobstore.ErrUnreachable) {
+		t.Fatalf("close error %v", err)
+	}
+	v := r.head(cellID)
+	if v.Head != before || v.Lease.Expires == 0 || v.Lease.ReleasedAt != 0 || b.Pending() != 1 {
+		t.Fatalf("failed quit exposed a release or lost pending work: %+v pending %d", v, b.Pending())
+	}
+	if _, err := r.dir.For(devB).Acquire(context.Background(), cellID, directory.AcquireOpts{}); !errors.Is(err, directory.ErrLeaseHeld) {
+		t.Fatalf("failed quit became available: %v", err)
+	}
+	r.store.set(nil, nil)
+	if err := b.Close(context.Background()); err != nil {
+		t.Fatal(err)
+	}
+	v = r.head(cellID)
+	if v.Head != latest || v.Lease.Expires != 0 || v.Lease.ReleasedAt == 0 || b.Pending() != 0 {
+		t.Fatalf("successful retry did not publish before release: %+v", v)
+	}
+}

internal/manual/chat/continuing-a-chat-on-another-computer.md

@@ -203,6 +203,8 @@ A command's result returns after its call log is written, while its file capture
 
 A wait lasts at most 30 seconds. A timeout says `still saving this chat on this computer: context deadline exceeded`; the pending save is not silently treated as done. The saving status reports failures, and quitting reports an error if saving could not finish. A final disk-flush failure says `codeaf: could not finish saving this chat:` followed by the cause and exits unsuccessfully.
 
+If the last upload fails when you quit, for example because the connection dropped, this computer does not let the chat go. Your other computer keeps showing it as running here and does not open the older save by itself; the unsent turns stay on this computer until an upload succeeds.
+
 ## I pressed alt+m before the other chat appeared — still arriving, home on a second computer
 
 Device presence can arrive before its list of chats. If you press `alt+m` early, home says `chats are still arriving from your other computer; this opens when they are ready`. It keeps your request and opens the ordinary confirmation once the chat arrives; it never takes the chat without that confirmation. A recent chat left on another computer is offered when home opens, including one that computer closed cleanly.

internal/syncsetup/quit_contract_test.go

@@ -0,0 +1,49 @@
+package syncsetup
+
+import (
+	"context"
+	"errors"
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/Agent-Field/codeaf/internal/directory"
+	"github.com/Agent-Field/codeaf/internal/executor"
+)
+
+// C28/C31 drive the real engine and recorded call path. Failing only the
+// upload leaves the directory reachable, so an incorrect clean-quit marker
+// cannot hide behind a network failure affecting both wires.
+func TestC28C31FailedQuitKeepsFinalRecordsAndDoesNotRelease(t *testing.T) {
+	h := newTwoHomes(t)
+	a := h.openA()
+	a.mustSay("first published answer")
+	h.durable(h.cell.ID, h.cell)
+	before, e := h.b.Dir.Cell(context.Background(), h.cell.ID)
+	if e != nil {
+		t.Fatal(e)
+	}
+	h.block.down.Store(true)
+	a.mustSay("final locally saved answer")
+	executor.Settle(context.Background(), a.seat)
+	if e = a.drive.Close(context.Background()); e == nil {
+		t.Fatal("failed final upload reported a successful quit")
+	}
+	v, e := h.b.Dir.Cell(context.Background(), h.cell.ID)
+	if e != nil {
+		t.Fatal(e)
+	}
+	if v.Cell.Head != before.Cell.Head || v.Cell.Lease.Expires == 0 || v.Cell.Lease.ReleasedAt != 0 {
+		t.Fatalf("failed quit advertised a release: %+v", v)
+	}
+	if _, e = h.b.Dir.Acquire(context.Background(), h.cell.ID, directory.AcquireOpts{}); !errors.Is(e, directory.ErrLeaseHeld) {
+		t.Fatalf("failed quit made old head automatically takeable: %v", e)
+	}
+	raw, e := os.ReadFile(transcriptOf(h.cell))
+	if e != nil {
+		t.Fatal(e)
+	}
+	if !strings.Contains(string(raw), "final locally saved answer") {
+		t.Fatal("final call/conversation record was lost")
+	}
+}