Merge pull request #2289 from dgageot/board/docker-agent-issue-2267-status-inquiry-37a00c98

fix: prevent recursive run_skill loop in context:fork skill sub-sessions

Author: dgageot

pkg/runtime/agent_delegation.go

@@ -85,6 +85,10 @@ type SubSessionConfig struct {
 	// user message sent to the child session. This allows callers like skill
 	// sub-agents to pass the task description as the user message.
 	ImplicitUserMessage string
+	// ExcludedTools lists tool names that should be filtered out of the agent's
+	// tool list for the child session. This prevents recursive tool calls
+	// (e.g. run_skill calling itself in a skill sub-session).
+	ExcludedTools []string
 }
 
 // newSubSession builds a *session.Session from a SubSessionConfig and a parent
@@ -115,9 +119,39 @@ func newSubSession(parent *session.Session, cfg SubSessionConfig, childAgent *ag
 	if cfg.PinAgent {
 		opts = append(opts, session.WithAgentName(cfg.AgentName))
 	}
+	// Merge parent's excluded tools with config's excluded tools so that
+	// nested sub-sessions (e.g. skill → transfer_task → child) inherit
+	// exclusions from all ancestors and don't re-introduce filtered tools.
+	excludedTools := mergeExcludedTools(parent.ExcludedTools, cfg.ExcludedTools)
+	if len(excludedTools) > 0 {
+		opts = append(opts, session.WithExcludedTools(excludedTools))
+	}
 	return session.New(opts...)
 }
 
+// mergeExcludedTools combines two excluded-tool lists, deduplicating entries.
+// It returns nil when both inputs are empty.
+func mergeExcludedTools(parent, child []string) []string {
+	if len(parent) == 0 {
+		return child
+	}
+	if len(child) == 0 {
+		return parent
+	}
+	set := make(map[string]struct{}, len(parent)+len(child))
+	for _, t := range parent {
+		set[t] = struct{}{}
+	}
+	for _, t := range child {
+		set[t] = struct{}{}
+	}
+	merged := make([]string, 0, len(set))
+	for t := range set {
+		merged = append(merged, t)
+	}
+	return merged
+}
+
 // runSubSessionForwarding runs a child session within the parent, forwarding all
 // events to the caller's event channel and propagating tool approval state
 // back to the parent when done.

pkg/runtime/loop.go

@@ -104,6 +104,7 @@ func (r *LocalRuntime) RunStream(ctx context.Context, sess *session.Session) <-c
 			events <- Error(fmt.Sprintf("failed to get tools: %v", err))
 			return
 		}
+		agentTools = filterExcludedTools(agentTools, sess.ExcludedTools)
 
 		events <- ToolsetInfo(len(agentTools), false, a.Name())
 
@@ -158,6 +159,7 @@ func (r *LocalRuntime) RunStream(ctx context.Context, sess *session.Session) <-c
 				events <- Error(fmt.Sprintf("failed to get tools: %v", err))
 				return
 			}
+			agentTools = filterExcludedTools(agentTools, sess.ExcludedTools)
 
 			// Emit updated tool count. After a ToolListChanged MCP notification
 			// the cache is invalidated, so getTools above re-fetches from the
@@ -573,6 +575,25 @@ func formatToolWarning(a *agent.Agent, warnings []string) string {
 	return strings.TrimSuffix(builder.String(), "\n")
 }
 
+// filterExcludedTools removes tools whose names appear in the excluded list.
+// This is used by skill sub-sessions to prevent recursive run_skill calls.
+func filterExcludedTools(agentTools []tools.Tool, excluded []string) []tools.Tool {
+	if len(excluded) == 0 {
+		return agentTools
+	}
+	excludeSet := make(map[string]bool, len(excluded))
+	for _, name := range excluded {
+		excludeSet[name] = true
+	}
+	filtered := make([]tools.Tool, 0, len(agentTools))
+	for _, t := range agentTools {
+		if !excludeSet[t.Name] {
+			filtered = append(filtered, t)
+		}
+	}
+	return filtered
+}
+
 // chanSend wraps a channel as a func(Event) for use with emitAgentWarnings
 // and RAG event forwarding. The send is non-blocking: if the channel is full
 // or closed, the event is silently dropped. This prevents a panic when a

pkg/runtime/runtime_test.go

@@ -1901,3 +1901,52 @@ func TestProcessToolCalls_UsesPinnedAgent(t *testing.T) {
 		}
 	}
 }
+
+func TestFilterExcludedTools(t *testing.T) {
+	allTools := []tools.Tool{
+		{Name: "read_skill"},
+		{Name: "run_skill"},
+		{Name: "shell"},
+	}
+
+	t.Run("no exclusions returns all tools", func(t *testing.T) {
+		result := filterExcludedTools(allTools, nil)
+		assert.Len(t, result, 3)
+	})
+
+	t.Run("excludes run_skill", func(t *testing.T) {
+		result := filterExcludedTools(allTools, []string{"run_skill"})
+		assert.Len(t, result, 2)
+		for _, tool := range result {
+			assert.NotEqual(t, "run_skill", tool.Name)
+		}
+	})
+
+	t.Run("excludes multiple tools", func(t *testing.T) {
+		result := filterExcludedTools(allTools, []string{"run_skill", "shell"})
+		assert.Len(t, result, 1)
+		assert.Equal(t, "read_skill", result[0].Name)
+	})
+}
+
+func TestMergeExcludedTools(t *testing.T) {
+	t.Run("both empty", func(t *testing.T) {
+		assert.Nil(t, mergeExcludedTools(nil, nil))
+	})
+
+	t.Run("parent only", func(t *testing.T) {
+		result := mergeExcludedTools([]string{"run_skill"}, nil)
+		assert.Equal(t, []string{"run_skill"}, result)
+	})
+
+	t.Run("child only", func(t *testing.T) {
+		result := mergeExcludedTools(nil, []string{"run_skill"})
+		assert.Equal(t, []string{"run_skill"}, result)
+	})
+
+	t.Run("deduplicates", func(t *testing.T) {
+		result := mergeExcludedTools([]string{"run_skill", "shell"}, []string{"run_skill", "read_skill"})
+		assert.Len(t, result, 3)
+		assert.ElementsMatch(t, []string{"run_skill", "shell", "read_skill"}, result)
+	})
+}

pkg/runtime/skill_runner.go

@@ -74,6 +74,7 @@ func (r *LocalRuntime) handleRunSkill(ctx context.Context, sess *session.Session
 		AgentName:           ca,
 		Title:               "Skill: " + params.Name,
 		ToolsApproved:       sess.ToolsApproved,
+		ExcludedTools:       []string{builtin.ToolNameRunSkill},
 	}
 
 	s := newSubSession(sess, cfg, a)

pkg/session/session.go

@@ -128,6 +128,11 @@ type Session struct {
 	// These are shown in the model picker for easy re-selection.
 	CustomModelsUsed []string `json:"custom_models_used,omitempty"`
 
+	// ExcludedTools lists tool names that should be filtered out of the agent's
+	// tool list for this session. This is used by skill sub-sessions to prevent
+	// recursive run_skill calls.
+	ExcludedTools []string `json:"-"`
+
 	// AgentName, when set, tells RunStream which agent to use for this session
 	// instead of reading from the shared runtime currentAgent field. This is
 	// required for background agent tasks where multiple sessions may run
@@ -524,6 +529,15 @@ func WithParentID(parentID string) Opt {
 	}
 }
 
+// WithExcludedTools sets tool names that should be filtered out of the agent's
+// tool list for this session. This prevents recursive tool calls in skill
+// sub-sessions.
+func WithExcludedTools(names []string) Opt {
+	return func(s *Session) {
+		s.ExcludedTools = names
+	}
+}
+
 // IsSubSession returns true if this session is a sub-session (has a parent).
 func (s *Session) IsSubSession() bool {
 	return s.ParentID != ""