diff --git a/docs/changes/unreleased/2091-displaced-window-keeps-turns.md b/docs/changes/unreleased/2091-displaced-window-keeps-turns.md new file mode 100644 index 0000000000..3e6ff5fced --- /dev/null +++ b/docs/changes/unreleased/2091-displaced-window-keeps-turns.md @@ -0,0 +1,11 @@ +--- +kind: fixed +title: A displaced window keeps its unsent turns, and a taken chat can be taken again +pr: 2091 +surface: [remote, chat, docs] +invalidates: + - "When an ownership check found another device holding the chat, Drive.CheckLease made the window a viewer without telling the batcher, so unsent turns were not kept as a branch then; now the hold is retired at once and Batcher.Displaced resolves pending work before the window names the new holder, once." + - "A publish refused with a stale fence announced 'another device continued this chat' and then ' continued this chat'; OnLost now only retires the hold, and the window speaks once, after the branch." + - "Materialize left the sender's earlier .cell/published.json in place, naming a head this store never fetched, so the next take refused with 'snapshot … is not held in this store'; Materialize now records the received head as published." + - "The conversation tip read 'space space takes you back to home'; it now reads 'space space brings you back to home'." +--- diff --git a/internal/cellstore/syncops.go b/internal/cellstore/syncops.go index 44f4fdb4f7..b51b6ef5c7 100644 --- a/internal/cellstore/syncops.go +++ b/internal/cellstore/syncops.go @@ -288,6 +288,12 @@ func (e SyncEngine) Materialize(ctx context.Context, c cell.Cell, head string) e if err := noteAdopted(c, head); err != nil { return err } + // A snapshot sealed after the first publish carries the sender's earlier + // upload note, whose snapshot this store may never have fetched; record + // the received head so local-work checks compare against a snapshot held here. + if err := NotePublished(c, head); err != nil { + return err + } return noteHeld(c, e.holdDir(c), mergeHeld(earlier, out.Held)) } diff --git a/internal/cellsync/batch.go b/internal/cellsync/batch.go index 49d30391dc..d1d1b979cd 100644 --- a/internal/cellsync/batch.go +++ b/internal/cellsync/batch.go @@ -516,6 +516,17 @@ func (b *Batcher) beat(ctx context.Context) { } } +// Displaced records that an ownership read found this writer's hold gone: new +// work is refused immediately and any pending turns are resolved under flushMu +// so an in-flight publish and a watch wakeup never announce displacement twice. +func (b *Batcher) Displaced(ctx context.Context) { + seen, _, stale := b.view() + if stale { + return + } + b.lost(ctx, seen) +} + // lost handles a heartbeat that found the lease of seen gone. It waits for any // flush in flight, and does nothing if that flush already branched. When the // branch fails the state stays stale and the next flush retries it. @@ -525,7 +536,7 @@ func (b *Batcher) lost(ctx context.Context, seen Driving) { if b.isClosing() { return } - if now, _, _ := b.view(); now.ID() != seen.ID() || now.Fence != seen.Fence { + if now, _, stale := b.view(); stale || now.ID() != seen.ID() || now.Fence != seen.Fence { return } b.setStale() diff --git a/internal/e2e/pairjourney_steps_e2e_test.go b/internal/e2e/pairjourney_steps_e2e_test.go index 92f0b11e7d..b4ca3f1e94 100644 --- a/internal/e2e/pairjourney_steps_e2e_test.go +++ b/internal/e2e/pairjourney_steps_e2e_test.go @@ -34,7 +34,7 @@ func journeyRun(j *journey, env []string, homeA, homeB, wsA, wsB string, online a.keys("Space") screen, ok := waitPlain(a, 12*time.Second, "Add another device") j.check(ok, "the home screen never offered another machine") - j.check(strings.Contains(screen, "pick up your work anywhere"), "pitch missing") + j.check(strings.Contains(screen, say(t, "addMachinePitch")), "pitch missing") j.see(a) }) diff --git a/internal/e2e/tuiwords_test.go b/internal/e2e/tuiwords_test.go index afcb8d8357..ba7fd08756 100644 --- a/internal/e2e/tuiwords_test.go +++ b/internal/e2e/tuiwords_test.go @@ -1227,6 +1227,11 @@ var tuiWords = map[string]tuiWord{ why: "the dim line the conversation says after esc, which is the other half of a front door: " + "a person who declined is told the next direct road rather than left on an empty screen", }, + "addMachinePitch": { + screen: "Pick up your work anywhere, exactly where you left it.", + why: "the pitch on home's add-another-device card (homeband_addmachine.go's addMachinePitch), " + + "read by the pairing journey so a respelled card fails the untagged word gate before tmux runs", + }, } // say is how the tmux suite reaches a literal, and THE ONLY WAY IT MAY. diff --git a/internal/manual/chat/hints-and-tips.md b/internal/manual/chat/hints-and-tips.md index ca7eb7d012..5676b66ff0 100644 --- a/internal/manual/chat/hints-and-tips.md +++ b/internal/manual/chat/hints-and-tips.md @@ -144,7 +144,7 @@ build if the two disagree), so a tip you saw is on it word for word. has three or more earlier conversations. Retired the first time the automations place opens, by any door (`/automations`, `alt+7`, a row that leads there), or the first time an automation is proposed to you on a card in a conversation. -- `space space takes you back to home` — after the first exchange, in a conversation only +- `space space brings you back to home` — after the first exchange, in a conversation only (never on home itself). Retired the first time two spaces in an empty box open home, from a conversation or from a place; reaching home by `/home` or the tab does not retire it. It is the first tip a conversation says, ahead of `/task`. diff --git a/internal/manual/chat/home.md b/internal/manual/chat/home.md index 58779205e1..265037a99a 100644 --- a/internal/manual/chat/home.md +++ b/internal/manual/chat/home.md @@ -1856,7 +1856,7 @@ advertised. (Until 2026-09-17 these words were the right end of the rule above t **It stays when a tip is showing.** A conversation's tip covers the project at the keys row's right end, never the keys. Until 2026-09-24 a tip took the whole row and the door went with it. One of the tips is about the door itself: -`space space takes you back to home`. +`space space brings you back to home`. **You can click it.** A press on the words `space space home` opens home; a press on the rule beside them is a press on a rule. diff --git a/internal/syncsetup/drive.go b/internal/syncsetup/drive.go index 71cc7013a6..4490a3a70b 100644 --- a/internal/syncsetup/drive.go +++ b/internal/syncsetup/drive.go @@ -60,11 +60,13 @@ type Drive struct { sent string // fingerprint of the vault as last sent; touched by the flush loop and Close only - mu sync.Mutex - line string // the superseded line, empty while this device drives - spoken bool - sealMu sync.Mutex - stopped bool + mu sync.Mutex + line string // the superseded line, empty until pending work is resolved and the notice said + lost bool // true as soon as this writer learns its hold is gone, before branching finishes + announced bool // true once becomeViewer has claimed announcing displacement + spoken bool + sealMu sync.Mutex + stopped bool } // Drive starts the drive side of c. The lease is acquired when the chat has a @@ -332,7 +334,7 @@ func (s *Sync) batcher(eng cellstore.Engine, c cell.Cell, drv cellsync.Driving, Info: d.info, OnFlush: d.flushed(), OnSuperseded: d.becomeViewer, - OnLost: func() { d.refusal(directory.ErrFenceStale) }, + OnLost: d.loseHold, OnError: d.refusal, Sleep: d.opt.Sleep, Liveness: s.Liveness(), @@ -349,28 +351,47 @@ func (d *Drive) info() cellsync.PublishInfo { return d.sync.publishInfo(d.cell, title) } -// becomeViewer is the Batcher's word that another device took the chat over: -// from now on this window only shows it, and says who has it. -func (d *Drive) becomeViewer(s cellsync.Superseded) { +// loseHold retires this writer's permission to run tools and clears its send +// receipt at once, while leaving the viewer line and notice until pending work +// has been preserved as a branch and the new holder named. +func (d *Drive) loseHold() { d.mu.Lock() - changed := d.line == "" - d.line = chatlist.Superseded(unknownDevice) + changed := !d.lost && d.line == "" + d.lost = true expected := sendLease{Cell: d.directoryID, Fence: d.recordedFence} d.mu.Unlock() - if err := d.sync.forgetSendLease(d.cell.ID, expected); err != nil { - d.notice(err.Error()) + if d.sync != nil { + if err := d.sync.forgetSendLease(d.cell.ID, expected); err != nil { + d.notice(err.Error()) + } } if changed && d.opt.OnRenewalChange != nil { d.opt.OnRenewalChange() } +} + +func (d *Drive) displaced() bool { + d.mu.Lock() + defer d.mu.Unlock() + return d.lost || d.line != "" +} + +// becomeViewer is the Batcher's word that another device took the chat over: +// from now on this window only shows it, and says who has it. +func (d *Drive) becomeViewer(s cellsync.Superseded) { + d.loseHold() + d.mu.Lock() + if d.announced { + d.mu.Unlock() + return + } + d.announced = true + d.mu.Unlock() device := unknownDevice - if s.By != "" && s.By != unknownDevice { + if s.By != "" && s.By != unknownDevice && d.sync != nil { device = d.sync.deviceName(s.By) } line := chatlist.Superseded(device) - d.mu.Lock() - d.line = line - d.mu.Unlock() // Refuse new work immediately, then preserve the pending save before // announcing displacement. A routine ownership check does no fsync (C28). if d.batcher != nil { @@ -384,6 +405,9 @@ func (d *Drive) becomeViewer(s cellsync.Superseded) { d.notice(chatlist.KeptEdits(s.Turns, d.opt.DeviceName)) } d.notice(line) + d.mu.Lock() + d.line = line + d.mu.Unlock() } // refusal shows the person-facing line of a refused request, which the @@ -430,7 +454,7 @@ func (d *Drive) Viewer() (string, bool) { // RenewalStatus reads this writer's cached renewal health without network work. func (d *Drive) RenewalStatus() (time.Time, time.Duration, bool) { - if _, viewer := d.Viewer(); viewer || d.batcher == nil || d.sync.removed.Load() { + if d.displaced() || d.batcher == nil || d.sync.removed.Load() { return time.Time{}, 0, false } return d.batcher.RenewalStatus() @@ -443,7 +467,7 @@ func (d *Drive) RenewalStatus() (time.Time, time.Duration, bool) { // directory that cannot be asked answers true, because a drive side that cannot // tell must not be thrown away for it. func (d *Drive) HoldsLease(ctx context.Context) bool { - if _, viewer := d.Viewer(); viewer || d.batcher == nil { + if d.displaced() || d.batcher == nil { return false } v, err := d.sync.Dir.Cell(ctx, d.directoryID) @@ -461,12 +485,17 @@ func (d *Drive) CheckLease(ctx context.Context) error { if err := cellstore.Captured(ctx, d.cell); err != nil { return err } - if _, viewer := d.Viewer(); viewer || d.batcher == nil { + if d.displaced() || d.batcher == nil { return nil } v, err := d.sync.Dir.Cell(ctx, d.directoryID) if err == nil && (v.Cell.Lease.Expires <= v.Now || v.Cell.Lease.Device != d.sync.Device.ID() || v.Cell.Lease.Fence != d.batcher.Fence()) { - d.becomeViewer(cellsync.Superseded{By: v.Cell.Lease.Device}) + // Retire the hold at once so no new seal starts, then wait for any seal + // already past Gate to note its turn before resolving pending work. + d.loseHold() + d.sealMu.Lock() + d.sealMu.Unlock() + d.batcher.Displaced(ctx) } return nil } @@ -489,6 +518,9 @@ func (d *Drive) Gate() error { if d.line != "" { return errors.New(d.line) } + if d.lost { + return errors.New(chatlist.Superseded(unknownDevice)) + } return nil } diff --git a/internal/syncsetup/drive_refusal_safety_test.go b/internal/syncsetup/drive_refusal_safety_test.go index 869a8eeb59..cb6f72ca81 100644 --- a/internal/syncsetup/drive_refusal_safety_test.go +++ b/internal/syncsetup/drive_refusal_safety_test.go @@ -1,20 +1,59 @@ package syncsetup import ( + "slices" "testing" "time" "github.com/Agent-Field/codeaf/internal/cellstore" + "github.com/Agent-Field/codeaf/internal/chatlist" "github.com/Agent-Field/codeaf/internal/directory" ) func TestAStaleDriveRefusalImmediatelyMakesTheWindowAViewer(t *testing.T) { // Refusal retires a saved send hold, so even this minimal drive needs an isolated home. - d := &Drive{sync: &Sync{Home: t.TempDir()}} + var notices []string + d := &Drive{ + sync: &Sync{Home: t.TempDir()}, + opt: DriveOptions{OnNotice: func(line string) { notices = append(notices, line) }}, + } + d.refusal(directory.ErrFenceStale) d.refusal(directory.ErrFenceStale) if d.Gate() == nil { t.Fatal("a refused drive still allows new work") } + want := chatlist.Superseded(unknownDevice) + if line, viewer := d.Viewer(); !viewer || line != want { + t.Fatalf("viewer = %q, %v; want %q, true", line, viewer, want) + } + if !slices.Equal(notices, []string{want}) { + t.Fatalf("notices = %v, want [%q] once", notices, want) + } +} + +func TestStalePublishRefusalMakesViewerAndNotifiesOnce(t *testing.T) { + h := newTwoHomes(t) + ctx := t.Context() + a := h.openA() + a.mustSay("durable") + h.durable(h.cell.ID, h.cell) + h.block.set(true) + a.mustSay("orphaned") + h.lapse() + mustNil(t, h.b.putDevice(ctx, nameB)) + if _, err := h.continuerB().Take(ctx, h.cell.ID); err != nil { + t.Fatal(err) + } + h.block.set(false) + want := chatlist.Superseded(nameB) + waitFor(t, "A to become a viewer after stale publish", func() bool { _, viewer := a.drive.Viewer(); return viewer }) + if line, viewer := a.drive.Viewer(); !viewer || line != want { + t.Fatalf("viewer = %q, %v; want %q, true", line, viewer, want) + } + kept := chatlist.KeptEdits(1, nameA) + if got := h.wall.all(); !slices.Equal(got, []string{kept, want}) { + t.Fatalf("notices = %v, want [%q %q]", got, kept, want) + } } func TestTheSendProofNamesTheDrivesActualFence(t *testing.T) { diff --git a/internal/syncsetup/drive_watch.go b/internal/syncsetup/drive_watch.go index 235706e573..cbbe306f6b 100644 --- a/internal/syncsetup/drive_watch.go +++ b/internal/syncsetup/drive_watch.go @@ -19,7 +19,7 @@ func (d *Drive) followOwnership(ctx context.Context) { _ = d.CheckLease(read) cancel() } - if _, viewer := d.Viewer(); viewer || state.Refused != nil { + if d.displaced() || state.Refused != nil { return } select { diff --git a/internal/syncsetup/leadership_native_test.go b/internal/syncsetup/leadership_native_test.go index 6e4a58954b..1b0e765f25 100644 --- a/internal/syncsetup/leadership_native_test.go +++ b/internal/syncsetup/leadership_native_test.go @@ -39,6 +39,10 @@ func personalLeader(t *testing.T, r *memberRig, s *Sync) cs.LeadershipReceipt { if status.Leader != receipt.Node || !slices.Equal(status.Configuration.Voters, []uint64{1, 2, receipt.Node}) || status.Configuration.Joint() || !slices.Contains(status.Configuration.Learners, 3) || status.Membership.Leadership == nil || !status.Membership.Leadership.Completed { t.Fatalf("R6 transfer lacks durable settlement/elected leader: %+v %+v", receipt, status) } + // Status returns as soon as one anchor voter answers ReadIndex quorum, while + // the remaining peer responses and their cursor acknowledgments finish on + // the member socket; settle them before callers snapshot post-decision traffic. + waitMember(t, s, func(m *personalMember) bool { return m.idle }) return receipt } diff --git a/internal/syncsetup/sendlease.go b/internal/syncsetup/sendlease.go index b78455918f..4f0085af7a 100644 --- a/internal/syncsetup/sendlease.go +++ b/internal/syncsetup/sendlease.go @@ -48,7 +48,7 @@ func (d *Drive) recordSendLease() error { fence := d.batcher.Fence() d.mu.Lock() defer d.mu.Unlock() - if d.line != "" || fence == 0 || fence == d.recordedFence { + if d.lost || d.line != "" || fence == 0 || fence == d.recordedFence { return nil } if err := d.sync.recordSendLease(d.cell.ID, sendLease{Cell: d.directoryID, Fence: fence}); err != nil { diff --git a/internal/tui3/chattip_test.go b/internal/tui3/chattip_test.go index d6a59fb460..1960f34963 100644 --- a/internal/tui3/chattip_test.go +++ b/internal/tui3/chattip_test.go @@ -251,7 +251,7 @@ func TestTheWayHomeTipIsSaidAwayFromHomeOnly(t *testing.T) { if row.id == "" { t.Fatal("the way-home tip is not on the table") } - if row.text != "space space takes you back to home" || row.retire != eventHomeGesture { + if row.text != "space space brings you back to home" || row.retire != eventHomeGesture { t.Fatalf("the way-home tip reads %q and retires on %q", row.text, row.retire) } a, _ := chatTipLab(t) diff --git a/internal/tui3/notice.go b/internal/tui3/notice.go index 355bc7a2ae..4593088bdf 100644 --- a/internal/tui3/notice.go +++ b/internal/tui3/notice.go @@ -402,7 +402,7 @@ var notices = []notice{ // it. id: "home-by-two-spaces", slot: slotHint, armed: awayFromHome, - text: "space space takes you back to home", + text: "space space brings you back to home", retire: eventHomeGesture, }, {