From 7d35ee53c75f1ea2c9f764d60b45daa0142af260 Mon Sep 17 00:00:00 2001 From: viettranx Date: Tue, 31 Mar 2026 11:44:54 +0700 Subject: [PATCH] fix(agent): smart delegation prompt + compaction pending state + block team tools in subagents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Leader prompt: replace blanket "prefer delegation" with conditional — delegate complex work, handle simple requests directly - Compaction prompt: explicitly preserve pending subagent/team task state and "waiting for" expectations across summarization - Add team_tasks to SubagentDenyAlways — subagents must not use team orchestration tools Closes partially #600 --- internal/agent/loop_compact.go | 3 + internal/agent/resolver_helpers.go | 2 +- internal/tools/subagent.go | 217 ++--------------------------- internal/tools/subagent_roster.go | 62 +++++++++ 4 files changed, 81 insertions(+), 203 deletions(-) create mode 100644 internal/tools/subagent_roster.go diff --git a/internal/agent/loop_compact.go b/internal/agent/loop_compact.go index 489ceba7..1f8a136e 100644 --- a/internal/agent/loop_compact.go +++ b/internal/agent/loop_compact.go @@ -17,6 +17,9 @@ const compactionSummaryPrompt = `Summarize this conversation concisely for the A MUST PRESERVE: - Active tasks and their current status (in-progress, blocked, pending) +- Pending subagent tasks (IDs, labels, statuses) — agent needs to know what is still running +- Pending team task results awaiting delivery (task IDs, assignees, statuses) +- Any "waiting for..." state — do NOT drop expectations of future results - Batch operation progress (e.g., "5/17 items completed") - The last thing the user requested and what was being done about it - Decisions made and their rationale diff --git a/internal/agent/resolver_helpers.go b/internal/agent/resolver_helpers.go index b689e2a7..22b68b4b 100644 --- a/internal/agent/resolver_helpers.go +++ b/internal/agent/resolver_helpers.go @@ -86,7 +86,7 @@ func buildTeamMD(team *store.TeamData, members []store.TeamMemberData, selfID uu sb.WriteString("- **Create ALL tasks upfront** in one batch, then announce — then STOP. Do NOT create one task, wait for it to finish, then create the next\n") sb.WriteString("- Delegation is NOT completion — do NOT say \"done\"/\"xong\"/\"finished\" after delegating. Only report completion when ALL task results have been delivered\n") sb.WriteString("- Results arrive automatically — do NOT present partial results\n") - sb.WriteString("- **Prefer delegation** — delegate tasks to members, do NOT do the work yourself. If you need research, create a research task. If you need code, create a coding task. Only use tools like web_search yourself for quick clarifications needed to plan tasks (e.g., checking a member's capability), NOT for producing deliverables\n") + sb.WriteString("- **Delegate complex work** — tasks requiring member expertise, multiple steps, or parallel execution should use team_tasks. Handle simple requests yourself (greetings, clarifications, lookups, translations, single-tool calls). If you can answer in one response with a single tool call, do it yourself. If it requires sustained work, multiple skills, or member expertise → delegate\n") sb.WriteString("- **Do NOT block on completed tasks** — pass completed task's result in the description instead of using blocked_by\n") sb.WriteString("\n## Task Planning\n\n") diff --git a/internal/tools/subagent.go b/internal/tools/subagent.go index 3e2ea0dd..52259e83 100644 --- a/internal/tools/subagent.go +++ b/internal/tools/subagent.go @@ -10,20 +10,14 @@ package tools import ( - "cmp" "context" - "fmt" - "log/slog" - "slices" "sync" - "time" "github.com/google/uuid" "github.com/nextlevelbuilder/goclaw/internal/bus" "github.com/nextlevelbuilder/goclaw/internal/providers" "github.com/nextlevelbuilder/goclaw/internal/store" - "github.com/nextlevelbuilder/goclaw/internal/tracing" ) // SubagentConfig configures the subagent system. @@ -32,6 +26,7 @@ type SubagentConfig struct { MaxSpawnDepth int // max nesting depth (default 3) MaxChildrenPerAgent int // max children per parent (default 8) ArchiveAfterMinutes int // auto-archive completed tasks (default 30) + MaxRetries int // max LLM call retries on error (default 2) Model string // model override for subagents (empty = inherit) } @@ -53,6 +48,8 @@ type SubagentTask struct { Result string `json:"result,omitempty"` Depth int `json:"depth"` Model string `json:"model,omitempty"` // model override for this subagent + TotalInputTokens int64 `json:"totalInputTokens,omitempty"` + TotalOutputTokens int64 `json:"totalOutputTokens,omitempty"` OriginChannel string `json:"originChannel,omitempty"` OriginChatID string `json:"originChatId,omitempty"` OriginPeerKind string `json:"originPeerKind,omitempty"` // "direct" or "group" (for session key building) @@ -67,6 +64,7 @@ type SubagentTask struct { OriginRootSpanID uuid.UUID `json:"-"` // parent agent's root span ID cancelFunc context.CancelFunc `json:"-"` // per-task context cancel spawnConfig SubagentConfig `json:"-"` // resolved config at spawn time (per-agent override merged) + dbID uuid.UUID `json:"-"` // persistent DB UUID (zero if not persisted) } // SubagentManager manages the lifecycle of spawned subagents. @@ -81,7 +79,8 @@ type SubagentManager struct { // createTools builds a tool registry for subagents (without spawn/subagent tools). createTools func() *Registry - announceQueue *AnnounceQueue // optional: batches announces with debounce + announceQueue *AnnounceQueue // optional: batches announces with debounce + taskStore store.SubagentTaskStore // optional: persists tasks to DB (fire-and-forget) } // NewSubagentManager creates a new subagent manager. @@ -110,6 +109,11 @@ func (sm *SubagentManager) SetAnnounceQueue(q *AnnounceQueue) { sm.announceQueue = q } +// SetTaskStore sets the persistent store for subagent tasks (write-through, fire-and-forget). +func (sm *SubagentManager) SetTaskStore(s store.SubagentTaskStore) { + sm.taskStore = s +} + // effectiveConfig returns the per-agent context override merged with defaults, // or falls back to sm.config when no override is present. func (sm *SubagentManager) effectiveConfig(ctx context.Context) SubagentConfig { @@ -130,68 +134,15 @@ func (sm *SubagentManager) effectiveConfig(ctx context.Context) SubagentConfig { if override.ArchiveAfterMinutes > 0 { cfg.ArchiveAfterMinutes = override.ArchiveAfterMinutes } + if override.MaxRetries > 0 { + cfg.MaxRetries = override.MaxRetries + } if override.Model != "" { cfg.Model = override.Model } return cfg } -// SubagentRosterEntry summarizes one subagent task for the roster. -type SubagentRosterEntry struct { - Label string - Status string // "running", "completed", "failed", "cancelled" -} - -// SubagentRoster holds the full roster of subagent tasks for a parent, -// including per-agent config limits for deterministic LLM context. -type SubagentRoster struct { - Entries []SubagentRosterEntry - Total int // total tasks for this parent - MaxPerAgent int // from spawnConfig.MaxChildrenPerAgent -} - -// RosterForParent returns the full roster of tasks for a parent. -// Sorted: completed/failed/cancelled first, then running (deterministic output). -func (sm *SubagentManager) RosterForParent(parentID string) SubagentRoster { - sm.mu.RLock() - defer sm.mu.RUnlock() - - var entries []SubagentRosterEntry - maxPerAgent := 0 - for _, t := range sm.tasks { - if t.ParentID != parentID { - continue - } - entries = append(entries, SubagentRosterEntry{ - Label: t.Label, - Status: t.Status, - }) - if maxPerAgent == 0 { - maxPerAgent = t.spawnConfig.MaxChildrenPerAgent - } - } - - // Sort: non-running first (completed/failed/cancelled), then running. - // Within same group, sort alphabetically by label for determinism. - slices.SortFunc(entries, func(a, b SubagentRosterEntry) int { - aRunning := a.Status == TaskStatusRunning - bRunning := b.Status == TaskStatusRunning - if aRunning != bRunning { - if aRunning { - return 1 // running goes last - } - return -1 - } - return cmp.Compare(a.Label, b.Label) - }) - - return SubagentRoster{ - Entries: entries, - Total: len(entries), - MaxPerAgent: maxPerAgent, - } -} - // SubagentDenyAlways is the list of tools always denied to subagents. var SubagentDenyAlways = []string{ "gateway", @@ -202,6 +153,7 @@ var SubagentDenyAlways = []string{ "memory_search", "memory_get", "sessions_send", + "team_tasks", // subagents must not use team orchestration } // SubagentDenyLeaf is the additional deny list for subagents at max depth. @@ -211,142 +163,3 @@ var SubagentDenyLeaf = []string{ "sessions_spawn", "spawn", } - -// Spawn creates a new subagent task that runs asynchronously. -// Returns immediately with a status message. The subagent runs in a goroutine. -// modelOverride optionally overrides the LLM model for this subagent (matching TS sessions-spawn-tool.ts). -func (sm *SubagentManager) Spawn( - ctx context.Context, - parentID string, - depth int, - task, label, modelOverride string, - channel, chatID, peerKind string, - callback AsyncCallback, -) (string, error) { - cfg := sm.effectiveConfig(ctx) - sm.mu.Lock() - - // Check depth limit - if depth >= cfg.MaxSpawnDepth { - sm.mu.Unlock() - return "", fmt.Errorf("spawn depth limit reached (%d/%d)", depth, cfg.MaxSpawnDepth) - } - - // Check concurrent limit - running := 0 - for _, t := range sm.tasks { - if t.Status == TaskStatusRunning { - running++ - } - } - if running >= cfg.MaxConcurrent { - sm.mu.Unlock() - return "", fmt.Errorf("max concurrent subagents reached (%d/%d)", running, cfg.MaxConcurrent) - } - - // Check per-parent children limit - childCount := 0 - for _, t := range sm.tasks { - if t.ParentID == parentID { - childCount++ - } - } - if childCount >= cfg.MaxChildrenPerAgent { - sm.mu.Unlock() - return "", fmt.Errorf("max children per agent reached (%d/%d)", childCount, cfg.MaxChildrenPerAgent) - } - - id := generateSubagentID() - if label == "" { - label = truncate(task, 50) - } - - subTask := &SubagentTask{ - ID: id, - ParentID: parentID, - Task: task, - Label: label, - Status: "running", - Depth: depth + 1, - Model: modelOverride, - OriginChannel: channel, - OriginChatID: chatID, - OriginPeerKind: peerKind, - OriginLocalKey: ToolLocalKeyFromCtx(ctx), - OriginUserID: store.UserIDFromContext(ctx), - OriginSessionKey: ToolSessionKeyFromCtx(ctx), - OriginTenantID: store.TenantIDFromContext(ctx), - OriginTraceID: tracing.TraceIDFromContext(ctx), - OriginRootSpanID: tracing.ParentSpanIDFromContext(ctx), - CreatedAt: time.Now().UnixMilli(), - spawnConfig: cfg, - } - // Detach from parent's cancellation chain so subagent survives after parent run completes. - // WithoutCancel preserves all context values (agent ID, workspace, trace info, etc.) - // but parent Done() no longer propagates. Manual cancel via taskCancel() still works. - detached := context.WithoutCancel(ctx) - taskCtx, taskCancel := context.WithCancel(detached) - subTask.cancelFunc = taskCancel - - sm.tasks[id] = subTask - sm.mu.Unlock() - - slog.Info("subagent spawned", "id", id, "parent", parentID, "depth", subTask.Depth, "label", label) - - go sm.runTask(taskCtx, subTask, callback) - - return fmt.Sprintf("Spawned subagent '%s' (id=%s, depth=%d) for task: %s", - label, id, subTask.Depth, truncate(task, 100)), nil -} - -// RunSync executes a subagent task synchronously, blocking until completion. -func (sm *SubagentManager) RunSync( - ctx context.Context, - parentID string, - depth int, - task, label string, - channel, chatID string, -) (string, int, error) { - cfg := sm.effectiveConfig(ctx) - - sm.mu.Lock() - - if depth >= cfg.MaxSpawnDepth { - sm.mu.Unlock() - return "", 0, fmt.Errorf("spawn depth limit reached (%d/%d)", depth, cfg.MaxSpawnDepth) - } - - id := generateSubagentID() - if label == "" { - label = truncate(task, 50) - } - - subTask := &SubagentTask{ - ID: id, - ParentID: parentID, - Task: task, - Label: label, - Status: "running", - Depth: depth + 1, - OriginChannel: channel, - OriginChatID: chatID, - OriginLocalKey: ToolLocalKeyFromCtx(ctx), - OriginUserID: store.UserIDFromContext(ctx), - OriginTraceID: tracing.TraceIDFromContext(ctx), - OriginRootSpanID: tracing.ParentSpanIDFromContext(ctx), - CreatedAt: time.Now().UnixMilli(), - spawnConfig: cfg, - } - sm.tasks[id] = subTask - sm.mu.Unlock() - - slog.Info("subagent sync started", "id", id, "parent", parentID, "depth", subTask.Depth, "label", label) - - iterations := sm.executeTask(ctx, subTask) - - if subTask.Status == TaskStatusFailed { - return subTask.Result, iterations, fmt.Errorf("subagent failed: %s", subTask.Result) - } - - return subTask.Result, iterations, nil -} diff --git a/internal/tools/subagent_roster.go b/internal/tools/subagent_roster.go new file mode 100644 index 00000000..e73d839e --- /dev/null +++ b/internal/tools/subagent_roster.go @@ -0,0 +1,62 @@ +package tools + +import ( + "cmp" + "slices" +) + +// SubagentRosterEntry summarizes one subagent task for the roster. +type SubagentRosterEntry struct { + Label string + Status string // "running", "completed", "failed", "cancelled" +} + +// SubagentRoster holds the full roster of subagent tasks for a parent, +// including per-agent config limits for deterministic LLM context. +type SubagentRoster struct { + Entries []SubagentRosterEntry + Total int // total tasks for this parent + MaxPerAgent int // from spawnConfig.MaxChildrenPerAgent +} + +// RosterForParent returns the full roster of tasks for a parent. +// Sorted: completed/failed/cancelled first, then running (deterministic output). +func (sm *SubagentManager) RosterForParent(parentID string) SubagentRoster { + sm.mu.RLock() + defer sm.mu.RUnlock() + + var entries []SubagentRosterEntry + maxPerAgent := 0 + for _, t := range sm.tasks { + if t.ParentID != parentID { + continue + } + entries = append(entries, SubagentRosterEntry{ + Label: t.Label, + Status: t.Status, + }) + if maxPerAgent == 0 { + maxPerAgent = t.spawnConfig.MaxChildrenPerAgent + } + } + + // Sort: non-running first (completed/failed/cancelled), then running. + // Within same group, sort alphabetically by label for determinism. + slices.SortFunc(entries, func(a, b SubagentRosterEntry) int { + aRunning := a.Status == TaskStatusRunning + bRunning := b.Status == TaskStatusRunning + if aRunning != bRunning { + if aRunning { + return 1 // running goes last + } + return -1 + } + return cmp.Compare(a.Label, b.Label) + }) + + return SubagentRoster{ + Entries: entries, + Total: len(entries), + MaxPerAgent: maxPerAgent, + } +}