mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
test(coderd/chatd): add P0 coverage tests for subagent auth and panic recovery (#23309)
The processChat defer at line 2464 catches panics on its main goroutine and transitions the chat to error status. This was previously untested. The test wraps the database Store to panic during PersistStep's InTx call, which runs synchronously on the processChat goroutine. A tool-level panic wouldn't work because executeTools has its own recover that converts panics into tool error results.
This commit is contained in:
Mathias Fredriksson
@@ -3372,3 +3372,83 @@ func TestInterruptChatPersistsPartialResponse(t *testing.T) {
|
||||
require.Contains(t, foundText, "hello world",
|
||||
"partial assistant response should contain the streamed text")
|
||||
}
|
||||
|
||||
func TestProcessChatPanicRecovery(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
|
||||
// Wrap the database so we can trigger a panic on the main
|
||||
// goroutine of processChat. The chatloop's executeTools has
|
||||
// its own recover, so panicking inside a tool goroutine won't
|
||||
// reach the processChat-level recovery. Instead, we panic
|
||||
// during PersistStep's InTx call, which runs synchronously on
|
||||
// the processChat goroutine.
|
||||
panicWrapper := &panicOnInTxDB{Store: db}
|
||||
|
||||
openAIURL := chattest.NewOpenAI(t, func(req *chattest.OpenAIRequest) chattest.OpenAIResponse {
|
||||
if !req.Stream {
|
||||
return chattest.OpenAINonStreamingResponse("Panic recovery test")
|
||||
}
|
||||
return chattest.OpenAIStreamingResponse(
|
||||
chattest.OpenAITextChunks("hello")...,
|
||||
)
|
||||
})
|
||||
|
||||
ctx := testutil.Context(t, testutil.WaitLong)
|
||||
user, model := seedChatDependenciesWithProvider(ctx, t, db, "openai-compat", openAIURL)
|
||||
|
||||
// Pass the panic wrapper to the server, but use the real
|
||||
// database for seeding so those operations don't panic.
|
||||
server := newActiveTestServer(t, panicWrapper, ps)
|
||||
|
||||
chat, err := server.CreateChat(ctx, chatd.CreateOptions{
|
||||
OwnerID: user.ID,
|
||||
Title: "panic-recovery",
|
||||
ModelConfigID: model.ID,
|
||||
InitialUserContent: []codersdk.ChatMessagePart{
|
||||
codersdk.ChatMessageText("hello"),
|
||||
},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Enable the panic now that CreateChat's InTx has completed.
|
||||
// The next InTx call is PersistStep inside the chatloop,
|
||||
// running synchronously on the processChat goroutine.
|
||||
panicWrapper.enablePanic()
|
||||
|
||||
// Wait for the panic to be recovered and the chat to
|
||||
// transition to error status.
|
||||
var chatResult database.Chat
|
||||
require.Eventually(t, func() bool {
|
||||
got, getErr := db.GetChatByID(ctx, chat.ID)
|
||||
if getErr != nil {
|
||||
return false
|
||||
}
|
||||
chatResult = got
|
||||
return got.Status == database.ChatStatusError
|
||||
}, testutil.WaitLong, testutil.IntervalFast)
|
||||
|
||||
require.True(t, chatResult.LastError.Valid, "LastError should be set")
|
||||
require.Contains(t, chatResult.LastError.String, "chat processing panicked")
|
||||
require.Contains(t, chatResult.LastError.String, "intentional test panic")
|
||||
}
|
||||
|
||||
// panicOnInTxDB wraps a database.Store and panics on the first InTx
|
||||
// call after enablePanic is called. Subsequent calls pass through
|
||||
// so the processChat cleanup defer can update the chat status.
|
||||
type panicOnInTxDB struct {
|
||||
database.Store
|
||||
active atomic.Bool
|
||||
panicked atomic.Bool
|
||||
}
|
||||
|
||||
func (d *panicOnInTxDB) enablePanic() { d.active.Store(true) }
|
||||
|
||||
func (d *panicOnInTxDB) InTx(f func(database.Store) error, opts *database.TxOptions) error {
|
||||
if d.active.Load() && !d.panicked.Load() {
|
||||
d.panicked.Store(true)
|
||||
panic("intentional test panic")
|
||||
}
|
||||
return d.Store.InTx(f, opts)
|
||||
}
|
||||
|
||||
@@ -332,3 +332,139 @@ func TestSpawnComputerUseAgent_UsesComputerUseModelNotParent(t *testing.T) {
|
||||
assert.Equal(t, "anthropic", chattool.ComputerUseModelProvider)
|
||||
assert.NotEmpty(t, chattool.ComputerUseModelName)
|
||||
}
|
||||
|
||||
func TestIsSubagentDescendant(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
db, ps := dbtestutil.NewDB(t)
|
||||
server := newInternalTestServer(t, db, ps, chatprovider.ProviderAPIKeys{})
|
||||
|
||||
ctx := chatdTestContext(t)
|
||||
user, model := seedInternalChatDeps(ctx, t, db)
|
||||
|
||||
// Build a chain: root -> child -> grandchild.
|
||||
root, err := server.CreateChat(ctx, CreateOptions{
|
||||
OwnerID: user.ID,
|
||||
Title: "root",
|
||||
ModelConfigID: model.ID,
|
||||
InitialUserContent: []codersdk.ChatMessagePart{codersdk.ChatMessageText("root")},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
child, err := server.CreateChat(ctx, CreateOptions{
|
||||
OwnerID: user.ID,
|
||||
ParentChatID: uuid.NullUUID{
|
||||
UUID: root.ID,
|
||||
Valid: true,
|
||||
},
|
||||
RootChatID: uuid.NullUUID{
|
||||
UUID: root.ID,
|
||||
Valid: true,
|
||||
},
|
||||
Title: "child",
|
||||
ModelConfigID: model.ID,
|
||||
InitialUserContent: []codersdk.ChatMessagePart{codersdk.ChatMessageText("child")},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
grandchild, err := server.CreateChat(ctx, CreateOptions{
|
||||
OwnerID: user.ID,
|
||||
ParentChatID: uuid.NullUUID{
|
||||
UUID: child.ID,
|
||||
Valid: true,
|
||||
},
|
||||
RootChatID: uuid.NullUUID{
|
||||
UUID: root.ID,
|
||||
Valid: true,
|
||||
},
|
||||
Title: "grandchild",
|
||||
ModelConfigID: model.ID,
|
||||
InitialUserContent: []codersdk.ChatMessagePart{codersdk.ChatMessageText("grandchild")},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Build a separate, unrelated chain.
|
||||
unrelated, err := server.CreateChat(ctx, CreateOptions{
|
||||
OwnerID: user.ID,
|
||||
Title: "unrelated-root",
|
||||
ModelConfigID: model.ID,
|
||||
InitialUserContent: []codersdk.ChatMessagePart{codersdk.ChatMessageText("unrelated")},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
unrelatedChild, err := server.CreateChat(ctx, CreateOptions{
|
||||
OwnerID: user.ID,
|
||||
ParentChatID: uuid.NullUUID{
|
||||
UUID: unrelated.ID,
|
||||
Valid: true,
|
||||
},
|
||||
RootChatID: uuid.NullUUID{
|
||||
UUID: unrelated.ID,
|
||||
Valid: true,
|
||||
},
|
||||
Title: "unrelated-child",
|
||||
ModelConfigID: model.ID,
|
||||
InitialUserContent: []codersdk.ChatMessagePart{codersdk.ChatMessageText("unrelated-child")},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
ancestor uuid.UUID
|
||||
target uuid.UUID
|
||||
want bool
|
||||
}{
|
||||
{
|
||||
name: "SameID",
|
||||
ancestor: root.ID,
|
||||
target: root.ID,
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "DirectChild",
|
||||
ancestor: root.ID,
|
||||
target: child.ID,
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "GrandChild",
|
||||
ancestor: root.ID,
|
||||
target: grandchild.ID,
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "Unrelated",
|
||||
ancestor: root.ID,
|
||||
target: unrelatedChild.ID,
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "RootChat",
|
||||
ancestor: child.ID,
|
||||
target: root.ID,
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "BrokenChain",
|
||||
ancestor: root.ID,
|
||||
target: uuid.New(),
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "NotDescendant",
|
||||
ancestor: unrelated.ID,
|
||||
target: child.ID,
|
||||
want: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := chatdTestContext(t)
|
||||
got, err := isSubagentDescendant(ctx, db, tt.ancestor, tt.target)
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, tt.want, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user