From a797616e57acb7c8e932d2c40d9c2dd823990567 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Fri, 6 Nov 2026 11:02:37 -0500 Subject: [PATCH 1/3] fix(sync): join the ownership watcher before releasing an idle chat Give the ownership watcher a child context and a separate join so its own lease release cannot announce a viewer location after idle closure. Keep final publication on the existing run context until Batcher.Close joins it. Cancelling that shared context early breaks the closing-upload guard. Reuse the deterministic release regression and add run-cancellation coverage. Keep takeover, failed-take recovery and work refusal semantics unchanged. Co-Authored-By: Claude Opus 5.5 --- .../unreleased/0-own-idle-chat-send.md | 16 ++++ internal/manual/chat/desktop-sync.md | 2 + internal/syncsetup/drive.go | 34 +++++-- internal/syncsetup/drive_own_release_test.go | 90 +++++++++++++++++++ internal/syncsetup/drive_watch.go | 10 +++ internal/syncsetup/ownership_undertest.go | 19 ++++ 6 files changed, 164 insertions(+), 7 deletions(-) create mode 100644 docs/changes/unreleased/0-own-idle-chat-send.md create mode 100644 internal/syncsetup/drive_own_release_test.go create mode 100644 internal/syncsetup/ownership_undertest.go diff --git a/docs/changes/unreleased/0-own-idle-chat-send.md b/docs/changes/unreleased/0-own-idle-chat-send.md new file mode 100644 index 0000000000..0a83aea34d --- /dev/null +++ b/docs/changes/unreleased/0-own-idle-chat-send.md @@ -0,0 +1,16 @@ +--- +kind: fixed +title: Typing into your own idle chat no longer says it's available to continue here +pr: 0 +surface: [app, sync] +invalidates: [] +--- + +Sending a message into your own idle background chat opens it again when its +hold is still free. Previously, the old ownership watcher could mistake the +chat's own release for losing ownership. Send could then refuse with +`This chat is available to continue here.` + +Closing now stops that watcher before releasing the hold, while still waiting +for the final upload to finish. Checks for another computer taking the chat +and recovery after a failed continuation keep their existing rules. diff --git a/internal/manual/chat/desktop-sync.md b/internal/manual/chat/desktop-sync.md index e8566c2d47..a60abc033e 100644 --- a/internal/manual/chat/desktop-sync.md +++ b/internal/manual/chat/desktop-sync.md @@ -202,6 +202,8 @@ ownership changes or repeat unchanged chat rows. A conversation opened only for background work is let go after ten idle minutes; the sweep checks every 30 seconds. It stays released until work needs it again. Sending, queueing or retrying opens it through the normal ownership check. +Sending into your own released chat opens it again when its hold is still free; +its own idle release does not require Continue here. Steering still needs a running turn. A task or job can also open it. Running turns, unanswered questions, held crew members and kept services retain their conversations. If another computer continues a chat, the existing Continue here diff --git a/internal/syncsetup/drive.go b/internal/syncsetup/drive.go index 5fb136c9bb..548b2f4d27 100644 --- a/internal/syncsetup/drive.go +++ b/internal/syncsetup/drive.go @@ -59,12 +59,16 @@ type Drive struct { directoryID string recordedFence uint64 - cancel context.CancelFunc - done chan struct{} - once sync.Once - closeErr error - publicationDone func() - ownership chan struct{} + cancel context.CancelFunc + done chan struct{} + once sync.Once + closeErr error + publicationDone func() + watcherCancel context.CancelFunc + watcherDone chan struct{} + ownership chan struct{} + beforeOwnershipCheck func(context.Context) func() + afterReleaseInTests func() sent string // fingerprint of the vault as last sent; touched by the flush loop and Close only @@ -129,10 +133,17 @@ func (s *Sync) Drive(ctx context.Context, eng cellstore.Engine, c cell.Cell, opt } run, cancel := context.WithCancel(context.WithoutCancel(ctx)) d.cancel = cancel + watch, cancelWatch := context.WithCancel(run) + d.watcherCancel = cancelWatch + d.watcherDone = make(chan struct{}) go func() { defer close(d.done) var work sync.WaitGroup - work.Go(func() { d.followOwnership(run) }) + work.Go(func() { + defer close(d.watcherDone) + defer cancelWatch() + d.followOwnership(watch) + }) _ = d.batcher.Run(run) work.Wait() }() @@ -779,6 +790,12 @@ func (d *Drive) Close(ctx context.Context) error { return } stop := context.AfterFunc(ctx, d.cancel) + // Stop ownership readings before our own release can look like displacement. + // The batcher keeps its run context until the final publish has joined. + if d.watcherCancel != nil { + d.watcherCancel() + <-d.watcherDone + } if pending == nil && !d.sync.isRemoved() { // Join the publish before stopping its request. Canceling a committed // request can lose its reply and leave the next publish using an old head. @@ -786,6 +803,9 @@ func (d *Drive) Close(ctx context.Context) error { } else { d.closeErr = pending } + if d.afterReleaseInTests != nil { + d.afterReleaseInTests() + } d.cancel() <-d.done stop() diff --git a/internal/syncsetup/drive_own_release_test.go b/internal/syncsetup/drive_own_release_test.go new file mode 100644 index 0000000000..a560f1b6c1 --- /dev/null +++ b/internal/syncsetup/drive_own_release_test.go @@ -0,0 +1,90 @@ +package syncsetup + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/directory" +) + +func TestDriveOwnReleaseDoesNotBecomeViewer(t *testing.T) { + released, checked := make(chan struct{}), make(chan struct{}) + r := newDriveRig(t) + r.spokenFixture() + var notices noticeLog + c := r.startChat(¬ices) + c.mustSay() + waitFor(t, "published head", func() bool { return r.directoryHead(r.cell.ID) == r.head() }) + entered := make(chan struct{}) + var once sync.Once + c.drive.BeforeOwnershipCheckInTests(func(ctx context.Context) func() { + var after func() + once.Do(func() { + close(entered) + select { + case <-released: + case <-ctx.Done(): + } + after = func() { close(checked) } + }) + return after + }, func() { close(released); <-checked }) + // An unrelated directory change places the watcher at the read barrier. + if err := r.b.Dir.PutDevice(t.Context(), r.b.Device.ID(), directory.Device{V: 1, Name: "barrier"}); err != nil { + t.Fatal(err) + } + select { + case <-entered: + case <-t.Context().Done(): + t.Fatal(t.Context().Err()) + } + if err := c.drive.Close(t.Context()); err != nil { + t.Fatal(err) + } + if line, viewer := c.drive.Viewer(); viewer { + t.Fatalf("own clean release became viewer: %q", line) + } + if got := r.directoryHead(r.cell.ID); got != r.head() { + t.Fatalf("close head = %q, want %q", got, r.head()) + } +} + +// Cancelling the run must stop its child watcher even without a Close call. +func TestDriveRunCancelStopsOwnershipWatcher(t *testing.T) { + r := newDriveRig(t) + r.spokenFixture() + var notices noticeLog + c := r.startChat(¬ices) + c.mustSay() + waitFor(t, "published head", func() bool { return r.directoryHead(r.cell.ID) == r.head() }) + entered := make(chan struct{}) + var once sync.Once + c.drive.BeforeOwnershipCheckInTests(func(ctx context.Context) func() { + once.Do(func() { + close(entered) + <-ctx.Done() + }) + return nil + }, nil) + if err := r.b.Dir.PutDevice(t.Context(), r.b.Device.ID(), directory.Device{V: 1, Name: "barrier"}); err != nil { + t.Fatal(err) + } + select { + case <-entered: + case <-time.After(5 * time.Second): + t.Fatal("ownership watcher did not enter its reading") + } + c.drive.cancel() + select { + case <-c.drive.watcherDone: + case <-time.After(5 * time.Second): + t.Fatal("run cancellation did not join the ownership watcher") + } + select { + case <-c.drive.done: + case <-time.After(5 * time.Second): + t.Fatal("run cancellation did not join the batcher") + } +} diff --git a/internal/syncsetup/drive_watch.go b/internal/syncsetup/drive_watch.go index bdc37c07de..56297ecc77 100644 --- a/internal/syncsetup/drive_watch.go +++ b/internal/syncsetup/drive_watch.go @@ -16,7 +16,17 @@ func (d *Drive) followOwnership(ctx context.Context) { if state.Version > version && d.batcher.Fence() != 0 { version = state.Version read, cancel := context.WithTimeout(ctx, requestTimeout) + d.mu.Lock() + before := d.beforeOwnershipCheck + d.mu.Unlock() + var after func() + if before != nil { + after = before(read) + } _ = d.CheckLease(read) + if after != nil { + after() + } cancel() } if state.Refused != nil { diff --git a/internal/syncsetup/ownership_undertest.go b/internal/syncsetup/ownership_undertest.go new file mode 100644 index 0000000000..d1d48ac321 --- /dev/null +++ b/internal/syncsetup/ownership_undertest.go @@ -0,0 +1,19 @@ +package syncsetup + +import ( + "context" + "testing" +) + +// BeforeOwnershipCheckInTests pauses this drive's watcher before its reading. +// The returned callback runs after the reading and its viewer notification. +// Install only while the drive is idle, and close the drive before cleanup. +func (d *Drive) BeforeOwnershipCheckInTests(before func(context.Context) func(), afterRelease func()) { + if !testing.Testing() { + panic("syncsetup: ownership hook requires a test binary") + } + d.mu.Lock() + d.beforeOwnershipCheck = before + d.afterReleaseInTests = afterRelease + d.mu.Unlock() +} From 94acdee8d6d9330b21626e391afcfdec5aa7e60b Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Fri, 6 Nov 2026 11:21:24 -0500 Subject: [PATCH 2/3] changelog: name the pull request Co-Authored-By: Claude Opus 5.5 --- .../{0-own-idle-chat-send.md => 2469-own-idle-chat-send.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename docs/changes/unreleased/{0-own-idle-chat-send.md => 2469-own-idle-chat-send.md} (98%) diff --git a/docs/changes/unreleased/0-own-idle-chat-send.md b/docs/changes/unreleased/2469-own-idle-chat-send.md similarity index 98% rename from docs/changes/unreleased/0-own-idle-chat-send.md rename to docs/changes/unreleased/2469-own-idle-chat-send.md index 0a83aea34d..95178ae9d6 100644 --- a/docs/changes/unreleased/0-own-idle-chat-send.md +++ b/docs/changes/unreleased/2469-own-idle-chat-send.md @@ -1,7 +1,7 @@ --- kind: fixed title: Typing into your own idle chat no longer says it's available to continue here -pr: 0 +pr: 2469 surface: [app, sync] invalidates: [] --- From 8c267c6af7696f6c8e8410feeae015c987986a34 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Fri, 6 Nov 2026 11:21:39 -0500 Subject: [PATCH 3/3] changelog: use a known surface Co-Authored-By: Claude Opus 5.5 --- docs/changes/unreleased/2469-own-idle-chat-send.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changes/unreleased/2469-own-idle-chat-send.md b/docs/changes/unreleased/2469-own-idle-chat-send.md index 95178ae9d6..7ef07132c3 100644 --- a/docs/changes/unreleased/2469-own-idle-chat-send.md +++ b/docs/changes/unreleased/2469-own-idle-chat-send.md @@ -2,7 +2,7 @@ kind: fixed title: Typing into your own idle chat no longer says it's available to continue here pr: 2469 -surface: [app, sync] +surface: [app, remote] invalidates: [] ---