fix(subturn): dismiss child tool feedback after sync completion
This commit is contained in:
parent
1c25dcd239
commit
fee085985f
6 changed files with 164 additions and 1 deletions
|
|
@ -49,3 +49,12 @@ func (a *channelManagerAdapter) DismissToolFeedback(
|
||||||
) {
|
) {
|
||||||
a.inner.DismissToolFeedback(ctx, channel, chatID, outboundCtx)
|
a.inner.DismissToolFeedback(ctx, channel, chatID, outboundCtx)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (a *channelManagerAdapter) DismissToolFeedbackForSession(
|
||||||
|
ctx context.Context,
|
||||||
|
channel, chatID string,
|
||||||
|
outboundCtx *bus.InboundContext,
|
||||||
|
sessionKey string,
|
||||||
|
) {
|
||||||
|
a.inner.DismissToolFeedbackForSession(ctx, channel, chatID, outboundCtx, sessionKey)
|
||||||
|
}
|
||||||
|
|
|
||||||
|
|
@ -51,4 +51,15 @@ type ChannelManager interface {
|
||||||
// outboundCtx carries topic/thread info needed for channels that use
|
// outboundCtx carries topic/thread info needed for channels that use
|
||||||
// scoped tracker keys (e.g., Telegram forum topics); may be nil.
|
// scoped tracker keys (e.g., Telegram forum topics); may be nil.
|
||||||
DismissToolFeedback(ctx context.Context, channel, chatID string, outboundCtx *bus.InboundContext)
|
DismissToolFeedback(ctx context.Context, channel, chatID string, outboundCtx *bus.InboundContext)
|
||||||
|
|
||||||
|
// DismissToolFeedbackForSession clears a session-scoped tool feedback
|
||||||
|
// message. This is used for child sub-turns whose progress messages are
|
||||||
|
// visible in the originating chat, but whose result may be delivered
|
||||||
|
// separately from the child channel lifecycle.
|
||||||
|
DismissToolFeedbackForSession(
|
||||||
|
ctx context.Context,
|
||||||
|
channel, chatID string,
|
||||||
|
outboundCtx *bus.InboundContext,
|
||||||
|
sessionKey string,
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -460,6 +460,23 @@ func spawnSubTurn(
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Child turns publish session-scoped tool feedback in the originating
|
||||||
|
// chat. Dismiss that feedback when the child finishes regardless of
|
||||||
|
// async/sync delivery; synchronous delegate/subagent calls return their
|
||||||
|
// result inline to the parent and should not leave an orphaned animator
|
||||||
|
// behind.
|
||||||
|
if al != nil && al.channelManager != nil && childTS.channel != "" {
|
||||||
|
dismissCtx, dismissCancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||||
|
al.channelManager.DismissToolFeedbackForSession(
|
||||||
|
dismissCtx,
|
||||||
|
childTS.channel,
|
||||||
|
childTS.chatID,
|
||||||
|
childTS.opts.Dispatch.InboundContext,
|
||||||
|
childID,
|
||||||
|
)
|
||||||
|
dismissCancel()
|
||||||
|
}
|
||||||
|
|
||||||
// Result Delivery Strategy (Async vs Sync)
|
// Result Delivery Strategy (Async vs Sync)
|
||||||
if cfg.Async {
|
if cfg.Async {
|
||||||
deliverSubTurnResult(al, parentTS, childID, result)
|
deliverSubTurnResult(al, parentTS, childID, result)
|
||||||
|
|
|
||||||
|
|
@ -12,6 +12,7 @@ import (
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/sipeed/picoclaw/pkg/bus"
|
"github.com/sipeed/picoclaw/pkg/bus"
|
||||||
|
"github.com/sipeed/picoclaw/pkg/channels"
|
||||||
"github.com/sipeed/picoclaw/pkg/config"
|
"github.com/sipeed/picoclaw/pkg/config"
|
||||||
runtimeevents "github.com/sipeed/picoclaw/pkg/events"
|
runtimeevents "github.com/sipeed/picoclaw/pkg/events"
|
||||||
"github.com/sipeed/picoclaw/pkg/providers"
|
"github.com/sipeed/picoclaw/pkg/providers"
|
||||||
|
|
@ -29,6 +30,37 @@ type eventCollector struct {
|
||||||
events []runtimeevents.Event
|
events []runtimeevents.Event
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type recordingChannelManager struct {
|
||||||
|
dismissedSessions []string
|
||||||
|
dismissedChatIDs []string
|
||||||
|
}
|
||||||
|
|
||||||
|
func (m *recordingChannelManager) GetChannel(name string) (channels.Channel, bool) { return nil, false }
|
||||||
|
func (m *recordingChannelManager) GetEnabledChannels() []string { return nil }
|
||||||
|
func (m *recordingChannelManager) InvokeTypingStop(channel, chatID string) {}
|
||||||
|
func (m *recordingChannelManager) SendMessage(ctx context.Context, msg bus.OutboundMessage) error {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
func (m *recordingChannelManager) SendMedia(ctx context.Context, msg bus.OutboundMediaMessage) error {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
func (m *recordingChannelManager) SendPlaceholder(ctx context.Context, channel, chatID string) bool {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
func (m *recordingChannelManager) DismissToolFeedback(
|
||||||
|
ctx context.Context, channel, chatID string, outboundCtx *bus.InboundContext,
|
||||||
|
) {
|
||||||
|
m.dismissedChatIDs = append(m.dismissedChatIDs, fmt.Sprintf("%s:%s", channel, chatID))
|
||||||
|
}
|
||||||
|
func (m *recordingChannelManager) DismissToolFeedbackForSession(
|
||||||
|
ctx context.Context,
|
||||||
|
channel, chatID string,
|
||||||
|
outboundCtx *bus.InboundContext,
|
||||||
|
sessionKey string,
|
||||||
|
) {
|
||||||
|
m.dismissedSessions = append(m.dismissedSessions, fmt.Sprintf("%s:%s:%s", channel, chatID, sessionKey))
|
||||||
|
}
|
||||||
|
|
||||||
func newEventCollector(t *testing.T, al *AgentLoop) (*eventCollector, func()) {
|
func newEventCollector(t *testing.T, al *AgentLoop) (*eventCollector, func()) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
c := &eventCollector{}
|
c := &eventCollector{}
|
||||||
|
|
@ -1539,6 +1571,56 @@ func TestSyncSubTurn_NoChannelDelivery(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestSyncSubTurn_DismissesSessionScopedToolFeedback(t *testing.T) {
|
||||||
|
al, _, _, provider, cleanup := newTestAgentLoop(t)
|
||||||
|
_ = provider
|
||||||
|
defer cleanup()
|
||||||
|
|
||||||
|
cm := &recordingChannelManager{}
|
||||||
|
al.channelManager = cm
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
parentTS := &turnState{
|
||||||
|
ctx: ctx,
|
||||||
|
turnID: "parent-sync-dismiss",
|
||||||
|
depth: 0,
|
||||||
|
session: newEphemeralSession(nil),
|
||||||
|
pendingResults: make(chan *tools.ToolResult, 16),
|
||||||
|
concurrencySem: make(chan struct{}, testMaxConcurrentSubTurns),
|
||||||
|
}
|
||||||
|
parentTS.ctx, parentTS.cancelFunc = context.WithCancel(ctx)
|
||||||
|
parentTS.agent = al.registry.GetDefaultAgent()
|
||||||
|
parentTS.opts = processOptions{
|
||||||
|
Dispatch: DispatchRequest{
|
||||||
|
InboundContext: &bus.InboundContext{
|
||||||
|
Channel: "telegram",
|
||||||
|
ChatID: "-100123",
|
||||||
|
TopicID: "6",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}
|
||||||
|
parentTS.channel = "telegram"
|
||||||
|
parentTS.chatID = "-100123"
|
||||||
|
defer parentTS.Finish(false)
|
||||||
|
|
||||||
|
result, err := spawnSubTurn(ctx, al, parentTS, SubTurnConfig{
|
||||||
|
Model: "gpt-4o-mini",
|
||||||
|
Async: false,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("spawnSubTurn failed: %v", err)
|
||||||
|
}
|
||||||
|
if result == nil {
|
||||||
|
t.Fatal("expected non-nil result")
|
||||||
|
}
|
||||||
|
if len(cm.dismissedSessions) != 1 {
|
||||||
|
t.Fatalf("dismissedSessions = %v, want exactly one dismissal", cm.dismissedSessions)
|
||||||
|
}
|
||||||
|
if got := cm.dismissedSessions[0]; !strings.HasPrefix(got, "telegram:-100123:subturn-") {
|
||||||
|
t.Fatalf("dismissed session key = %q, want telegram:-100123:subturn-*", got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestAsyncSubTurn_ChannelDelivery verifies that asynchronous sub-turns
|
// TestAsyncSubTurn_ChannelDelivery verifies that asynchronous sub-turns
|
||||||
// DO deliver results to the pendingResults channel.
|
// DO deliver results to the pendingResults channel.
|
||||||
func TestAsyncSubTurn_ChannelDelivery(t *testing.T) {
|
func TestAsyncSubTurn_ChannelDelivery(t *testing.T) {
|
||||||
|
|
|
||||||
|
|
@ -206,6 +206,32 @@ func dismissTrackedToolFeedbackMessage(
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func dismissTrackedToolFeedbackMessageForSession(
|
||||||
|
ctx context.Context,
|
||||||
|
ch Channel,
|
||||||
|
chatID string,
|
||||||
|
outboundCtx *bus.InboundContext,
|
||||||
|
sessionKey string,
|
||||||
|
) {
|
||||||
|
sessionKey = strings.TrimSpace(sessionKey)
|
||||||
|
if sessionKey == "" {
|
||||||
|
dismissTrackedToolFeedbackMessage(ctx, ch, chatID, outboundCtx)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
trackedChatID := trackedToolFeedbackMessageChatID(ch, chatID, outboundCtx)
|
||||||
|
if trackedChatID == "" {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
sessionTrackedChatID := trackedChatID + "#session:" + sessionKey
|
||||||
|
if cleaner, ok := ch.(toolFeedbackMessageCleaner); ok {
|
||||||
|
cleaner.DismissToolFeedbackMessage(ctx, sessionTrackedChatID)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if tracker, ok := ch.(toolFeedbackMessageTracker); ok {
|
||||||
|
tracker.ClearToolFeedbackMessage(sessionTrackedChatID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func clearTrackedToolFeedbackMessage(
|
func clearTrackedToolFeedbackMessage(
|
||||||
ch Channel,
|
ch Channel,
|
||||||
chatID string,
|
chatID string,
|
||||||
|
|
@ -235,6 +261,16 @@ func (m *Manager) DismissToolFeedback(
|
||||||
dismissTrackedToolFeedbackMessage(ctx, ch, chatID, outboundCtx)
|
dismissTrackedToolFeedbackMessage(ctx, ch, chatID, outboundCtx)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (m *Manager) DismissToolFeedbackForSession(
|
||||||
|
ctx context.Context, channelName, chatID string, outboundCtx *bus.InboundContext, sessionKey string,
|
||||||
|
) {
|
||||||
|
ch, ok := m.GetChannel(channelName)
|
||||||
|
if !ok {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
dismissTrackedToolFeedbackMessageForSession(ctx, ch, chatID, outboundCtx, sessionKey)
|
||||||
|
}
|
||||||
|
|
||||||
func prepareToolFeedbackMessageContent(ch Channel, content string) string {
|
func prepareToolFeedbackMessageContent(ch Channel, content string) string {
|
||||||
prepared := strings.TrimSpace(content)
|
prepared := strings.TrimSpace(content)
|
||||||
if prepared == "" {
|
if prepared == "" {
|
||||||
|
|
|
||||||
|
|
@ -529,7 +529,7 @@ func (c *TelegramChannel) dismissTrackedToolFeedbackMessage(ctx context.Context,
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
c.ClearToolFeedbackMessage(chatID)
|
c.ClearToolFeedbackMessage(chatID)
|
||||||
_ = c.DeleteMessage(ctx, chatID, messageID)
|
_ = c.DeleteMessage(ctx, telegramToolFeedbackDeliveryChatID(chatID), messageID)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (c *TelegramChannel) finalizeTrackedToolFeedbackMessage(
|
func (c *TelegramChannel) finalizeTrackedToolFeedbackMessage(
|
||||||
|
|
@ -1130,6 +1130,14 @@ func telegramToolFeedbackChatKey(chatID string, outboundCtx *bus.InboundContext)
|
||||||
return fmt.Sprintf("%d/%d", resolvedChatID, threadID)
|
return fmt.Sprintf("%d/%d", resolvedChatID, threadID)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func telegramToolFeedbackDeliveryChatID(chatID string) string {
|
||||||
|
chatID = strings.TrimSpace(chatID)
|
||||||
|
if idx := strings.Index(chatID, "#session:"); idx >= 0 {
|
||||||
|
return strings.TrimSpace(chatID[:idx])
|
||||||
|
}
|
||||||
|
return chatID
|
||||||
|
}
|
||||||
|
|
||||||
func (c *TelegramChannel) ToolFeedbackMessageChatID(chatID string, outboundCtx *bus.InboundContext) string {
|
func (c *TelegramChannel) ToolFeedbackMessageChatID(chatID string, outboundCtx *bus.InboundContext) string {
|
||||||
return telegramToolFeedbackChatKey(chatID, outboundCtx)
|
return telegramToolFeedbackChatKey(chatID, outboundCtx)
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue