From 3a119f026ec34d7797f85540d47a46be37fe8044 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Wed, 28 Oct 2026 14:46:24 -0400 Subject: [PATCH] app: leave without waiting for crew member opens Quitting's optional file cleanup took every agent's gate, so a crew member still opening (its host's welcome slow or missing) held the engine past the desktop's stop. Clean up only conversations whose gate is free; a pending open keeps its files. Held conversations keep the quit and resume road. Closing the app's stdin now arms the leave bound at Close entry, before the backend lock, as the first SIGTERM does; before, EOF armed it only after taking that lock. Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/codeaf/app.go | 20 ++- cmd/codeaf/appcrewleave_test.go | 155 ++++++++++++++++++ .../unreleased/0-app-leave-with-crews.md | 14 ++ .../manual/chat/running-from-the-terminal.md | 6 + 4 files changed, 193 insertions(+), 2 deletions(-) create mode 100644 cmd/codeaf/appcrewleave_test.go create mode 100644 docs/changes/unreleased/0-app-leave-with-crews.md diff --git a/cmd/codeaf/app.go b/cmd/codeaf/app.go index 54e4d9cfa0..b76bf71b1e 100644 --- a/cmd/codeaf/app.go +++ b/cmd/codeaf/app.go @@ -147,6 +147,7 @@ func runApp(args []string) error { defer config.UseAppCredentials(config.NewAppCredentials())() server := appwire.NewServer(wire) backend := newAppBackend(server, appOpenInProcess) + backend.leaving = leaving backend.useSyncDrives(syncDrives) // A chat's banked pipeline line takes the editor's road while the app // holds the factory (factory_recipe_road.go). @@ -616,6 +617,8 @@ type appBackend struct { // pipe closing, its shutdown request, a signal or the app's process ending. // runApp bounds the leaving from that moment (appleave.go). left chan struct{} + // leaving is runApp's cancellation, installed before the backend starts. + leaving context.CancelFunc // spent is the last figure spend.changed said, so it is said on a change. spent float64 // mint makes a new conversation's folder and answers its transcript path: @@ -4247,6 +4250,10 @@ func appMemberAgent(member teamstore.Member) string { // Close lets every conversation go and stops the watchers. func (b *appBackend) Close() error { + // EOF must arm the process bound before any backend lock or crew wait. + if b.leaving != nil { + b.leaving() + } already := false var crews *appCrewRun var agents []*appAgent @@ -4343,11 +4350,20 @@ func (b *appBackend) Close() error { }) } wg.Wait() - b.closeReferences(agents) + // An unfinished crew open owns its gate but has no held conversation to + // release. Optional file cleanup must not join that open; retain its files. + var settled []*appAgent + for _, a := range agents { + if a.gate.TryLock() { + a.gate.Unlock() + settled = append(settled, a) + } + } + b.closeReferences(settled) // A BLANK CONVERSATION LIVES NO LONGER THAN THE PROCESS THAT MADE IT // (appblank.go): with everything let go, each one this process made that // nobody said anything in is taken away. - for _, a := range agents { + for _, a := range settled { b.reapBlank(a) } b.closeCollections() diff --git a/cmd/codeaf/appcrewleave_test.go b/cmd/codeaf/appcrewleave_test.go new file mode 100644 index 0000000000..44813d5d33 --- /dev/null +++ b/cmd/codeaf/appcrewleave_test.go @@ -0,0 +1,155 @@ +package main + +import ( + "context" + "io" + "os" + "reflect" + "runtime" + "strings" + "sync" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/appwire" +) + +// EOF must arm the same process bound as a signal before Close can wait on +// the backend lock. A crew opening or watcher must not keep that bound unarmed. +func TestAppEOFArmsLeaveBeforeBackendLock(t *testing.T) { + ctx, leaving := context.WithCancel(context.Background()) + defer leaving() + server := appwire.NewServer(io.Discard) + b := newFakeBackend(server, nil) + b.leaving = leaving + b.warmed = make(chan struct{}) + close(b.warmed) + b.mu.Lock() + returned := make(chan struct{}) + served := make(chan error, 1) + go func() { + served <- server.Serve(ctx, strings.NewReader(""), appwire.Hello{}, b) + close(returned) + }() + ended := make(chan struct{}) + go leaveWithin(ctx, b.left, returned, time.Millisecond, func() { close(ended) }) + select { + case <-ended: + case <-time.After(time.Second): + stack := make([]byte, 64<<10) + t.Errorf("EOF left the leave bound unarmed while Close waited for the backend lock:\n%s", stack[:runtime.Stack(stack, true)]) + } + b.mu.Unlock() + select { + case err := <-served: + if err != nil { + t.Fatal(err) + } + case <-time.After(time.Second): + t.Fatal("the app did not finish after the backend lock was released") + } +} + +type quitCrewConversation struct { + *fakeConversation + stop sync.Once +} + +func (c *quitCrewConversation) Interrupt() { + c.fakeConversation.Interrupt() + c.stop.Do(func() { close(c.hold) }) +} + +func TestAppCloseHeldCrewKeepsMemberResumeFacts(t *testing.T) { + c := newAppMemberCrew(t) + a := c.backend.agent(c.member) + a.gate.Lock() + c.conv.hold = make(chan struct{}) + conv := &quitCrewConversation{fakeConversation: c.conv} + released := make(chan struct{}) + c.backend.locked(func() { + a.live.conn.Conv = conv + a.live.conn.Assembled = true + a.live.conn.Release = func() { close(released) } + }) + a.gate.Unlock() + defer conv.Interrupt() + if _, err := c.backend.Send(c.member, "continue the member's work", ""); err != nil { + t.Fatal(err) + } + closed := make(chan error, 1) + go func() { closed <- c.backend.Close() }() + select { + case err := <-closed: + if err != nil { + t.Fatal(err) + } + case <-time.After(time.Second): + t.Fatal("closing a held crew waited for its member's turn") + } + select { + case <-released: + default: + t.Error("the held member was not released") + } + select { + case <-c.run.done: + default: + t.Error("the crew watcher outlived Close") + } + conv.mu.Lock() + interrupted := conv.interrupted + conv.mu.Unlock() + if interrupted != 1 { + t.Errorf("member interrupted %d times, want once", interrupted) + } + if err := c.backend.state.read(func(f *appStateFile) { + if !reflect.DeepEqual(f.ResumeAfterQuit, []string{c.member}) || !reflect.DeepEqual(f.QuitReplies, []string{c.member}) { + t.Errorf("member quit facts = resumes %v, replies %v", f.ResumeAfterQuit, f.QuitReplies) + } + }); err != nil { + t.Fatal(err) + } +} + +func TestAppCloseDoesNotWaitForCrewMemberOpening(t *testing.T) { + b, _, paths, _ := lazyBackend(t, 2) + run := b.crews() + entered, release := make(chan struct{}), make(chan struct{}) + member := appAgentID(paths[1]) + defer func() { + close(release) + a := b.agent(member) + a.gate.Lock() + a.gate.Unlock() + }() + before, err := os.ReadFile(paths[1]) + if err != nil { + t.Fatal(err) + } + b.open = func(appOpenAsk) (*appConn, error) { + close(entered) + <-release + return nil, appwire.Errorf(appwire.CodeClosed, "the opener stopped") + } + b.holdMembers(run, map[string][]string{member: {run.teams[0].ID}}) + select { + case <-entered: + case <-time.After(time.Second): + t.Fatal("the crew did not try to open its member") + } + closed := make(chan error, 1) + go func() { closed <- b.Close() }() + select { + case err := <-closed: + if err != nil { + t.Fatal(err) + } + case <-time.After(time.Second): + stack := make([]byte, 128<<10) + t.Fatalf("Close waited for a crew member's pending open:\n%s", stack[:runtime.Stack(stack, true)]) + } + if after, err := os.ReadFile(paths[1]); err != nil || string(after) != string(before) { + t.Errorf("closing changed the opening member's record: %v", err) + } +} diff --git a/docs/changes/unreleased/0-app-leave-with-crews.md b/docs/changes/unreleased/0-app-leave-with-crews.md new file mode 100644 index 0000000000..241793209c --- /dev/null +++ b/docs/changes/unreleased/0-app-leave-with-crews.md @@ -0,0 +1,14 @@ +--- +kind: fixed +title: The app can quit while a crew member is still opening +pr: 0 +surface: [engine, docs] +invalidates: + - "Optional file cleanup on app quit waited on every conversation's gate, including a crew member still opening. Cleanup now keeps a busy member's files and lets the engine leave." + - "Closing the desktop input pipe armed the leave bound only after acquiring the backend lock. It now starts the same bounded leave as the first SIGTERM before waiting for that lock." +--- + +Held conversations still record their quit and resume facts, interrupt running +turns and flush their records before release. Settled conversations keep the +existing attachment and blank-conversation cleanup. Terminal behavior and the +overall shutdown allowance are unchanged. diff --git a/internal/manual/chat/running-from-the-terminal.md b/internal/manual/chat/running-from-the-terminal.md index 47fda3a0da..f65601499d 100644 --- a/internal/manual/chat/running-from-the-terminal.md +++ b/internal/manual/chat/running-from-the-terminal.md @@ -541,6 +541,12 @@ hold so another computer can continue. Each saving boundary waits up to 30 secon A successful save is quiet; a definite final-save failure is kept in the engine log and leaves unsaved turns on this computer. +Crew members follow the same quit and resume rules. A member still opening +does not hold up quitting for optional file cleanup; its files are kept. +Closing the desktop's input pipe starts the same bounded shutdown as a first +SIGTERM, including while the engine is waiting for a lock. A second SIGTERM +ends the process immediately. + ## When the desktop app closes unexpectedly — a reply stuck working, carrying on after it closed If the desktop app closes unexpectedly during a reply, opening it again carries