Skip to content

Commit df6bae8

Browse files
fix(agent): stop title generation from outliving its run
Run spawned GenerateTitle on a goroutine with a detached context and never joined it. Nothing else joined it either: CancelAll waits only on activeRequests, which never contained the title goroutine, and the detached context means a cancel cannot reach it. So the goroutine outlived its owner. When that owner closed the database (test cleanup, or app.Shutdown calling CancelAll and then releasing the connection), the title write landed on a dead handle: ERROR Failed to save session title and usage error="sql: database is closed" ERROR Failed to save fallback session title error="sql: database is closed" and the session kept its placeholder title. Track title generation in a WaitGroup on the agent. Run defers a wait on the goroutine it spawned, registered ahead of the cancel and the activeRequests cleanup so it runs last: the session stops reporting busy and the run context is released before Run blocks on the title write. CancelAll drains the same group, since shutdown closes the database right after it returns. Also bound GenerateTitle with a 60s timeout. It runs on a detached context, so without a deadline a stalled request pinned the goroutine for the life of the process and the fallback rename never ran. The fallback re-detaches, so it still fires after the deadline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 9efc08c commit df6bae8

2 files changed

Lines changed: 204 additions & 5 deletions

File tree

internal/agent/agent.go

Lines changed: 86 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,13 @@ const (
5757
largeContextWindowThreshold = 200_000
5858
largeContextWindowBuffer = 20_000
5959
smallContextWindowRatio = 0.2
60+
61+
// titleGenerationTimeout bounds one call to GenerateTitle, covering
62+
// both the small-model attempt and the large-model retry. Titles from
63+
// a reasoning model can take tens of seconds, so the bound is
64+
// generous; it exists to keep a stalled request finite, not to cut
65+
// slow ones short.
66+
titleGenerationTimeout = 60 * time.Second
6067
)
6168

6269
var userAgent = fmt.Sprintf("Charm-Crush/%s (https://charm.land/crush)", version.Version)
@@ -220,6 +227,12 @@ type sessionAgent struct {
220227
// across the agent. Cancel uses its current value as the per-session
221228
// high-water mark.
222229
acceptSeqGen uint64
230+
// titleWG tracks in-flight title generation. Those goroutines run on
231+
// a context detached from the run (see startTitleGeneration), so a
232+
// cancel cannot stop them and nothing else knows they exist. The
233+
// group is what lets Run and CancelAll join them before whoever owns
234+
// the database closes it.
235+
titleWG sync.WaitGroup
223236
}
224237

225238
type SessionAgentOptions struct {
@@ -649,6 +662,17 @@ func (a *sessionAgent) Run(ctx context.Context, call SessionAgentCall) (result *
649662
}
650663
sessMu.Unlock()
651664

665+
// waitForTitle is set below when this run spawns title generation. It
666+
// is deferred here, ahead of the cancel and the activeRequests
667+
// cleanup, so it runs last: the session stops reporting busy and the
668+
// run context is released before Run blocks on the title write.
669+
var waitForTitle func()
670+
defer func() {
671+
if waitForTitle != nil {
672+
waitForTitle()
673+
}
674+
}()
675+
652676
defer cancel()
653677
// Conditional cleanup: only remove our entry if it hasn't been replaced
654678
// by a newer run. Without this guard, the deferred Del fires after a
@@ -701,12 +725,8 @@ func (a *sessionAgent) Run(ctx context.Context, call SessionAgentCall) (result *
701725
}
702726

703727
// Generate title from the first real (non-shell) user prompt.
704-
// can take tens of seconds. Blocking Run on it delays the
705-
// response to the caller. Use a detached context so the title
706-
// goroutine survives Run's cancel.
707728
if !hasUserTextMessage(msgs) {
708-
titleCtx := context.WithoutCancel(ctx)
709-
go a.GenerateTitle(titleCtx, call.SessionID, call.Prompt)
729+
waitForTitle = a.startTitleGeneration(ctx, call.SessionID, call.Prompt)
710730
}
711731

712732
// Add the user message to the session.
@@ -1724,12 +1744,61 @@ func hasUserTextMessage(msgs []message.Message) bool {
17241744
return false
17251745
}
17261746

1747+
// startTitleGeneration generates the session title on its own goroutine and
1748+
// returns a function that blocks until that goroutine is done.
1749+
//
1750+
// The title call can take tens of seconds on a reasoning model, so it must not
1751+
// sit in front of the model's response, and it runs on a context detached from
1752+
// the run so a cancel (or a workspace shutdown) can't drop the title write.
1753+
// That detachment is exactly why the caller has to join it: nothing can stop
1754+
// this goroutine, and if it outlives the database handle its writes fail with
1755+
// "sql: database is closed" and the session keeps its placeholder title. Run
1756+
// defers the returned wait and CancelAll drains the same group, so a title
1757+
// write is never left in flight past the lifetime of whoever owns the
1758+
// database. GenerateTitle bounds itself with titleGenerationTimeout, so both
1759+
// waits terminate.
1760+
func (a *sessionAgent) startTitleGeneration(ctx context.Context, sessionID, userPrompt string) func() {
1761+
titleCtx := context.WithoutCancel(ctx)
1762+
done := make(chan struct{})
1763+
a.titleWG.Add(1)
1764+
go func() {
1765+
// Done first, so a caller released by done also observes the
1766+
// group as drained.
1767+
defer close(done)
1768+
defer a.titleWG.Done()
1769+
a.GenerateTitle(titleCtx, sessionID, userPrompt)
1770+
}()
1771+
return func() { <-done }
1772+
}
1773+
1774+
// waitForTitles blocks until every in-flight title generation has finished or
1775+
// timeout elapses, whichever comes first.
1776+
func (a *sessionAgent) waitForTitles(timeout time.Duration) {
1777+
done := make(chan struct{})
1778+
go func() {
1779+
a.titleWG.Wait()
1780+
close(done)
1781+
}()
1782+
select {
1783+
case <-done:
1784+
case <-time.After(timeout):
1785+
}
1786+
}
1787+
17271788
// GenerateTitle generates a session title based on the initial prompt.
17281789
func (a *sessionAgent) GenerateTitle(ctx context.Context, sessionID string, userPrompt string) {
17291790
if userPrompt == "" {
17301791
return
17311792
}
17321793

1794+
// Bound the attempt. Callers hand this a context detached from the
1795+
// run, so without a deadline a stalled request would pin the
1796+
// goroutine, and anything waiting on it, for the life of the
1797+
// process. The fallback below re-detaches, so it still runs after
1798+
// this deadline fires.
1799+
ctx, cancelTitle := context.WithTimeout(ctx, titleGenerationTimeout)
1800+
defer cancelTitle()
1801+
17331802
// Ensure the session always gets a title even if every path below
17341803
// fails or the context is cancelled before we finish.
17351804
var titleSaved bool
@@ -2016,6 +2085,18 @@ func (a *sessionAgent) ClearQueue(sessionID string) {
20162085
}
20172086

20182087
func (a *sessionAgent) CancelAll() {
2088+
a.cancelActiveRuns()
2089+
2090+
// Title generation is detached from the run context, so the cancels
2091+
// above don't reach it and IsBusy never reported it. Drain it here:
2092+
// callers use CancelAll to make the agent quiescent before closing
2093+
// the database (see app.Shutdown).
2094+
a.waitForTitles(5 * time.Second)
2095+
}
2096+
2097+
// cancelActiveRuns cancels every active run and waits, within a bounded
2098+
// budget, for them to unwind.
2099+
func (a *sessionAgent) cancelActiveRuns() {
20192100
if !a.IsBusy() {
20202101
return
20212102
}

internal/agent/title_test.go

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
package agent
2+
3+
import (
4+
"context"
5+
"errors"
6+
"testing"
7+
"time"
8+
9+
"charm.land/fantasy"
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
// slowTitleModel is a small model whose Stream takes long enough to still be
14+
// in flight when a fast large model has already finished the turn.
15+
type slowTitleModel struct {
16+
delay time.Duration
17+
title string
18+
// entered, when non-nil, receives once the stream has started, so a
19+
// test can act at a point where title generation is provably in
20+
// flight.
21+
entered chan struct{}
22+
}
23+
24+
func (slowTitleModel) Provider() string { return "fake" }
25+
func (slowTitleModel) Model() string { return "fake-small" }
26+
27+
func (slowTitleModel) Generate(context.Context, fantasy.Call) (*fantasy.Response, error) {
28+
return nil, errors.New("not implemented")
29+
}
30+
31+
func (m slowTitleModel) Stream(context.Context, fantasy.Call) (fantasy.StreamResponse, error) {
32+
return func(yield func(fantasy.StreamPart) bool) {
33+
if m.entered != nil {
34+
select {
35+
case m.entered <- struct{}{}:
36+
default:
37+
}
38+
}
39+
time.Sleep(m.delay)
40+
yield(fantasy.StreamPart{Type: fantasy.StreamPartTypeTextStart, ID: "1"})
41+
yield(fantasy.StreamPart{Type: fantasy.StreamPartTypeTextDelta, ID: "1", Delta: m.title})
42+
yield(fantasy.StreamPart{Type: fantasy.StreamPartTypeTextEnd, ID: "1"})
43+
yield(fantasy.StreamPart{Type: fantasy.StreamPartTypeFinish, FinishReason: fantasy.FinishReasonStop})
44+
}, nil
45+
}
46+
47+
func (slowTitleModel) GenerateObject(context.Context, fantasy.ObjectCall) (*fantasy.ObjectResponse, error) {
48+
return nil, errors.New("not implemented")
49+
}
50+
51+
func (slowTitleModel) StreamObject(context.Context, fantasy.ObjectCall) (fantasy.ObjectStreamResponse, error) {
52+
return nil, errors.New("not implemented")
53+
}
54+
55+
// TestRun_WaitsForTitleGeneration pins the lifetime of the title goroutine to
56+
// the run that spawned it. Title generation is detached from the run context
57+
// so a cancel can't drop the write; that also means nothing else can stop it,
58+
// and when it was left unjoined it kept running after its owner tore down the
59+
// database, writing into a closed connection ("sql: database is closed") and
60+
// leaving the session with a placeholder title. Run must not return until the
61+
// title has landed.
62+
func TestRun_WaitsForTitleGeneration(t *testing.T) {
63+
t.Parallel()
64+
env := testEnv(t)
65+
small := slowTitleModel{delay: 250 * time.Millisecond, title: "a good title"}
66+
sa := testSessionAgent(env, fastModel{}, small, "system")
67+
68+
sess, err := env.sessions.Create(t.Context(), "New Session")
69+
require.NoError(t, err)
70+
71+
_, err = sa.Run(t.Context(), SessionAgentCall{
72+
SessionID: sess.ID,
73+
Prompt: "hello",
74+
})
75+
require.NoError(t, err)
76+
77+
got, err := env.sessions.Get(t.Context(), sess.ID)
78+
require.NoError(t, err)
79+
require.Equal(t, "a good title", got.Title)
80+
}
81+
82+
// TestCancelAll_WaitsForTitleGeneration covers the shutdown path. app.Shutdown
83+
// calls CancelAll to make the agent quiescent and then closes the database, so
84+
// CancelAll has to drain title generation too — the run cancel it issues never
85+
// reaches those goroutines.
86+
func TestCancelAll_WaitsForTitleGeneration(t *testing.T) {
87+
t.Parallel()
88+
env := testEnv(t)
89+
small := slowTitleModel{
90+
delay: 250 * time.Millisecond,
91+
title: "shutdown title",
92+
entered: make(chan struct{}, 1),
93+
}
94+
sa := testSessionAgent(env, fastModel{}, small, "system").(*sessionAgent)
95+
96+
sess, err := env.sessions.Create(t.Context(), "New Session")
97+
require.NoError(t, err)
98+
99+
go func() {
100+
_, _ = sa.Run(t.Context(), SessionAgentCall{
101+
SessionID: sess.ID,
102+
Prompt: "hello",
103+
})
104+
}()
105+
106+
// Only cancel once title generation is provably in flight.
107+
select {
108+
case <-small.entered:
109+
case <-time.After(5 * time.Second):
110+
t.Fatal("title generation never started")
111+
}
112+
113+
sa.CancelAll()
114+
115+
got, err := env.sessions.Get(t.Context(), sess.ID)
116+
require.NoError(t, err)
117+
require.Equal(t, "shutdown title", got.Title)
118+
}

0 commit comments

Comments
 (0)