diff --git a/docs/features/skills/index.md b/docs/features/skills/index.md index 7cfcdfe3c..91c962f4d 100644 --- a/docs/features/skills/index.md +++ b/docs/features/skills/index.md @@ -57,6 +57,53 @@ When asked to create a Dockerfile: 4. Follow security best practices (non-root user, etc.) ``` +### Frontmatter Fields + +| Field | Required | Description | +| ---------------- | -------- | --------------------------------------------------------------------------- | +| `name` | Yes | Unique skill identifier | +| `description` | Yes | Short description shown to the agent for skill matching | +| `context` | No | Set to `fork` to run the skill as an isolated sub-agent (see below) | +| `allowed-tools` | No | List of tools the skill needs (YAML list or comma-separated string) | +| `license` | No | License identifier (e.g. `Apache-2.0`) | +| `compatibility` | No | Free-text compatibility notes | +| `metadata` | No | Arbitrary key-value pairs (e.g. `author`, `version`) | + +## Running a Skill as a Sub-Agent + +By default, when an agent invokes a skill it reads the instructions inline into its own conversation. For complex, multi-step skills this can consume a large portion of the agent's context window and pollute the parent conversation with intermediate tool calls. + +Adding `context: fork` to the SKILL.md frontmatter tells the agent to run the skill in an **isolated sub-agent** instead: + + +```yaml +--- +name: bump-go-dependencies +description: Update Go module dependencies one by one +context: fork +--- + +# Bump Dependencies + +1. List outdated deps +2. Update each one, run tests, commit or revert +3. Produce a summary table +``` + +When the agent encounters a task that matches a `context: fork` skill, it uses the `run_skill` tool instead of `read_skill`. This: + +- **Spawns a child session** with the skill content as the system prompt and the caller's task as the user message +- **Isolates the context window** — the sub-agent has its own conversation history, so lengthy tool-call chains don't eat into the parent's token budget +- **Folds the result** — only the sub-agent's final answer is returned to the parent as the tool result +- **Inherits the parent's model and tools** — the sub-agent can use all tools available to the parent agent + +
+
💡 When to use context: fork +
+

Use context: fork for skills that involve many steps, heavy tool usage, or that should not clutter the main conversation — for example dependency bumping, large refactors, or code generation pipelines.

+ +
+ ## Search Paths Skills are discovered from these locations (later overrides earlier): diff --git a/pkg/runtime/agent_delegation.go b/pkg/runtime/agent_delegation.go index 36bde3d2f..ad2b75422 100644 --- a/pkg/runtime/agent_delegation.go +++ b/pkg/runtime/agent_delegation.go @@ -58,55 +58,110 @@ func buildTaskSystemMessage(task, expectedOutput string) string { return msg } -// CurrentAgentSubAgentNames implements agenttool.Runner. -func (r *LocalRuntime) CurrentAgentSubAgentNames() []string { - a := r.CurrentAgent() - if a == nil { - return nil - } - return agentNames(a.SubAgents()) +// SubSessionConfig describes how to build and run a child session. +// Both handleTaskTransfer and RunAgent (background agents) use this +// to avoid duplicating session-construction logic. Future callers +// (e.g. skill-as-sub-agent) can use it as well. +type SubSessionConfig struct { + // Task is the user-facing task description. + Task string + // ExpectedOutput is an optional description of what the sub-agent should produce. + ExpectedOutput string + // SystemMessage, when non-empty, replaces the default task-based system + // message. This is used by skill sub-agents whose system prompt is the + // skill content itself rather than the team delegation boilerplate. + SystemMessage string + // AgentName is the name of the agent that will execute the sub-session. + AgentName string + // Title is a human-readable label for the sub-session (e.g. "Transferred task"). + Title string + // ToolsApproved overrides whether tools are pre-approved in the child session. + ToolsApproved bool + // Thinking propagates the parent's thinking-mode flag. + Thinking bool + // PinAgent, when true, pins the child session to AgentName via + // session.WithAgentName. This is required for concurrent background + // tasks that must not share the runtime's mutable currentAgent field. + PinAgent bool + // ImplicitUserMessage, when non-empty, overrides the default "Please proceed." + // 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 } -// RunAgent implements agenttool.Runner. It starts a sub-agent synchronously and -// blocks until completion or cancellation. -func (r *LocalRuntime) RunAgent(ctx context.Context, params agenttool.RunParams) *agenttool.RunResult { - child, err := r.team.Agent(params.AgentName) - if err != nil { - return &agenttool.RunResult{ErrMsg: fmt.Sprintf("agent %q not found: %s", params.AgentName, err)} +// newSubSession builds a *session.Session from a SubSessionConfig and a parent +// session. It consolidates the session options that were previously duplicated +// across handleTaskTransfer and RunAgent. +func newSubSession(parent *session.Session, cfg SubSessionConfig, childAgent *agent.Agent) *session.Session { + sysMsg := cfg.SystemMessage + if sysMsg == "" { + sysMsg = buildTaskSystemMessage(cfg.Task, cfg.ExpectedOutput) } - sess := params.ParentSession + userMsg := cfg.ImplicitUserMessage + if userMsg == "" { + userMsg = "Please proceed." + } - // Background tasks run with tools pre-approved because there is no user present - // to respond to interactive approval prompts during async execution. This is a - // deliberate design trade-off: the user implicitly authorises all tool calls made - // by the sub-agent when they approve run_background_agent. Callers should be aware - // that prompt injection in the sub-agent's context could exploit this gate-bypass. - // - // TODO: propagate the parent session's per-tool permission rules once the runtime - // supports per-session permission scoping rather than a single shared ToolsApproved flag. - s := session.New( - session.WithSystemMessage(buildTaskSystemMessage(params.Task, params.ExpectedOutput)), - session.WithImplicitUserMessage("Please proceed."), - session.WithMaxIterations(child.MaxIterations()), - session.WithMaxConsecutiveToolCalls(child.MaxConsecutiveToolCalls()), - session.WithTitle("Background agent task"), - session.WithToolsApproved(true), - session.WithThinking(sess.Thinking), + opts := []session.Opt{ + session.WithSystemMessage(sysMsg), + session.WithImplicitUserMessage(userMsg), + session.WithMaxIterations(childAgent.MaxIterations()), + session.WithMaxConsecutiveToolCalls(childAgent.MaxConsecutiveToolCalls()), + session.WithTitle(cfg.Title), + session.WithToolsApproved(cfg.ToolsApproved), + session.WithThinking(cfg.Thinking), session.WithSendUserMessage(false), - session.WithParentID(sess.ID), - session.WithAgentName(params.AgentName), - ) + session.WithParentID(parent.ID), + } + if cfg.PinAgent { + opts = append(opts, session.WithAgentName(cfg.AgentName)) + } + return session.New(opts...) +} + +// runSubSessionForwarding runs a child session within the parent, forwarding all +// events to the caller's event channel and propagating session state (tool +// approvals, thinking) back to the parent when done. +// +// This is the "interactive" path used by transfer_task where the parent agent +// loop is blocked while the child executes. +func (r *LocalRuntime) runSubSessionForwarding(ctx context.Context, parent, child *session.Session, span trace.Span, evts chan Event, callerAgent string) (*tools.ToolCallResult, error) { + for event := range r.RunStream(ctx, child) { + evts <- event + if errEvent, ok := event.(*ErrorEvent); ok { + span.RecordError(fmt.Errorf("%s", errEvent.Error)) + span.SetStatus(codes.Error, "sub-session error") + return nil, fmt.Errorf("%s", errEvent.Error) + } + } + parent.ToolsApproved = child.ToolsApproved + parent.Thinking = child.Thinking + + parent.AddSubSession(child) + evts <- SubSessionCompleted(parent.ID, child, callerAgent) + + span.SetStatus(codes.Ok, "sub-session completed") + return tools.ResultSuccess(child.GetLastAssistantMessageContent()), nil +} + +// runSubSessionCollecting runs a child session, collecting output via an +// optional content callback instead of forwarding events. This is the path +// used by background agents and other non-interactive callers. +// +// It returns a RunResult containing either the final assistant message or +// an error message. +func (r *LocalRuntime) runSubSessionCollecting(ctx context.Context, parent, child *session.Session, onContent func(string)) *agenttool.RunResult { var errMsg string - events := r.RunStream(ctx, s) + events := r.RunStream(ctx, child) for event := range events { if ctx.Err() != nil { break } if choice, ok := event.(*AgentChoiceEvent); ok && choice.Content != "" { - if params.OnContent != nil { - params.OnContent(choice.Content) + if onContent != nil { + onContent(choice.Content) } } if errEvt, ok := event.(*ErrorEvent); ok { @@ -123,11 +178,53 @@ func (r *LocalRuntime) RunAgent(ctx context.Context, params agenttool.RunParams) return &agenttool.RunResult{ErrMsg: errMsg} } - result := s.GetLastAssistantMessageContent() - sess.AddSubSession(s) + result := child.GetLastAssistantMessageContent() + parent.AddSubSession(child) return &agenttool.RunResult{Result: result} } +// CurrentAgentSubAgentNames implements agenttool.Runner. +func (r *LocalRuntime) CurrentAgentSubAgentNames() []string { + a := r.CurrentAgent() + if a == nil { + return nil + } + return agentNames(a.SubAgents()) +} + +// RunAgent implements agenttool.Runner. It starts a sub-agent synchronously and +// blocks until completion or cancellation. +func (r *LocalRuntime) RunAgent(ctx context.Context, params agenttool.RunParams) *agenttool.RunResult { + child, err := r.team.Agent(params.AgentName) + if err != nil { + return &agenttool.RunResult{ErrMsg: fmt.Sprintf("agent %q not found: %s", params.AgentName, err)} + } + + sess := params.ParentSession + + // Background tasks run with tools pre-approved because there is no user present + // to respond to interactive approval prompts during async execution. This is a + // deliberate design trade-off: the user implicitly authorises all tool calls made + // by the sub-agent when they approve run_background_agent. Callers should be aware + // that prompt injection in the sub-agent's context could exploit this gate-bypass. + // + // TODO: propagate the parent session's per-tool permission rules once the runtime + // supports per-session permission scoping rather than a single shared ToolsApproved flag. + cfg := SubSessionConfig{ + Task: params.Task, + ExpectedOutput: params.ExpectedOutput, + AgentName: params.AgentName, + Title: "Background agent task", + ToolsApproved: true, + Thinking: sess.Thinking, + PinAgent: true, + } + + s := newSubSession(sess, cfg, child) + + return r.runSubSessionCollecting(ctx, sess, s, params.OnContent) +} + func (r *LocalRuntime) handleTaskTransfer(ctx context.Context, sess *session.Session, toolCall tools.ToolCall, evts chan Event) (*tools.ToolCallResult, error) { var params struct { Agent string `json:"agent"` @@ -185,41 +282,18 @@ func (r *LocalRuntime) handleTaskTransfer(ctx context.Context, sess *session.Ses return nil, err } - s := session.New( - session.WithSystemMessage(buildTaskSystemMessage(params.Task, params.ExpectedOutput)), - session.WithImplicitUserMessage("Please proceed."), - session.WithMaxIterations(child.MaxIterations()), - session.WithMaxConsecutiveToolCalls(child.MaxConsecutiveToolCalls()), - session.WithTitle("Transferred task"), - session.WithToolsApproved(sess.ToolsApproved), - session.WithThinking(sess.Thinking), - session.WithSendUserMessage(false), - session.WithParentID(sess.ID), - ) - - return r.runSubSession(ctx, sess, s, span, evts, a.Name()) -} - -// runSubSession runs a child session within the parent, forwarding events and -// propagating state (tool approvals, thinking) back to the parent when done. -func (r *LocalRuntime) runSubSession(ctx context.Context, parent, child *session.Session, span trace.Span, evts chan Event, agentName string) (*tools.ToolCallResult, error) { - for event := range r.RunStream(ctx, child) { - evts <- event - if errEvent, ok := event.(*ErrorEvent); ok { - span.RecordError(fmt.Errorf("%s", errEvent.Error)) - span.SetStatus(codes.Error, "sub-session error") - return nil, fmt.Errorf("%s", errEvent.Error) - } + cfg := SubSessionConfig{ + Task: params.Task, + ExpectedOutput: params.ExpectedOutput, + AgentName: params.Agent, + Title: "Transferred task", + ToolsApproved: sess.ToolsApproved, + Thinking: sess.Thinking, } - parent.ToolsApproved = child.ToolsApproved - parent.Thinking = child.Thinking - - parent.AddSubSession(child) - evts <- SubSessionCompleted(parent.ID, child, agentName) + s := newSubSession(sess, cfg, child) - span.SetStatus(codes.Ok, "sub-session completed") - return tools.ResultSuccess(child.GetLastAssistantMessageContent()), nil + return r.runSubSessionForwarding(ctx, sess, s, span, evts, a.Name()) } func (r *LocalRuntime) handleHandoff(_ context.Context, _ *session.Session, toolCall tools.ToolCall, _ chan Event) (*tools.ToolCallResult, error) { diff --git a/pkg/runtime/agent_delegation_test.go b/pkg/runtime/agent_delegation_test.go new file mode 100644 index 000000000..c7f2f1c75 --- /dev/null +++ b/pkg/runtime/agent_delegation_test.go @@ -0,0 +1,198 @@ +package runtime + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/agent" + "github.com/docker/docker-agent/pkg/session" +) + +func TestBuildTaskSystemMessage(t *testing.T) { + t.Run("with expected output", func(t *testing.T) { + msg := buildTaskSystemMessage("do the thing", "a result") + assert.Contains(t, msg, "\ndo the thing\n") + assert.Contains(t, msg, "\na result\n") + }) + + t.Run("without expected output", func(t *testing.T) { + msg := buildTaskSystemMessage("do the thing", "") + assert.Contains(t, msg, "\ndo the thing\n") + assert.NotContains(t, msg, "expected_output") + }) +} + +func TestAgentNames(t *testing.T) { + agents := []*agent.Agent{ + agent.New("alpha", ""), + agent.New("beta", ""), + } + assert.Equal(t, []string{"alpha", "beta"}, agentNames(agents)) + assert.Empty(t, agentNames(nil)) +} + +func TestValidateAgentInList(t *testing.T) { + agents := []*agent.Agent{ + agent.New("sub1", ""), + agent.New("sub2", ""), + } + + t.Run("valid agent returns nil", func(t *testing.T) { + result := validateAgentInList("root", "sub1", "transfer to", "sub-agents", agents) + assert.Nil(t, result) + }) + + t.Run("invalid agent with non-empty list", func(t *testing.T) { + result := validateAgentInList("root", "missing", "transfer to", "sub-agents", agents) + require.NotNil(t, result) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "sub1") + assert.Contains(t, result.Output, "sub2") + }) + + t.Run("invalid agent with empty list", func(t *testing.T) { + result := validateAgentInList("root", "missing", "transfer to", "sub-agents", nil) + require.NotNil(t, result) + assert.True(t, result.IsError) + assert.Contains(t, result.Output, "No agents are configured") + }) +} + +func TestNewSubSession(t *testing.T) { + parent := session.New(session.WithUserMessage("hello")) + childAgent := agent.New("worker", "a worker agent", + agent.WithMaxIterations(10), + ) + + t.Run("basic config", func(t *testing.T) { + cfg := SubSessionConfig{ + Task: "write tests", + ExpectedOutput: "passing tests", + AgentName: "worker", + Title: "Test task", + ToolsApproved: true, + Thinking: true, + } + + s := newSubSession(parent, cfg, childAgent) + + assert.Equal(t, parent.ID, s.ParentID) + assert.Equal(t, "Test task", s.Title) + assert.True(t, s.ToolsApproved) + assert.True(t, s.Thinking) + assert.False(t, s.SendUserMessage) + assert.Equal(t, 10, s.MaxIterations) + // AgentName should NOT be set when PinAgent is false + assert.Empty(t, s.AgentName) + }) + + t.Run("pin agent", func(t *testing.T) { + cfg := SubSessionConfig{ + Task: "background work", + AgentName: "worker", + Title: "Background task", + PinAgent: true, + } + + s := newSubSession(parent, cfg, childAgent) + + assert.Equal(t, "worker", s.AgentName) + }) + + t.Run("custom implicit user message", func(t *testing.T) { + cfg := SubSessionConfig{ + Task: "bump deps", + AgentName: "worker", + Title: "Skill task", + ImplicitUserMessage: "Update all Go dependencies", + } + + s := newSubSession(parent, cfg, childAgent) + + // The implicit user message should be the custom one, not "Please proceed." + assert.Equal(t, "Update all Go dependencies", s.GetLastUserMessageContent()) + }) + + t.Run("default implicit user message", func(t *testing.T) { + cfg := SubSessionConfig{ + Task: "do work", + AgentName: "worker", + Title: "Task", + } + + s := newSubSession(parent, cfg, childAgent) + + assert.Equal(t, "Please proceed.", s.GetLastUserMessageContent()) + }) + + t.Run("custom system message", func(t *testing.T) { + cfg := SubSessionConfig{ + Task: "bump deps", + SystemMessage: "You are a skill sub-agent. Follow these instructions.", + AgentName: "worker", + Title: "Skill task", + } + + s := newSubSession(parent, cfg, childAgent) + + // When SystemMessage is set, the default task-based message should not be used. + // We can verify the user message is still the default. + assert.Equal(t, "Please proceed.", s.GetLastUserMessageContent()) + }) +} + +func TestSubSessionConfig_DefaultValues(t *testing.T) { + // Verify zero-value SubSessionConfig produces a valid session + parent := session.New(session.WithUserMessage("hello")) + childAgent := agent.New("worker", "") + + cfg := SubSessionConfig{ + Task: "minimal task", + AgentName: "worker", + Title: "Minimal", + } + + s := newSubSession(parent, cfg, childAgent) + + assert.False(t, s.ToolsApproved) + assert.False(t, s.Thinking) + assert.False(t, s.SendUserMessage) + assert.Empty(t, s.AgentName) +} + +func TestSubSessionConfig_InheritsAgentLimits(t *testing.T) { + parent := session.New(session.WithUserMessage("hello")) + + t.Run("with custom limits", func(t *testing.T) { + childAgent := agent.New("worker", "", + agent.WithMaxIterations(42), + agent.WithMaxConsecutiveToolCalls(7), + ) + + cfg := SubSessionConfig{ + Task: "work", + AgentName: "worker", + Title: "test", + } + + s := newSubSession(parent, cfg, childAgent) + assert.Equal(t, 42, s.MaxIterations) + assert.Equal(t, 7, s.MaxConsecutiveToolCalls) + }) + + t.Run("with zero limits (defaults)", func(t *testing.T) { + childAgent := agent.New("worker", "") + + cfg := SubSessionConfig{ + Task: "work", + AgentName: "worker", + Title: "test", + } + + s := newSubSession(parent, cfg, childAgent) + assert.Equal(t, 0, s.MaxIterations) + assert.Equal(t, 0, s.MaxConsecutiveToolCalls) + }) +} diff --git a/pkg/runtime/loop.go b/pkg/runtime/loop.go index 3f768e958..d841b35b4 100644 --- a/pkg/runtime/loop.go +++ b/pkg/runtime/loop.go @@ -33,6 +33,7 @@ func (r *LocalRuntime) registerDefaultTools() { r.toolMap[builtin.ToolNameHandoff] = r.handleHandoff r.toolMap[builtin.ToolNameChangeModel] = r.handleChangeModel r.toolMap[builtin.ToolNameRevertModel] = r.handleRevertModel + r.toolMap[builtin.ToolNameRunSkill] = r.handleRunSkill r.bgAgents.RegisterHandlers(func(name string, fn func(context.Context, *session.Session, tools.ToolCall) (*tools.ToolCallResult, error)) { r.toolMap[name] = func(ctx context.Context, sess *session.Session, tc tools.ToolCall, _ chan Event) (*tools.ToolCallResult, error) { diff --git a/pkg/runtime/skill_runner.go b/pkg/runtime/skill_runner.go new file mode 100644 index 000000000..3cdc19bb2 --- /dev/null +++ b/pkg/runtime/skill_runner.go @@ -0,0 +1,83 @@ +package runtime + +import ( + "context" + "encoding/json" + "fmt" + "log/slog" + + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/trace" + + "github.com/docker/docker-agent/pkg/session" + "github.com/docker/docker-agent/pkg/tools" + "github.com/docker/docker-agent/pkg/tools/builtin" +) + +// handleRunSkill executes a skill as an isolated sub-agent. The skill's +// SKILL.md content (with command expansions) becomes the system prompt, and +// the caller-provided task becomes the implicit user message. The sub-agent +// runs in a child session using the current agent's model and tools, and +// its final response is returned as the tool result. +// +// This implements the `context: fork` behaviour from the SKILL.md +// frontmatter, following the same convention as Claude Code. +func (r *LocalRuntime) handleRunSkill(ctx context.Context, sess *session.Session, toolCall tools.ToolCall, evts chan Event) (*tools.ToolCallResult, error) { + var params builtin.RunSkillArgs + if err := json.Unmarshal([]byte(toolCall.Function.Arguments), ¶ms); err != nil { + return nil, fmt.Errorf("invalid arguments: %w", err) + } + + st := r.CurrentAgentSkillsToolset() + if st == nil { + return tools.ResultError("no skills are available for the current agent"), nil + } + + skill := st.FindSkill(params.Name) + if skill == nil { + return tools.ResultError(fmt.Sprintf("skill %q not found", params.Name)), nil + } + + if !skill.IsFork() { + return tools.ResultError(fmt.Sprintf( + "skill %q is not configured for sub-agent execution (missing context: fork in SKILL.md frontmatter); use read_skill instead", + params.Name, + )), nil + } + + // Load and expand the skill content for the system prompt. + skillContent, err := st.ReadSkillContent(ctx, params.Name) + if err != nil { + return tools.ResultError(fmt.Sprintf("failed to read skill content: %s", err)), nil + } + + a := r.CurrentAgent() + ca := a.Name() + + ctx, span := r.startSpan(ctx, "runtime.run_skill", trace.WithAttributes( + attribute.String("agent", ca), + attribute.String("skill", params.Name), + attribute.String("session.id", sess.ID), + )) + defer span.End() + + slog.Debug("Running skill as sub-agent", + "agent", ca, + "skill", params.Name, + "task", params.Task, + ) + + cfg := SubSessionConfig{ + Task: params.Task, + SystemMessage: skillContent, + ImplicitUserMessage: params.Task, + AgentName: ca, + Title: "Skill: " + params.Name, + ToolsApproved: sess.ToolsApproved, + Thinking: sess.Thinking, + } + + s := newSubSession(sess, cfg, a) + + return r.runSubSessionForwarding(ctx, sess, s, span, evts, ca) +} diff --git a/pkg/skills/skills.go b/pkg/skills/skills.go index 0fcd5e782..9ee61c772 100644 --- a/pkg/skills/skills.go +++ b/pkg/skills/skills.go @@ -27,6 +27,14 @@ type Skill struct { Compatibility string `yaml:"compatibility"` Metadata map[string]string `yaml:"metadata"` AllowedTools stringOrList `yaml:"allowed-tools"` + Context string `yaml:"context"` // "fork" to run the skill as an isolated sub-agent +} + +// IsFork returns true when the skill should be executed in an isolated +// sub-agent context rather than inline in the current conversation. +// This matches Claude Code's `context: fork` frontmatter syntax. +func (s *Skill) IsFork() bool { + return s.Context == "fork" } // stringOrList is a []string that can be unmarshalled from either a YAML list diff --git a/pkg/skills/skills_test.go b/pkg/skills/skills_test.go index 38f730c2d..35ab553e8 100644 --- a/pkg/skills/skills_test.go +++ b/pkg/skills/skills_test.go @@ -125,6 +125,40 @@ Body`, }, wantOK: true, }, + { + name: "context fork", + content: `--- +name: forked-skill +description: A skill that runs as a sub-agent +context: fork +--- + +Body`, + want: Skill{ + Name: "forked-skill", + Description: "A skill that runs as a sub-agent", + Context: "fork", + }, + wantOK: true, + }, + { + name: "context fork with allowed-tools", + content: `--- +name: scoped-fork +description: Fork skill with tool restrictions +context: fork +allowed-tools: Read, Grep +--- + +Body`, + want: Skill{ + Name: "scoped-fork", + Description: "Fork skill with tool restrictions", + Context: "fork", + AllowedTools: stringOrList{"Read", "Grep"}, + }, + wantOK: true, + }, } for _, tt := range tests { @@ -138,6 +172,7 @@ Body`, assert.Equal(t, tt.want.Compatibility, got.Compatibility) assert.Equal(t, tt.want.Metadata, got.Metadata) assert.Equal(t, tt.want.AllowedTools, got.AllowedTools) + assert.Equal(t, tt.want.Context, got.Context) } }) } @@ -607,6 +642,13 @@ func TestFindGitRoot(t *testing.T) { }) } +func TestSkill_IsFork(t *testing.T) { + assert.True(t, (&Skill{Context: "fork"}).IsFork()) + assert.False(t, (&Skill{Context: ""}).IsFork()) + assert.False(t, (&Skill{Context: "inline"}).IsFork()) + assert.False(t, (&Skill{}).IsFork()) +} + func TestProjectSearchDirs(t *testing.T) { t.Run("in git repo", func(t *testing.T) { tmpRepo := t.TempDir() diff --git a/pkg/tools/builtin/skills.go b/pkg/tools/builtin/skills.go index 38379c00b..fe9fbb116 100644 --- a/pkg/tools/builtin/skills.go +++ b/pkg/tools/builtin/skills.go @@ -14,6 +14,7 @@ import ( const ( ToolNameReadSkill = "read_skill" ToolNameReadSkillFile = "read_skill_file" + ToolNameRunSkill = "run_skill" ) var ( @@ -50,6 +51,11 @@ func (s *SkillsToolset) findSkill(name string) *skills.Skill { return nil } +// FindSkill returns the skill with the given name, or nil if not found. +func (s *SkillsToolset) FindSkill(name string) *skills.Skill { + return s.findSkill(name) +} + // ReadSkillContent returns the content of a skill's SKILL.md by name. // For local skills, it expands any !`command` patterns in the content by // executing the commands and replacing the patterns with their stdout output. @@ -154,6 +160,16 @@ func (s *SkillsToolset) hasFiles() bool { return false } +// hasForkSkills reports whether any loaded skill uses context: fork. +func (s *SkillsToolset) hasForkSkills() bool { + for i := range s.skills { + if s.skills[i].IsFork() { + return true + } + } + return false +} + func (s *SkillsToolset) Instructions() string { if len(s.skills) == 0 { return "" @@ -163,6 +179,13 @@ func (s *SkillsToolset) Instructions() string { sb.WriteString("Skills provide specialized instructions for specific tasks. ") sb.WriteString("When a user's request matches a skill's description, use read_skill to load its instructions.\n\n") + hasFork := s.hasForkSkills() + if hasFork { + sb.WriteString("Some skills are configured to run as isolated sub-agents (context: fork). ") + sb.WriteString("For those skills use run_skill instead of read_skill so they execute in a dedicated context ") + sb.WriteString("with their own conversation history.\n\n") + } + if s.hasFiles() { sb.WriteString("Some skills have supporting files. ") sb.WriteString("Use read_skill_file to load referenced files on demand — do not preload them.\n\n") @@ -177,6 +200,9 @@ func (s *SkillsToolset) Instructions() string { sb.WriteString(" ") sb.WriteString(skill.Description) sb.WriteString("\n") + if skill.IsFork() { + sb.WriteString(" sub-agent\n") + } if len(skill.Files) > 1 { sb.WriteString(" ") // List files excluding SKILL.md itself @@ -200,6 +226,12 @@ func (s *SkillsToolset) Instructions() string { return sb.String() } +// RunSkillArgs specifies the parameters for the run_skill tool. +type RunSkillArgs struct { + Name string `json:"name" jsonschema:"The name of the skill to run as a sub-agent"` + Task string `json:"task" jsonschema:"A clear description of the task the skill sub-agent should achieve"` +} + func (s *SkillsToolset) Tools(context.Context) ([]tools.Tool, error) { if len(s.skills) == 0 { return nil, nil @@ -236,5 +268,19 @@ func (s *SkillsToolset) Tools(context.Context) ([]tools.Tool, error) { }) } + // Expose run_skill if any skill uses context: fork + if s.hasForkSkills() { + result = append(result, tools.Tool{ + Name: ToolNameRunSkill, + Category: "skills", + Description: "Run a skill as an isolated sub-agent with its own conversation context. Use this for skills marked with sub-agent mode.", + Parameters: tools.MustSchemaFor[RunSkillArgs](), + OutputSchema: tools.MustSchemaFor[string](), + Annotations: tools.ToolAnnotations{ + Title: "Run Skill", + }, + }) + } + return result, nil } diff --git a/pkg/tools/builtin/skills_test.go b/pkg/tools/builtin/skills_test.go index 5fc0c20cf..7242c7ea1 100644 --- a/pkg/tools/builtin/skills_test.go +++ b/pkg/tools/builtin/skills_test.go @@ -280,3 +280,102 @@ func TestSkillsToolset_ReadSkillContent_RemoteSkillSkipsExpansion(t *testing.T) require.NoError(t, err) assert.Equal(t, content, result, "commands in remote skills must not be expanded") } + +func TestSkillsToolset_FindSkill(t *testing.T) { + st := NewSkillsToolset([]skills.Skill{ + {Name: "alpha", Description: "Alpha skill"}, + {Name: "beta", Description: "Beta skill"}, + }, "") + + found := st.FindSkill("alpha") + require.NotNil(t, found) + assert.Equal(t, "alpha", found.Name) + + found = st.FindSkill("beta") + require.NotNil(t, found) + assert.Equal(t, "beta", found.Name) + + assert.Nil(t, st.FindSkill("missing")) +} + +func TestSkillsToolset_Instructions_ForkSkills(t *testing.T) { + st := NewSkillsToolset([]skills.Skill{ + {Name: "inline-skill", Description: "Runs inline"}, + {Name: "fork-skill", Description: "Runs as sub-agent", Context: "fork"}, + }, "") + + instructions := st.Instructions() + + // Should mention run_skill for fork skills + assert.Contains(t, instructions, "run_skill") + assert.Contains(t, instructions, "context: fork") + assert.Contains(t, instructions, "isolated sub-agents") + + // Fork skill should have sub-agent mode tag + assert.Contains(t, instructions, "sub-agent") + + // Inline skill should NOT have the mode tag + // We check that inline-skill's entry does not contain + assert.Contains(t, instructions, "inline-skill") + assert.Contains(t, instructions, "fork-skill") +} + +func TestSkillsToolset_Instructions_NoForkSkills(t *testing.T) { + st := NewSkillsToolset([]skills.Skill{ + {Name: "normal", Description: "Normal skill"}, + }, "") + + instructions := st.Instructions() + + // Should NOT mention run_skill or sub-agent + assert.NotContains(t, instructions, "run_skill") + assert.NotContains(t, instructions, "sub-agent") + assert.NotContains(t, instructions, "context: fork") +} + +func TestSkillsToolset_Tools_WithForkSkills(t *testing.T) { + st := NewSkillsToolset([]skills.Skill{ + {Name: "inline", Description: "Inline skill"}, + {Name: "forked", Description: "Forked skill", Context: "fork"}, + }, "") + + result, err := st.Tools(t.Context()) + require.NoError(t, err) + + // Should have read_skill + run_skill (no files, so no read_skill_file) + require.Len(t, result, 2) + assert.Equal(t, ToolNameReadSkill, result[0].Name) + assert.Equal(t, ToolNameRunSkill, result[1].Name) +} + +func TestSkillsToolset_Tools_NoForkSkills(t *testing.T) { + st := NewSkillsToolset([]skills.Skill{ + {Name: "inline", Description: "Inline skill"}, + }, "") + + result, err := st.Tools(t.Context()) + require.NoError(t, err) + + // Should only have read_skill + require.Len(t, result, 1) + assert.Equal(t, ToolNameReadSkill, result[0].Name) +} + +func TestSkillsToolset_Tools_ForkAndFiles(t *testing.T) { + st := NewSkillsToolset([]skills.Skill{ + {Name: "full", Description: "Full skill", Context: "fork", Files: []string{"SKILL.md", "ref.md"}}, + }, "") + + result, err := st.Tools(t.Context()) + require.NoError(t, err) + + // Should have read_skill + read_skill_file + run_skill + require.Len(t, result, 3) + names := make([]string, len(result)) + for i, tool := range result { + names[i] = tool.Name + } + assert.Contains(t, names, ToolNameReadSkill) + assert.Contains(t, names, ToolNameReadSkillFile) + assert.Contains(t, names, ToolNameRunSkill) +}