From d30c823c0eb76421a2546573699bb0a8cc0a1f5d Mon Sep 17 00:00:00 2001 From: Cai_Tang <106404101+Cai-Tang-www@users.noreply.github.com> Date: Sat, 18 Apr 2026 15:31:29 +0800 Subject: [PATCH 1/5] =?UTF-8?q?feat:=20=E5=AD=90=E4=BB=A3=E7=90=86?= =?UTF-8?q?=E6=8E=A5=E5=85=A5=E5=B7=A5=E5=85=B7=E8=B0=83=E7=94=A8=E9=97=AD?= =?UTF-8?q?=E7=8E=AF=E4=B8=8E=E5=AE=89=E5=85=A8=E9=93=BE=E8=B7=AF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- internal/runtime/events_subagent.go | 17 + internal/runtime/subagent_engine.go | 483 ++++++++++++++++++ internal/runtime/subagent_engine_test.go | 308 +++++++++++ internal/runtime/subagent_factory.go | 13 +- internal/runtime/subagent_run.go | 57 ++- internal/runtime/subagent_tool_executor.go | 216 ++++++++ .../runtime/subagent_tool_executor_test.go | 164 ++++++ internal/subagent/execution_context_test.go | 132 +++++ internal/subagent/factory.go | 45 +- internal/subagent/policy.go | 46 +- internal/subagent/types.go | 45 ++ internal/subagent/worker.go | 36 +- 12 files changed, 1534 insertions(+), 28 deletions(-) create mode 100644 internal/runtime/subagent_engine.go create mode 100644 internal/runtime/subagent_engine_test.go create mode 100644 internal/runtime/subagent_tool_executor.go create mode 100644 internal/runtime/subagent_tool_executor_test.go create mode 100644 internal/subagent/execution_context_test.go diff --git a/internal/runtime/events_subagent.go b/internal/runtime/events_subagent.go index abe258b6..d962427d 100644 --- a/internal/runtime/events_subagent.go +++ b/internal/runtime/events_subagent.go @@ -19,6 +19,17 @@ type SubAgentEventPayload struct { Error string `json:"error,omitempty"` } +// SubAgentToolCallEventPayload 描述子代理工具调用事件载荷。 +type SubAgentToolCallEventPayload struct { + Role subagent.Role `json:"role"` + TaskID string `json:"task_id"` + ToolName string `json:"tool_name"` + Decision string `json:"decision"` + ElapsedMS int64 `json:"elapsed_ms"` + Truncated bool `json:"truncated"` + Error string `json:"error,omitempty"` +} + const ( // EventSubAgentStarted 在子代理任务启动后触发。 EventSubAgentStarted EventType = "subagent_started" @@ -32,4 +43,10 @@ const ( EventSubAgentFailed EventType = "subagent_failed" // EventSubAgentCanceled 在子代理被取消后触发。 EventSubAgentCanceled EventType = "subagent_canceled" + // EventSubAgentToolCallStarted 在子代理发起工具调用时触发。 + EventSubAgentToolCallStarted EventType = "subagent_tool_call_started" + // EventSubAgentToolCallResult 在子代理工具调用返回后触发。 + EventSubAgentToolCallResult EventType = "subagent_tool_call_result" + // EventSubAgentToolCallDenied 在子代理工具调用被权限拒绝时触发。 + EventSubAgentToolCallDenied EventType = "subagent_tool_call_denied" ) diff --git a/internal/runtime/subagent_engine.go b/internal/runtime/subagent_engine.go new file mode 100644 index 00000000..8f238f5d --- /dev/null +++ b/internal/runtime/subagent_engine.go @@ -0,0 +1,483 @@ +package runtime + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "strings" + "time" + + "neo-code/internal/config" + "neo-code/internal/partsrender" + "neo-code/internal/provider/streaming" + providertypes "neo-code/internal/provider/types" + "neo-code/internal/subagent" + "neo-code/internal/tools" +) + +const ( + subAgentMaxStepTurnsDefault = 6 + subAgentMaxStepTurnsLimit = 12 +) + +type subAgentOutputJSON struct { + Summary string `json:"summary"` + Findings []string `json:"findings"` + Patches []string `json:"patches"` + Risks []string `json:"risks"` + NextActions []string `json:"next_actions"` + Artifacts []string `json:"artifacts"` +} + +// runtimeSubAgentEngine 提供基于 runtime provider + tools 的子代理执行引擎。 +type runtimeSubAgentEngine struct { + service *Service + role subagent.Role + policy subagent.RolePolicy +} + +// newRuntimeSubAgentEngineBuilder 创建绑定 Service 的子代理引擎构建器。 +func newRuntimeSubAgentEngineBuilder(service *Service) subagent.EngineBuilder { + return func(role subagent.Role, policy subagent.RolePolicy) subagent.Engine { + return runtimeSubAgentEngine{ + service: service, + role: role, + policy: policy, + } + } +} + +// RunStep 执行子代理单步推理，并在单步内完成工具调用闭环。 +func (e runtimeSubAgentEngine) RunStep(ctx context.Context, input subagent.StepInput) (subagent.StepOutput, error) { + if err := ctx.Err(); err != nil { + return subagent.StepOutput{}, err + } + + runtimeConfig, model, toolTimeout, err := e.resolveSettings() + if err != nil { + return fallbackSubAgentStep(input), nil + } + if input.Executor == nil { + return subagent.StepOutput{}, errors.New("runtime: subagent tool executor is nil") + } + + allowedTools := resolveAllowedTools(input) + toolSpecs, err := input.Executor.ListToolSpecs(ctx, subagent.ToolSpecListInput{ + SessionID: input.SessionID, + Role: input.Role, + AllowedTools: allowedTools, + }) + if err != nil { + return subagent.StepOutput{}, fmt.Errorf("runtime: list subagent tool specs: %w", err) + } + if input.Policy.ToolUseMode == subagent.ToolUseModeDisabled { + toolSpecs = nil + } + + systemPrompt := buildSubAgentSystemPrompt(input.Policy, allowedTools) + messages := buildSubAgentInitialMessages(input) + totalToolCalls := 0 + maxTurns := resolveSubAgentMaxTurns(input.Budget.MaxSteps) + + for turn := 1; turn <= maxTurns; turn++ { + outcome, err := e.generateStepMessage(ctx, runtimeConfig, model, systemPrompt, messages, toolSpecs) + if err != nil { + return subagent.StepOutput{}, err + } + assistant := outcome.message + if strings.TrimSpace(assistant.Role) == "" { + assistant.Role = providertypes.RoleAssistant + } + if !assistant.IsEmpty() { + messages = append(messages, assistant) + } + + if len(assistant.ToolCalls) == 0 { + if input.Policy.ToolUseMode == subagent.ToolUseModeRequired && totalToolCalls == 0 { + return subagent.StepOutput{}, errors.New("runtime: subagent policy requires at least one tool call") + } + output, err := parseSubAgentOutput(renderAssistantText(assistant)) + if err != nil { + return subagent.StepOutput{}, err + } + return subagent.StepOutput{ + Delta: fmt.Sprintf("subagent completed with %d tool call(s)", totalToolCalls), + Done: true, + Output: output, + }, nil + } + + if input.Policy.ToolUseMode == subagent.ToolUseModeDisabled { + return subagent.StepOutput{}, errors.New("runtime: subagent tool_use_mode is disabled but model requested tool calls") + } + + remainingCalls := effectiveMaxToolCallsPerStep(input.Policy.MaxToolCallsPerStep) - totalToolCalls + if remainingCalls <= 0 { + return subagent.StepOutput{}, errors.New("runtime: subagent max_tool_calls_per_step exceeded") + } + if len(assistant.ToolCalls) > remainingCalls { + return subagent.StepOutput{}, errors.New("runtime: subagent tool call batch exceeds remaining budget") + } + + nextMessages, executedCalls, err := executeSubAgentToolCallBatch(ctx, e.service, input, assistant.ToolCalls, toolTimeout) + if err != nil { + return subagent.StepOutput{}, err + } + messages = append(messages, nextMessages...) + totalToolCalls += executedCalls + } + return subagent.StepOutput{}, fmt.Errorf("runtime: subagent step exceeded max turns (%d)", maxTurns) +} + +// resolveSettings 读取当前 runtime 配置，并解析子代理调用 provider 所需参数。 +func (e runtimeSubAgentEngine) resolveSettings() (config.ResolvedProviderConfig, string, time.Duration, error) { + if e.service == nil || e.service.configManager == nil { + return config.ResolvedProviderConfig{}, "", 0, errors.New("runtime: subagent service or config manager is nil") + } + if e.service.providerFactory == nil { + return config.ResolvedProviderConfig{}, "", 0, errors.New("runtime: subagent provider factory is nil") + } + cfg := e.service.configManager.Get() + resolvedProvider, err := config.ResolveSelectedProvider(cfg) + if err != nil { + return config.ResolvedProviderConfig{}, "", 0, fmt.Errorf("runtime: resolve selected provider: %w", err) + } + model := strings.TrimSpace(cfg.CurrentModel) + if model == "" { + model = strings.TrimSpace(resolvedProvider.Model) + } + if model == "" { + return config.ResolvedProviderConfig{}, "", 0, errors.New("runtime: subagent model is empty") + } + timeout := time.Duration(cfg.ToolTimeoutSec) * time.Second + if timeout <= 0 { + timeout = defaultSubAgentToolTimeout + } + return resolvedProvider, model, timeout, nil +} + +// generateStepMessage 发起一次 provider 调用并返回本轮 assistant 输出。 +func (e runtimeSubAgentEngine) generateStepMessage( + ctx context.Context, + resolvedProvider config.ResolvedProviderConfig, + model string, + systemPrompt string, + messages []providertypes.Message, + toolSpecs []providertypes.ToolSpec, +) (streamGenerateResult, error) { + modelProvider, err := e.service.providerFactory.Build(ctx, resolvedProvider.ToRuntimeConfig()) + if err != nil { + return streamGenerateResult{}, fmt.Errorf("runtime: build subagent provider: %w", err) + } + outcome := generateStreamingMessage(ctx, modelProvider, providertypes.GenerateRequest{ + Model: model, + SystemPrompt: systemPrompt, + Messages: messages, + Tools: toolSpecs, + }, streamingHooksForSubAgent()) + if outcome.err != nil { + return streamGenerateResult{}, fmt.Errorf("runtime: subagent provider generate: %w", outcome.err) + } + return outcome, nil +} + +// executeSubAgentToolCallBatch 顺序执行本轮工具调用并转换为后续模型输入消息。 +func executeSubAgentToolCallBatch( + ctx context.Context, + service *Service, + stepInput subagent.StepInput, + calls []providertypes.ToolCall, + toolTimeout time.Duration, +) ([]providertypes.Message, int, error) { + if len(calls) == 0 { + return nil, 0, nil + } + allowedTools := normalizeToolAllowlist(resolveAllowedTools(stepInput)) + results := make([]providertypes.Message, 0, len(calls)) + executed := 0 + + for index, call := range calls { + if err := ctx.Err(); err != nil { + return results, executed, err + } + normalizedCall := normalizeSubAgentToolCall(call, index) + if !toolAllowed(allowedTools, normalizedCall.Name) { + denied := subagentCapabilityDeniedResult(stepInput, normalizedCall) + results = append(results, denied) + emitCapabilityDeniedEvent(service, stepInput, normalizedCall.Name) + executed++ + continue + } + + execResult, execErr := stepInput.Executor.ExecuteTool(ctx, subagent.ToolExecutionInput{ + RunID: stepInput.RunID, + SessionID: stepInput.SessionID, + TaskID: stepInput.Task.ID, + Role: stepInput.Role, + AgentID: stepInput.AgentID, + Workdir: stepInput.Workdir, + Timeout: toolTimeout, + Call: normalizedCall, + }) + message := subAgentToolResultToMessage(normalizedCall, execResult) + if execErr != nil && strings.TrimSpace(message.Parts[0].Text) == "" { + message.Parts[0] = providertypes.NewTextPart(strings.TrimSpace(execErr.Error())) + } + results = append(results, message) + executed++ + } + return results, executed, nil +} + +// buildSubAgentInitialMessages 组装子代理首轮输入消息，包含任务、上下文切片与历史 trace。 +func buildSubAgentInitialMessages(input subagent.StepInput) []providertypes.Message { + lines := []string{ + "task_id: " + strings.TrimSpace(input.Task.ID), + "goal: " + strings.TrimSpace(input.Task.Goal), + } + if expected := strings.TrimSpace(input.Task.ExpectedOutput); expected != "" { + lines = append(lines, "expected_output: "+expected) + } + if workdir := strings.TrimSpace(input.Workdir); workdir != "" { + lines = append(lines, "workdir: "+workdir) + } + if renderedSlice := strings.TrimSpace(input.Task.ContextSlice.Render()); renderedSlice != "" { + lines = append(lines, "", "context_slice:", renderedSlice) + } + if len(input.Trace) > 0 { + lines = append(lines, "", "recent_trace:") + for _, item := range input.Trace { + trimmed := strings.TrimSpace(item) + if trimmed == "" { + continue + } + lines = append(lines, "- "+trimmed) + } + } + return []providertypes.Message{{ + Role: providertypes.RoleUser, + Parts: []providertypes.ContentPart{providertypes.NewTextPart(strings.Join(lines, "\n"))}, + }} +} + +// buildSubAgentSystemPrompt 构建子代理策略提示词，约束工具决策和输出契约。 +func buildSubAgentSystemPrompt(policy subagent.RolePolicy, allowedTools []string) string { + lines := []string{strings.TrimSpace(policy.SystemPrompt)} + lines = append(lines, + "你是子代理执行引擎的一部分，必须根据任务目标自主决定是否调用工具。", + "当需要外部事实、文件状态或命令执行结果时必须调用工具；纯推理可直接完成。", + "工具失败后优先换参数或换工具，若仍失败则在输出中明确风险与后续动作。", + "最终输出必须是 JSON 对象，且必须包含键：summary, findings, patches, risks, next_actions, artifacts。", + fmt.Sprintf("tool_use_mode: %s", policy.ToolUseMode), + fmt.Sprintf("max_tool_calls_per_step: %d", policy.MaxToolCallsPerStep), + ) + if len(allowedTools) > 0 { + lines = append(lines, "allowed_tools: "+strings.Join(allowedTools, ", ")) + } + return strings.TrimSpace(strings.Join(lines, "\n")) +} + +// resolveAllowedTools 计算本步可用工具集合，优先使用 capability，回退到 policy。 +func resolveAllowedTools(input subagent.StepInput) []string { + if len(input.Capability.AllowedTools) > 0 { + return append([]string(nil), input.Capability.AllowedTools...) + } + return append([]string(nil), input.Policy.AllowedTools...) +} + +// resolveSubAgentMaxTurns 统一解析子代理单步内部最多可迭代的模型轮次。 +func resolveSubAgentMaxTurns(maxSteps int) int { + if maxSteps <= 0 { + return subAgentMaxStepTurnsDefault + } + if maxSteps > subAgentMaxStepTurnsLimit { + return subAgentMaxStepTurnsLimit + } + return maxSteps +} + +// effectiveMaxToolCallsPerStep 解析每步工具调用上限，并为零值回填默认值。 +func effectiveMaxToolCallsPerStep(limit int) int { + if limit <= 0 { + return subAgentMaxStepTurnsDefault + } + return limit +} + +// parseSubAgentOutput 从 assistant 文本中提取并解析结构化 JSON 输出。 +func parseSubAgentOutput(text string) (subagent.Output, error) { + jsonText, err := extractSubAgentJSONObject(text) + if err != nil { + return subagent.Output{}, err + } + var payload subAgentOutputJSON + if err := json.Unmarshal([]byte(jsonText), &payload); err != nil { + return subagent.Output{}, fmt.Errorf("runtime: parse subagent output json: %w", err) + } + return subagent.Output{ + Summary: strings.TrimSpace(payload.Summary), + Findings: payload.Findings, + Patches: payload.Patches, + Risks: payload.Risks, + NextActions: payload.NextActions, + Artifacts: payload.Artifacts, + }, nil +} + +// extractSubAgentJSONObject 从文本中提取首个完整 JSON 对象，容忍前后噪声。 +func extractSubAgentJSONObject(text string) (string, error) { + start := strings.Index(text, "{") + if start < 0 { + return "", errors.New("runtime: subagent output does not contain json object") + } + depth := 0 + inString := false + escaped := false + for index := start; index < len(text); index++ { + ch := text[index] + if inString { + if escaped { + escaped = false + continue + } + if ch == '\\' { + escaped = true + continue + } + if ch == '"' { + inString = false + } + continue + } + switch ch { + case '"': + inString = true + case '{': + depth++ + case '}': + depth-- + if depth == 0 { + return strings.TrimSpace(text[start : index+1]), nil + } + } + } + return "", errors.New("runtime: subagent output contains incomplete json object") +} + +// renderAssistantText 将 assistant parts 渲染为统一文本，用于 JSON 输出解析。 +func renderAssistantText(message providertypes.Message) string { + return strings.TrimSpace(partsrender.RenderDisplayParts(message.Parts)) +} + +// normalizeSubAgentToolCall 规整工具调用基础字段，并补齐空 call id。 +func normalizeSubAgentToolCall(call providertypes.ToolCall, index int) providertypes.ToolCall { + normalized := call + normalized.Name = strings.TrimSpace(normalized.Name) + normalized.Arguments = strings.TrimSpace(normalized.Arguments) + normalized.ID = strings.TrimSpace(normalized.ID) + if normalized.ID == "" { + normalized.ID = fmt.Sprintf("subagent-call-%d", index+1) + } + return normalized +} + +// normalizeToolAllowlist 规整工具白名单，便于大小写无关匹配。 +func normalizeToolAllowlist(items []string) map[string]struct{} { + if len(items) == 0 { + return nil + } + set := make(map[string]struct{}, len(items)) + for _, item := range items { + name := strings.ToLower(strings.TrimSpace(item)) + if name == "" { + continue + } + set[name] = struct{}{} + } + return set +} + +// toolAllowed 判断工具名是否在 allowlist 内；allowlist 为空时默认放行。 +func toolAllowed(allowlist map[string]struct{}, toolName string) bool { + if len(allowlist) == 0 { + return true + } + _, ok := allowlist[strings.ToLower(strings.TrimSpace(toolName))] + return ok +} + +// subagentCapabilityDeniedResult 构造 capability 越权时回灌给模型的标准 tool 消息。 +func subagentCapabilityDeniedResult(input subagent.StepInput, call providertypes.ToolCall) providertypes.Message { + content := tools.FormatError(call.Name, "capability denied", "tool is not allowed in current subagent capability") + return providertypes.Message{ + Role: providertypes.RoleTool, + ToolCallID: call.ID, + Parts: []providertypes.ContentPart{providertypes.NewTextPart(content)}, + IsError: true, + ToolMetadata: map[string]string{"tool_name": call.Name, "decision": permissionDecisionDeny, "task_id": input.Task.ID}, + } +} + +// emitCapabilityDeniedEvent 发射 capability 越权拒绝事件。 +func emitCapabilityDeniedEvent(service *Service, input subagent.StepInput, toolName string) { + if service == nil { + return + } + _ = service.emit(context.Background(), EventSubAgentToolCallDenied, input.RunID, input.SessionID, SubAgentToolCallEventPayload{ + Role: input.Role, + TaskID: input.Task.ID, + ToolName: strings.TrimSpace(toolName), + Decision: permissionDecisionDeny, + ElapsedMS: 0, + Truncated: false, + Error: "capability denied", + }) +} + +// subAgentToolResultToMessage 把工具执行结果转换为 provider 可消费的 tool 消息。 +func subAgentToolResultToMessage(call providertypes.ToolCall, result subagent.ToolExecutionResult) providertypes.Message { + name := strings.TrimSpace(result.Name) + if name == "" { + name = strings.TrimSpace(call.Name) + } + metadata := map[string]any{ + "tool_name": name, + "decision": strings.TrimSpace(result.Decision), + } + for key, value := range result.Metadata { + metadata[key] = value + } + return providertypes.Message{ + Role: providertypes.RoleTool, + ToolCallID: call.ID, + Parts: []providertypes.ContentPart{providertypes.NewTextPart(strings.TrimSpace(result.Content))}, + IsError: result.IsError, + ToolMetadata: tools.SanitizeToolMetadata(name, metadata), + } +} + +// fallbackSubAgentStep 在 runtime 依赖不可用时返回可验证的保底输出，避免子代理直接崩溃。 +func fallbackSubAgentStep(input subagent.StepInput) subagent.StepOutput { + summary := strings.TrimSpace(input.Task.ExpectedOutput) + if summary == "" { + summary = strings.TrimSpace(input.Task.Goal) + } + return subagent.StepOutput{ + Delta: "subagent fallback engine completed", + Done: true, + Output: subagent.Output{ + Summary: summary, + Findings: []string{"fallback-engine: runtime dependencies unavailable"}, + Patches: []string{"fallback-engine: no code changes"}, + Risks: []string{"fallback-engine: no provider/tool verification"}, + NextActions: []string{"initialize runtime config/provider to enable tool-call loop"}, + Artifacts: []string{"subagent-fallback"}, + }, + } +} + +// streamingHooksForSubAgent 返回子代理生成阶段使用的默认流式钩子。 +func streamingHooksForSubAgent() streaming.Hooks { + return streaming.Hooks{} +} diff --git a/internal/runtime/subagent_engine_test.go b/internal/runtime/subagent_engine_test.go new file mode 100644 index 00000000..d98611d6 --- /dev/null +++ b/internal/runtime/subagent_engine_test.go @@ -0,0 +1,308 @@ +package runtime + +import ( + "context" + "strings" + "testing" + + providertypes "neo-code/internal/provider/types" + "neo-code/internal/subagent" + "neo-code/internal/tools" +) + +func TestRuntimeSubAgentEngineRunStepToolLoopSuccess(t *testing.T) { + t.Parallel() + + manager := newRuntimeConfigManager(t) + providerImpl := &scriptedProvider{ + responses: []scriptedResponse{ + { + Message: providertypes.Message{ + Role: providertypes.RoleAssistant, + ToolCalls: []providertypes.ToolCall{ + {ID: "call-1", Name: "filesystem_read_file", Arguments: `{"path":"README.md"}`}, + }, + }, + FinishReason: "tool_calls", + }, + { + Message: providertypes.Message{ + Role: providertypes.RoleAssistant, + Parts: []providertypes.ContentPart{ + providertypes.NewTextPart(`{"summary":"done","findings":["f1"],"patches":["p1"],"risks":["r1"],"next_actions":["n1"],"artifacts":["a1"]}`), + }, + }, + FinishReason: "stop", + }, + }, + } + toolManager := &stubToolManager{ + specs: []providertypes.ToolSpec{{Name: "filesystem_read_file", Schema: map[string]any{"type": "object"}}}, + executeFn: func(ctx context.Context, input tools.ToolCallInput) (tools.ToolResult, error) { + return tools.ToolResult{ + ToolCallID: input.ID, + Name: input.Name, + Content: "file content", + IsError: false, + Metadata: map[string]any{"truncated": true}, + }, nil + }, + } + service := NewWithFactory(manager, toolManager, newMemoryStore(), &scriptedProviderFactory{provider: providerImpl}, nil) + policy, err := subagent.DefaultRolePolicy(subagent.RoleCoder) + if err != nil { + t.Fatalf("DefaultRolePolicy() error = %v", err) + } + policy.ToolUseMode = subagent.ToolUseModeAuto + policy.MaxToolCallsPerStep = 2 + engine := runtimeSubAgentEngine{service: service, role: subagent.RoleCoder, policy: policy} + + output, err := engine.RunStep(context.Background(), subagent.StepInput{ + Role: subagent.RoleCoder, + Policy: policy, + Task: subagent.Task{ID: "task-1", Goal: "read file and summarize"}, + Budget: subagent.Budget{MaxSteps: 4}, + Capability: subagent.Capability{ + AllowedTools: []string{"filesystem_read_file"}, + }, + RunID: "run-subagent-step-success", + SessionID: "session-subagent-step-success", + AgentID: "subagent:task-1", + Workdir: t.TempDir(), + Executor: newSubAgentRuntimeToolExecutor(service), + }) + if err != nil { + t.Fatalf("RunStep() error = %v", err) + } + if !output.Done { + t.Fatalf("expected step done") + } + if output.Output.Summary != "done" { + t.Fatalf("summary = %q, want %q", output.Output.Summary, "done") + } + if toolManager.executeCalls != 1 { + t.Fatalf("tool execute calls = %d, want 1", toolManager.executeCalls) + } + if providerImpl.callCount != 2 { + t.Fatalf("provider calls = %d, want 2", providerImpl.callCount) + } + + events := collectRuntimeEvents(service.Events()) + assertEventSequence(t, events, []EventType{EventSubAgentToolCallStarted, EventSubAgentToolCallResult}) + assertSubAgentToolEventPayload(t, events, EventSubAgentToolCallResult, "filesystem_read_file", permissionDecisionAllow, true) +} + +func TestRuntimeSubAgentEngineRunStepCapabilityDenied(t *testing.T) { + t.Parallel() + + manager := newRuntimeConfigManager(t) + providerImpl := &scriptedProvider{ + responses: []scriptedResponse{ + { + Message: providertypes.Message{ + Role: providertypes.RoleAssistant, + ToolCalls: []providertypes.ToolCall{ + {ID: "call-bash", Name: "bash", Arguments: `{"command":"echo hi"}`}, + }, + }, + FinishReason: "tool_calls", + }, + { + Message: providertypes.Message{ + Role: providertypes.RoleAssistant, + Parts: []providertypes.ContentPart{ + providertypes.NewTextPart(`{"summary":"fallback","findings":["f1"],"patches":["p1"],"risks":["r1"],"next_actions":["n1"],"artifacts":["a1"]}`), + }, + }, + FinishReason: "stop", + }, + }, + } + toolManager := &stubToolManager{ + specs: []providertypes.ToolSpec{{Name: "bash", Schema: map[string]any{"type": "object"}}}, + } + service := NewWithFactory(manager, toolManager, newMemoryStore(), &scriptedProviderFactory{provider: providerImpl}, nil) + policy, err := subagent.DefaultRolePolicy(subagent.RoleCoder) + if err != nil { + t.Fatalf("DefaultRolePolicy() error = %v", err) + } + policy.MaxToolCallsPerStep = 2 + engine := runtimeSubAgentEngine{service: service, role: subagent.RoleCoder, policy: policy} + + stepOutput, err := engine.RunStep(context.Background(), subagent.StepInput{ + Role: subagent.RoleCoder, + Policy: policy, + Task: subagent.Task{ID: "task-cap-deny", Goal: "execute bash"}, + Budget: subagent.Budget{MaxSteps: 4}, + Capability: subagent.Capability{ + AllowedTools: []string{"filesystem_read_file"}, + }, + RunID: "run-subagent-cap-deny", + SessionID: "session-subagent-cap-deny", + AgentID: "subagent:task-cap-deny", + Workdir: t.TempDir(), + Executor: newSubAgentRuntimeToolExecutor(service), + }) + if err != nil { + t.Fatalf("RunStep() error = %v", err) + } + if !stepOutput.Done { + t.Fatalf("expected step done") + } + if toolManager.executeCalls != 0 { + t.Fatalf("tool execute calls = %d, want 0", toolManager.executeCalls) + } + + events := collectRuntimeEvents(service.Events()) + assertEventSequence(t, events, []EventType{EventSubAgentToolCallDenied}) + assertSubAgentToolEventPayload(t, events, EventSubAgentToolCallDenied, "bash", permissionDecisionDeny, false) +} + +func TestRuntimeSubAgentEngineRunStepRequiredModeWithoutToolFails(t *testing.T) { + t.Parallel() + + manager := newRuntimeConfigManager(t) + providerImpl := &scriptedProvider{ + responses: []scriptedResponse{ + { + Message: providertypes.Message{ + Role: providertypes.RoleAssistant, + Parts: []providertypes.ContentPart{ + providertypes.NewTextPart(`{"summary":"done","findings":["f1"],"patches":["p1"],"risks":["r1"],"next_actions":["n1"],"artifacts":["a1"]}`), + }, + }, + FinishReason: "stop", + }, + }, + } + toolManager := &stubToolManager{ + specs: []providertypes.ToolSpec{{Name: "filesystem_read_file", Schema: map[string]any{"type": "object"}}}, + } + service := NewWithFactory(manager, toolManager, newMemoryStore(), &scriptedProviderFactory{provider: providerImpl}, nil) + policy, err := subagent.DefaultRolePolicy(subagent.RoleCoder) + if err != nil { + t.Fatalf("DefaultRolePolicy() error = %v", err) + } + policy.ToolUseMode = subagent.ToolUseModeRequired + policy.MaxToolCallsPerStep = 1 + engine := runtimeSubAgentEngine{service: service, role: subagent.RoleCoder, policy: policy} + + _, err = engine.RunStep(context.Background(), subagent.StepInput{ + Role: subagent.RoleCoder, + Policy: policy, + Task: subagent.Task{ID: "task-required", Goal: "must call tool"}, + Budget: subagent.Budget{MaxSteps: 2}, + Capability: subagent.Capability{AllowedTools: []string{"filesystem_read_file"}}, + RunID: "run-subagent-required", + SessionID: "session-subagent-required", + AgentID: "subagent:task-required", + Workdir: t.TempDir(), + Executor: newSubAgentRuntimeToolExecutor(service), + }) + if err == nil || !strings.Contains(err.Error(), "requires at least one tool call") { + t.Fatalf("expected required-mode error, got %v", err) + } +} + +func TestRuntimeSubAgentEngineFallbackWhenRuntimeUnavailable(t *testing.T) { + t.Parallel() + + policy, err := subagent.DefaultRolePolicy(subagent.RoleReviewer) + if err != nil { + t.Fatalf("DefaultRolePolicy() error = %v", err) + } + engine := runtimeSubAgentEngine{ + service: nil, + role: subagent.RoleReviewer, + policy: policy, + } + step, err := engine.RunStep(context.Background(), subagent.StepInput{ + Role: subagent.RoleReviewer, + Policy: policy, + Task: subagent.Task{ID: "task-fallback", Goal: "review"}, + }) + if err != nil { + t.Fatalf("RunStep() error = %v", err) + } + if !step.Done { + t.Fatalf("expected fallback step done") + } + if step.Output.Summary != "review" { + t.Fatalf("summary = %q, want %q", step.Output.Summary, "review") + } +} + +func TestParseSubAgentOutput(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input string + wantErr bool + }{ + { + name: "plain json", + input: `{"summary":"s","findings":["f"],"patches":["p"],"risks":["r"],"next_actions":["n"],"artifacts":["a"]}`, + }, + { + name: "json with prefix and suffix", + input: "result:\n```json\n{\"summary\":\"s\",\"findings\":[\"f\"],\"patches\":[\"p\"],\"risks\":[\"r\"],\"next_actions\":[\"n\"],\"artifacts\":[\"a\"]}\n```\nend", + }, + { + name: "invalid json", + input: `summary only`, + wantErr: true, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := parseSubAgentOutput(tt.input) + if tt.wantErr { + if err == nil { + t.Fatalf("expected error") + } + return + } + if err != nil { + t.Fatalf("parseSubAgentOutput() error = %v", err) + } + if got.Summary != "s" { + t.Fatalf("summary = %q, want %q", got.Summary, "s") + } + }) + } +} + +func assertSubAgentToolEventPayload( + t *testing.T, + events []RuntimeEvent, + eventType EventType, + toolName string, + decision string, + truncated bool, +) { + t.Helper() + for _, event := range events { + if event.Type != eventType { + continue + } + payload, ok := event.Payload.(SubAgentToolCallEventPayload) + if !ok { + t.Fatalf("payload type = %T, want SubAgentToolCallEventPayload", event.Payload) + } + if payload.ToolName != toolName { + t.Fatalf("tool_name = %q, want %q", payload.ToolName, toolName) + } + if payload.Decision != decision { + t.Fatalf("decision = %q, want %q", payload.Decision, decision) + } + if payload.Truncated != truncated { + t.Fatalf("truncated = %v, want %v", payload.Truncated, truncated) + } + return + } + t.Fatalf("event %q not found", eventType) +} diff --git a/internal/runtime/subagent_factory.go b/internal/runtime/subagent_factory.go index 68662d0b..9bf2ef24 100644 --- a/internal/runtime/subagent_factory.go +++ b/internal/runtime/subagent_factory.go @@ -66,8 +66,11 @@ func (r *subAgentFactoryRegistry) get(s *Service) (subagent.Factory, bool) { } // defaultSubAgentFactory 返回默认的子代理工厂实例。 -func defaultSubAgentFactory() subagent.Factory { - return subagent.NewWorkerFactory(nil) +func defaultSubAgentFactory(service *Service) subagent.Factory { + return subagent.NewWorkerFactory( + newRuntimeSubAgentEngineBuilder(service), + subagent.WithToolExecutor(newSubAgentRuntimeToolExecutor(service)), + ) } // SetSubAgentFactory 设置子代理运行时工厂；传入 nil 时回退到默认工厂。 @@ -76,7 +79,7 @@ func (s *Service) SetSubAgentFactory(factory subagent.Factory) { return } if factory == nil { - globalSubAgentFactories.set(s, defaultSubAgentFactory()) + globalSubAgentFactories.set(s, defaultSubAgentFactory(s)) return } globalSubAgentFactories.set(s, factory) @@ -85,13 +88,13 @@ func (s *Service) SetSubAgentFactory(factory subagent.Factory) { // SubAgentFactory 返回当前 runtime 持有的子代理运行时工厂。 func (s *Service) SubAgentFactory() subagent.Factory { if s == nil { - return defaultSubAgentFactory() + return defaultSubAgentFactory(nil) } if factory, ok := globalSubAgentFactories.get(s); ok && factory != nil { return factory } - defaultFactory := defaultSubAgentFactory() + defaultFactory := defaultSubAgentFactory(s) globalSubAgentFactories.set(s, defaultFactory) return defaultFactory } diff --git a/internal/runtime/subagent_run.go b/internal/runtime/subagent_run.go index b33c3bc5..a38efad3 100644 --- a/internal/runtime/subagent_run.go +++ b/internal/runtime/subagent_run.go @@ -15,6 +15,7 @@ import ( type SubAgentTaskInput struct { RunID string SessionID string + AgentID string Role subagent.Role Task subagent.Task Budget subagent.Budget @@ -35,6 +36,14 @@ func (s *Service) RunSubAgentTask(ctx context.Context, input SubAgentTaskInput) if err := input.Task.Validate(); err != nil { return subagent.Result{}, err } + task := input.Task + task.RunID = strings.TrimSpace(input.RunID) + task.SessionID = strings.TrimSpace(input.SessionID) + task.AgentID = resolveSubAgentExecutionAgentID(input) + if strings.TrimSpace(task.Workspace) == "" && s != nil && s.configManager != nil { + cfg := s.configManager.Get() + task.Workspace = strings.TrimSpace(cfg.Workdir) + } factory := s.SubAgentFactory() worker, err := factory.Create(input.Role) @@ -48,7 +57,7 @@ func (s *Service) RunSubAgentTask(ctx context.Context, input SubAgentTaskInput) return subagent.Result{}, err } - if err := worker.Start(input.Task, input.Budget, input.Capability); err != nil { + if err := worker.Start(task, input.Budget, input.Capability); err != nil { _ = s.emit(ctx, EventSubAgentFailed, input.RunID, input.SessionID, SubAgentEventPayload{ Role: input.Role, TaskID: input.Task.ID, @@ -60,7 +69,7 @@ func (s *Service) RunSubAgentTask(ctx context.Context, input SubAgentTaskInput) _ = s.emit(ctx, EventSubAgentStarted, input.RunID, input.SessionID, SubAgentEventPayload{ Role: input.Role, - TaskID: input.Task.ID, + TaskID: task.ID, State: worker.State(), }) @@ -69,7 +78,7 @@ func (s *Service) RunSubAgentTask(ctx context.Context, input SubAgentTaskInput) if stepResult.State == "" { stepResult.State = worker.State() } - emitSubAgentProgress(s, input, stepResult, stepErr) + emitSubAgentProgress(s, input.RunID, input.SessionID, input.Role, task.ID, stepResult, stepErr) if stepErr != nil { if errors.Is(stepErr, context.DeadlineExceeded) { @@ -78,7 +87,7 @@ func (s *Service) RunSubAgentTask(ctx context.Context, input SubAgentTaskInput) if resultErr != nil { result = subagent.Result{ Role: input.Role, - TaskID: input.Task.ID, + TaskID: task.ID, State: subagent.StateFailed, StopReason: subagent.StopReasonTimeout, Error: errorText(stepErr), @@ -93,7 +102,7 @@ func (s *Service) RunSubAgentTask(ctx context.Context, input SubAgentTaskInput) if resultErr != nil { result = subagent.Result{ Role: input.Role, - TaskID: input.Task.ID, + TaskID: task.ID, State: subagent.StateCanceled, StopReason: subagent.StopReasonCanceled, Error: errorText(stepErr), @@ -107,7 +116,7 @@ func (s *Service) RunSubAgentTask(ctx context.Context, input SubAgentTaskInput) if resultErr != nil { _ = s.emit(ctx, EventSubAgentFailed, input.RunID, input.SessionID, SubAgentEventPayload{ Role: input.Role, - TaskID: input.Task.ID, + TaskID: task.ID, State: subagent.StateFailed, Error: stepErr.Error(), }) @@ -125,7 +134,7 @@ func (s *Service) RunSubAgentTask(ctx context.Context, input SubAgentTaskInput) if err != nil { _ = s.emit(ctx, EventSubAgentFailed, input.RunID, input.SessionID, SubAgentEventPayload{ Role: input.Role, - TaskID: input.Task.ID, + TaskID: task.ID, State: subagent.StateFailed, Error: err.Error(), }) @@ -140,10 +149,18 @@ func (s *Service) RunSubAgentTask(ctx context.Context, input SubAgentTaskInput) } // emitSubAgentProgress 非阻塞发射进度事件，避免慢消费者反压执行路径。 -func emitSubAgentProgress(s *Service, input SubAgentTaskInput, stepResult subagent.StepResult, stepErr error) { +func emitSubAgentProgress( + s *Service, + runID string, + sessionID string, + role subagent.Role, + taskID string, + stepResult subagent.StepResult, + stepErr error, +) { payload := SubAgentEventPayload{ - Role: input.Role, - TaskID: input.Task.ID, + Role: role, + TaskID: strings.TrimSpace(taskID), State: stepResult.State, Step: stepResult.Step, Delta: stepResult.Delta, @@ -151,8 +168,8 @@ func emitSubAgentProgress(s *Service, input SubAgentTaskInput, stepResult subage } event := RuntimeEvent{ Type: EventSubAgentProgress, - RunID: input.RunID, - SessionID: input.SessionID, + RunID: strings.TrimSpace(runID), + SessionID: strings.TrimSpace(sessionID), Turn: turnUnspecified, Timestamp: time.Now(), PayloadVersion: controlplane.PayloadVersion, @@ -200,3 +217,19 @@ func subAgentResultError(result subagent.Result) error { } return fmt.Errorf("subagent ended with state=%s stop_reason=%s", result.State, result.StopReason) } + +// resolveSubAgentExecutionAgentID 生成子代理执行身份，供权限链路透传审计。 +func resolveSubAgentExecutionAgentID(input SubAgentTaskInput) string { + if agentID := strings.TrimSpace(input.AgentID); agentID != "" { + return agentID + } + role := strings.TrimSpace(string(input.Role)) + taskID := strings.TrimSpace(input.Task.ID) + if role == "" { + role = "subagent" + } + if taskID == "" { + return role + } + return role + ":" + taskID +} diff --git a/internal/runtime/subagent_tool_executor.go b/internal/runtime/subagent_tool_executor.go new file mode 100644 index 00000000..961fc223 --- /dev/null +++ b/internal/runtime/subagent_tool_executor.go @@ -0,0 +1,216 @@ +package runtime + +import ( + "context" + "errors" + "strings" + "time" + + providertypes "neo-code/internal/provider/types" + "neo-code/internal/subagent" + "neo-code/internal/tools" +) + +const subAgentToolDecisionPending = "pending" +const stringPermissionDecisionAsk = "ask" +const defaultSubAgentToolTimeout = 20 * time.Second + +// subAgentRuntimeToolExecutor 将 subagent 工具调用桥接到 runtime 的统一执行链路。 +type subAgentRuntimeToolExecutor struct { + service *Service +} + +// newSubAgentRuntimeToolExecutor 创建子代理工具桥接器。 +func newSubAgentRuntimeToolExecutor(service *Service) subagent.ToolExecutor { + return &subAgentRuntimeToolExecutor{service: service} +} + +// ListToolSpecs 返回子代理在当前会话可见的工具 schema，并按 allowlist 再过滤一层。 +func (e *subAgentRuntimeToolExecutor) ListToolSpecs( + ctx context.Context, + input subagent.ToolSpecListInput, +) ([]providertypes.ToolSpec, error) { + if e == nil || e.service == nil || e.service.toolManager == nil { + return nil, errors.New("runtime: subagent tool executor is unavailable") + } + specs, err := e.service.toolManager.ListAvailableSpecs(ctx, tools.SpecListInput{ + SessionID: strings.TrimSpace(input.SessionID), + Agent: strings.TrimSpace(string(input.Role)), + }) + if err != nil { + return nil, err + } + return filterToolSpecsByAllowlist(specs, input.AllowedTools), nil +} + +// ExecuteTool 执行一次子代理工具调用，并补齐 started/result/denied 事件。 +func (e *subAgentRuntimeToolExecutor) ExecuteTool( + ctx context.Context, + input subagent.ToolExecutionInput, +) (subagent.ToolExecutionResult, error) { + if e == nil || e.service == nil { + return subagent.ToolExecutionResult{}, errors.New("runtime: subagent tool executor is unavailable") + } + startedAt := time.Now() + timeout := input.Timeout + if timeout <= 0 { + timeout = defaultSubAgentToolTimeout + } + + payload := SubAgentToolCallEventPayload{ + Role: input.Role, + TaskID: strings.TrimSpace(input.TaskID), + ToolName: strings.TrimSpace(input.Call.Name), + Decision: subAgentToolDecisionPending, + ElapsedMS: 0, + } + e.emit(input.RunID, input.SessionID, EventSubAgentToolCallStarted, payload) + + result, execErr := e.service.executeToolCallWithPermission(ctx, permissionExecutionInput{ + RunID: strings.TrimSpace(input.RunID), + SessionID: strings.TrimSpace(input.SessionID), + TaskID: strings.TrimSpace(input.TaskID), + AgentID: strings.TrimSpace(input.AgentID), + Call: input.Call, + Workdir: strings.TrimSpace(input.Workdir), + ToolTimeout: timeout, + }) + + output := subagent.ToolExecutionResult{ + ToolCallID: input.Call.ID, + Name: strings.TrimSpace(input.Call.Name), + Content: result.Content, + IsError: result.IsError, + Decision: permissionDecisionAllow, + Metadata: cloneToolMetadata(result.Metadata), + } + if strings.TrimSpace(result.ToolCallID) != "" { + output.ToolCallID = strings.TrimSpace(result.ToolCallID) + } + if strings.TrimSpace(result.Name) != "" { + output.Name = strings.TrimSpace(result.Name) + } + + decision := permissionDecisionAllow + if execErr != nil { + var permissionErr *tools.PermissionDecisionError + if errors.As(execErr, &permissionErr) { + decision = permissionErr.Decision() + } else { + decision = "error" + } + output.Decision = decision + if strings.TrimSpace(output.Content) == "" { + output.Content = strings.TrimSpace(execErr.Error()) + } + output.IsError = true + } + + eventPayload := SubAgentToolCallEventPayload{ + Role: input.Role, + TaskID: strings.TrimSpace(input.TaskID), + ToolName: output.Name, + Decision: decision, + ElapsedMS: elapsedMilliseconds(startedAt), + Truncated: toolResultTruncated(output.Metadata), + } + if execErr != nil { + eventPayload.Error = strings.TrimSpace(execErr.Error()) + } + + eventType := EventSubAgentToolCallResult + if strings.EqualFold(decision, permissionDecisionDeny) || strings.EqualFold(decision, stringPermissionDecisionAsk) { + eventType = EventSubAgentToolCallDenied + } + e.emit(input.RunID, input.SessionID, eventType, eventPayload) + return output, execErr +} + +// emit 发出子代理工具调用事件，失败路径按 best-effort 忽略。 +func (e *subAgentRuntimeToolExecutor) emit(runID string, sessionID string, eventType EventType, payload SubAgentToolCallEventPayload) { + if e == nil || e.service == nil { + return + } + _ = e.service.emit(context.Background(), eventType, strings.TrimSpace(runID), strings.TrimSpace(sessionID), payload) +} + +// filterToolSpecsByAllowlist 按工具名白名单过滤 schema 列表，白名单为空时返回全量。 +func filterToolSpecsByAllowlist(specs []providertypes.ToolSpec, allowlist []string) []providertypes.ToolSpec { + if len(specs) == 0 { + return nil + } + normalizedAllowlist := normalizeAllowlist(allowlist) + if len(normalizedAllowlist) == 0 { + result := make([]providertypes.ToolSpec, len(specs)) + copy(result, specs) + return result + } + filtered := make([]providertypes.ToolSpec, 0, len(specs)) + for _, spec := range specs { + name := strings.ToLower(strings.TrimSpace(spec.Name)) + if _, ok := normalizedAllowlist[name]; !ok { + continue + } + filtered = append(filtered, spec) + } + return filtered +} + +// normalizeAllowlist 规整工具白名单，便于大小写无关匹配。 +func normalizeAllowlist(items []string) map[string]struct{} { + if len(items) == 0 { + return nil + } + result := make(map[string]struct{}, len(items)) + for _, item := range items { + normalized := strings.ToLower(strings.TrimSpace(item)) + if normalized == "" { + continue + } + result[normalized] = struct{}{} + } + return result +} + +// cloneToolMetadata 深拷贝工具元数据，避免后续修改污染事件载荷。 +func cloneToolMetadata(metadata map[string]any) map[string]any { + if len(metadata) == 0 { + return nil + } + cloned := make(map[string]any, len(metadata)) + for key, value := range metadata { + cloned[key] = value + } + return cloned +} + +// toolResultTruncated 从工具 metadata 提取截断标记。 +func toolResultTruncated(metadata map[string]any) bool { + if len(metadata) == 0 { + return false + } + value, ok := metadata["truncated"] + if !ok { + return false + } + switch typed := value.(type) { + case bool: + return typed + case string: + return strings.EqualFold(strings.TrimSpace(typed), "true") + default: + return false + } +} + +// elapsedMilliseconds 返回从起始时刻到当前的毫秒耗时。 +func elapsedMilliseconds(startedAt time.Time) int64 { + if startedAt.IsZero() { + return 0 + } + elapsed := time.Since(startedAt) + if elapsed < 0 { + return 0 + } + return elapsed.Milliseconds() +} diff --git a/internal/runtime/subagent_tool_executor_test.go b/internal/runtime/subagent_tool_executor_test.go new file mode 100644 index 00000000..416f1f23 --- /dev/null +++ b/internal/runtime/subagent_tool_executor_test.go @@ -0,0 +1,164 @@ +package runtime + +import ( + "context" + "errors" + "testing" + "time" + + providertypes "neo-code/internal/provider/types" + "neo-code/internal/security" + "neo-code/internal/subagent" + "neo-code/internal/tools" +) + +func TestSubAgentRuntimeToolExecutorListToolSpecs(t *testing.T) { + t.Parallel() + + service := NewWithFactory( + newRuntimeConfigManager(t), + &stubToolManager{ + specs: []providertypes.ToolSpec{ + {Name: "filesystem_read_file"}, + {Name: "bash"}, + }, + }, + newMemoryStore(), + &scriptedProviderFactory{provider: &scriptedProvider{}}, + nil, + ) + executor := newSubAgentRuntimeToolExecutor(service) + + tests := []struct { + name string + allow []string + wantSize int + }{ + {name: "no allowlist", allow: nil, wantSize: 2}, + {name: "single allowlist", allow: []string{"bash"}, wantSize: 1}, + {name: "case-insensitive allowlist", allow: []string{"FILESYSTEM_READ_FILE"}, wantSize: 1}, + {name: "empty allowlist behaves as full set", allow: []string{""}, wantSize: 2}, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + specs, err := executor.ListToolSpecs(context.Background(), subagent.ToolSpecListInput{ + SessionID: "session-list-tools", + Role: subagent.RoleCoder, + AllowedTools: tt.allow, + }) + if err != nil { + t.Fatalf("ListToolSpecs() error = %v", err) + } + if len(specs) != tt.wantSize { + t.Fatalf("len(specs) = %d, want %d", len(specs), tt.wantSize) + } + }) + } +} + +func TestSubAgentRuntimeToolExecutorExecuteToolEvents(t *testing.T) { + t.Parallel() + + t.Run("allow should emit started and result", func(t *testing.T) { + t.Parallel() + + toolManager := &stubToolManager{ + executeFn: func(ctx context.Context, input tools.ToolCallInput) (tools.ToolResult, error) { + return tools.ToolResult{ + ToolCallID: input.ID, + Name: input.Name, + Content: "ok", + Metadata: map[string]any{"truncated": true}, + }, nil + }, + } + service := NewWithFactory( + newRuntimeConfigManager(t), + toolManager, + newMemoryStore(), + &scriptedProviderFactory{provider: &scriptedProvider{}}, + nil, + ) + executor := newSubAgentRuntimeToolExecutor(service) + + result, err := executor.ExecuteTool(context.Background(), subagent.ToolExecutionInput{ + RunID: "run-subagent-tool-allow", + SessionID: "session-subagent-tool-allow", + TaskID: "task-subagent-tool-allow", + Role: subagent.RoleCoder, + AgentID: "subagent:allow", + Workdir: t.TempDir(), + Timeout: 2 * time.Second, + Call: providertypes.ToolCall{ + ID: "call-allow", + Name: "filesystem_read_file", + Arguments: `{"path":"README.md"}`, + }, + }) + if err != nil { + t.Fatalf("ExecuteTool() error = %v", err) + } + if result.Decision != permissionDecisionAllow { + t.Fatalf("decision = %q, want %q", result.Decision, permissionDecisionAllow) + } + + events := collectRuntimeEvents(service.Events()) + assertEventSequence(t, events, []EventType{EventSubAgentToolCallStarted, EventSubAgentToolCallResult}) + assertSubAgentToolEventPayload(t, events, EventSubAgentToolCallResult, "filesystem_read_file", permissionDecisionAllow, true) + }) + + t.Run("permission deny should emit denied", func(t *testing.T) { + t.Parallel() + + registry := tools.NewRegistry() + registry.Register(&stubTool{name: "bash", content: "ok"}) + gateway, err := security.NewStaticGateway(security.DecisionDeny, nil) + if err != nil { + t.Fatalf("NewStaticGateway() error = %v", err) + } + manager, err := tools.NewManager(registry, gateway, nil) + if err != nil { + t.Fatalf("NewManager() error = %v", err) + } + + service := NewWithFactory( + newRuntimeConfigManager(t), + manager, + newMemoryStore(), + &scriptedProviderFactory{provider: &scriptedProvider{}}, + nil, + ) + executor := newSubAgentRuntimeToolExecutor(service) + + result, execErr := executor.ExecuteTool(context.Background(), subagent.ToolExecutionInput{ + RunID: "run-subagent-tool-deny", + SessionID: "session-subagent-tool-deny", + TaskID: "task-subagent-tool-deny", + Role: subagent.RoleCoder, + AgentID: "subagent:deny", + Workdir: t.TempDir(), + Timeout: 2 * time.Second, + Call: providertypes.ToolCall{ + ID: "call-deny", + Name: "bash", + Arguments: `{"command":"echo hi"}`, + }, + }) + if execErr == nil { + t.Fatalf("expected permission error") + } + if !errors.Is(execErr, tools.ErrPermissionDenied) { + t.Fatalf("expected ErrPermissionDenied, got %v", execErr) + } + if result.Decision != string(security.DecisionDeny) { + t.Fatalf("decision = %q, want %q", result.Decision, security.DecisionDeny) + } + + events := collectRuntimeEvents(service.Events()) + assertEventSequence(t, events, []EventType{EventSubAgentToolCallStarted, EventSubAgentToolCallDenied}) + assertSubAgentToolEventPayload(t, events, EventSubAgentToolCallDenied, "bash", string(security.DecisionDeny), false) + }) +} diff --git a/internal/subagent/execution_context_test.go b/internal/subagent/execution_context_test.go new file mode 100644 index 00000000..7a07bc73 --- /dev/null +++ b/internal/subagent/execution_context_test.go @@ -0,0 +1,132 @@ +package subagent + +import ( + "context" + "testing" + + providertypes "neo-code/internal/provider/types" +) + +type noopToolExecutor struct{} + +func (noopToolExecutor) ListToolSpecs(ctx context.Context, input ToolSpecListInput) ([]providertypes.ToolSpec, error) { + _ = ctx + _ = input + return nil, nil +} + +func (noopToolExecutor) ExecuteTool(ctx context.Context, input ToolExecutionInput) (ToolExecutionResult, error) { + _ = ctx + _ = input + return ToolExecutionResult{}, nil +} + +func TestWorkerStepPropagatesExecutionContext(t *testing.T) { + t.Parallel() + + policy, err := DefaultRolePolicy(RoleCoder) + if err != nil { + t.Fatalf("DefaultRolePolicy() error = %v", err) + } + + var captured StepInput + worker, err := NewWorker(RoleCoder, policy, EngineFunc(func(ctx context.Context, input StepInput) (StepOutput, error) { + _ = ctx + captured = input + return StepOutput{ + Done: true, + Output: Output{ + Summary: "done", + Findings: []string{"f1"}, + Patches: []string{"p1"}, + Risks: []string{"r1"}, + NextActions: []string{"n1"}, + Artifacts: []string{"a1"}, + }, + }, nil + }), withExecutionContext(ExecutionContext{ToolExecutor: noopToolExecutor{}})) + if err != nil { + t.Fatalf("NewWorker() error = %v", err) + } + + task := Task{ + ID: "task-exec", + Goal: "goal", + Workspace: " /tmp/workspace ", + RunID: "run-exec", + SessionID: "session-exec", + AgentID: "agent-exec", + } + if err := worker.Start(task, Budget{MaxSteps: 1}, Capability{}); err != nil { + t.Fatalf("Start() error = %v", err) + } + if _, err := worker.Step(context.Background()); err != nil { + t.Fatalf("Step() error = %v", err) + } + + if captured.RunID != "run-exec" { + t.Fatalf("RunID = %q, want %q", captured.RunID, "run-exec") + } + if captured.SessionID != "session-exec" { + t.Fatalf("SessionID = %q, want %q", captured.SessionID, "session-exec") + } + if captured.AgentID != "agent-exec" { + t.Fatalf("AgentID = %q, want %q", captured.AgentID, "agent-exec") + } + if captured.Workdir != "/tmp/workspace" { + t.Fatalf("Workdir = %q, want %q", captured.Workdir, "/tmp/workspace") + } + if captured.Executor == nil { + t.Fatalf("expected executor to be injected") + } +} + +func TestRolePolicyToolDefaultsAndValidation(t *testing.T) { + t.Parallel() + + policy, err := DefaultRolePolicy(RoleReviewer) + if err != nil { + t.Fatalf("DefaultRolePolicy() error = %v", err) + } + if policy.ToolUseMode != ToolUseModeAuto { + t.Fatalf("ToolUseMode = %q, want %q", policy.ToolUseMode, ToolUseModeAuto) + } + if policy.MaxToolCallsPerStep <= 0 { + t.Fatalf("MaxToolCallsPerStep = %d, want > 0", policy.MaxToolCallsPerStep) + } + + tests := []struct { + name string + mode ToolUseMode + limit int + wantErr bool + }{ + {name: "valid auto", mode: ToolUseModeAuto, limit: 1, wantErr: false}, + {name: "valid required", mode: ToolUseModeRequired, limit: 2, wantErr: false}, + {name: "valid disabled", mode: ToolUseModeDisabled, limit: 3, wantErr: false}, + {name: "invalid mode", mode: ToolUseMode("unknown"), limit: 1, wantErr: true}, + {name: "invalid negative limit", mode: ToolUseModeAuto, limit: -1, wantErr: true}, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + p := RolePolicy{ + Role: RoleReviewer, + SystemPrompt: "prompt", + AllowedTools: []string{"filesystem_read_file"}, + ToolUseMode: tt.mode, + MaxToolCallsPerStep: tt.limit, + RequiredSections: []string{"summary"}, + } + err := p.Validate() + if tt.wantErr && err == nil { + t.Fatalf("expected error") + } + if !tt.wantErr && err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} diff --git a/internal/subagent/factory.go b/internal/subagent/factory.go index 77f7bf05..6f79044d 100644 --- a/internal/subagent/factory.go +++ b/internal/subagent/factory.go @@ -5,14 +5,49 @@ import "time" // EngineBuilder 定义基于角色策略构建执行引擎的工厂函数。 type EngineBuilder func(role Role, policy RolePolicy) Engine +// ExecutionContext 描述子代理执行时可选注入的运行时上下文。 +type ExecutionContext struct { + ToolExecutor ToolExecutor +} + +// FactoryOption 定义 WorkerFactory 的可选注入项。 +type FactoryOption func(*WorkerFactory) + // WorkerFactory 是默认的 WorkerRuntime 工厂实现。 type WorkerFactory struct { builder EngineBuilder + execCtx ExecutionContext } // NewWorkerFactory 创建 WorkerFactory；当 builder 为空时使用默认引擎。 -func NewWorkerFactory(builder EngineBuilder) *WorkerFactory { - return &WorkerFactory{builder: builder} +func NewWorkerFactory(builder EngineBuilder, opts ...FactoryOption) *WorkerFactory { + factory := &WorkerFactory{builder: builder} + for _, opt := range opts { + if opt != nil { + opt(factory) + } + } + return factory +} + +// WithExecutionContext 注入默认执行上下文。 +func WithExecutionContext(execCtx ExecutionContext) FactoryOption { + return func(factory *WorkerFactory) { + if factory == nil { + return + } + factory.execCtx = execCtx + } +} + +// WithToolExecutor 注入子代理工具执行桥接。 +func WithToolExecutor(executor ToolExecutor) FactoryOption { + return func(factory *WorkerFactory) { + if factory == nil { + return + } + factory.execCtx.ToolExecutor = executor + } } // Create 基于角色创建对应策略与执行引擎的 WorkerRuntime。 @@ -31,5 +66,9 @@ func (f *WorkerFactory) Create(role Role) (WorkerRuntime, error) { if f != nil && f.builder != nil { engine = f.builder(role, policy) } - return NewWorker(role, policy, engine) + var execCtx ExecutionContext + if f != nil { + execCtx = f.execCtx + } + return NewWorker(role, policy, engine, withExecutionContext(execCtx)) } diff --git a/internal/subagent/policy.go b/internal/subagent/policy.go index 12c81544..d848391e 100644 --- a/internal/subagent/policy.go +++ b/internal/subagent/policy.go @@ -8,15 +8,40 @@ import ( const ( defaultPolicyMaxSteps = 6 defaultPolicyTimeout = 30 + defaultToolCallsLimit = 6 ) +// ToolUseMode 描述子代理在单步中是否允许模型发起工具调用。 +type ToolUseMode string + +const ( + // ToolUseModeAuto 表示模型按需自行决定是否调用工具。 + ToolUseModeAuto ToolUseMode = "auto" + // ToolUseModeRequired 表示模型必须至少调用一次工具再结束。 + ToolUseModeRequired ToolUseMode = "required" + // ToolUseModeDisabled 表示禁止模型发起任何工具调用。 + ToolUseModeDisabled ToolUseMode = "disabled" +) + +// Valid 判断工具调用模式是否受支持。 +func (m ToolUseMode) Valid() bool { + switch m { + case ToolUseModeAuto, ToolUseModeRequired, ToolUseModeDisabled: + return true + default: + return false + } +} + // RolePolicy 定义不同角色的执行策略。 type RolePolicy struct { - Role Role - SystemPrompt string - AllowedTools []string - DefaultBudget Budget - RequiredSections []string + Role Role + SystemPrompt string + AllowedTools []string + ToolUseMode ToolUseMode + MaxToolCallsPerStep int + DefaultBudget Budget + RequiredSections []string } // Validate 校验角色策略是否合法。 @@ -33,6 +58,15 @@ func (p RolePolicy) Validate() error { if len(dedupeAndTrim(p.RequiredSections)) == 0 { return errorsf("role policy required sections is empty") } + if p.ToolUseMode == "" { + p.ToolUseMode = ToolUseModeAuto + } + if !p.ToolUseMode.Valid() { + return errorsf("role policy tool use mode %q is invalid", p.ToolUseMode) + } + if p.MaxToolCallsPerStep < 0 { + return errorsf("role policy max tool calls per step must not be negative") + } if _, err := normalizeRequiredSections(p.RequiredSections); err != nil { return err } @@ -51,6 +85,8 @@ func DefaultRolePolicy(role Role) (RolePolicy, error) { MaxSteps: defaultPolicyMaxSteps, Timeout: defaultPolicyTimeout * time.Second, }, + ToolUseMode: ToolUseModeAuto, + MaxToolCallsPerStep: defaultToolCallsLimit, RequiredSections: []string{ "summary", "findings", diff --git a/internal/subagent/types.go b/internal/subagent/types.go index ca8b426e..4cbad687 100644 --- a/internal/subagent/types.go +++ b/internal/subagent/types.go @@ -5,6 +5,8 @@ import ( "fmt" "strings" "time" + + providertypes "neo-code/internal/provider/types" ) // Role 表示子代理的执行角色。 @@ -73,6 +75,9 @@ type Task struct { Goal string ExpectedOutput string Workspace string + RunID string + SessionID string + AgentID string ContextSlice TaskContextSlice } @@ -161,6 +166,11 @@ type StepInput struct { Task Task Budget Budget Capability Capability + RunID string + SessionID string + AgentID string + Workdir string + Executor ToolExecutor StepIndex int Trace []string } @@ -195,6 +205,41 @@ type Result struct { Error string } +// ToolSpecListInput 描述列举子代理可见工具 schema 的输入上下文。 +type ToolSpecListInput struct { + SessionID string + Role Role + AllowedTools []string +} + +// ToolExecutionInput 描述一次子代理工具执行请求。 +type ToolExecutionInput struct { + RunID string + SessionID string + TaskID string + Role Role + AgentID string + Workdir string + Timeout time.Duration + Call providertypes.ToolCall +} + +// ToolExecutionResult 描述子代理工具执行后的标准结果。 +type ToolExecutionResult struct { + ToolCallID string + Name string + Content string + IsError bool + Decision string + Metadata map[string]any +} + +// ToolExecutor 定义子代理访问 runtime 工具能力的最小桥接接口。 +type ToolExecutor interface { + ListToolSpecs(ctx context.Context, input ToolSpecListInput) ([]providertypes.ToolSpec, error) + ExecuteTool(ctx context.Context, input ToolExecutionInput) (ToolExecutionResult, error) +} + // Engine 定义 WorkerRuntime 的单步执行引擎。 type Engine interface { RunStep(ctx context.Context, input StepInput) (StepOutput, error) diff --git a/internal/subagent/worker.go b/internal/subagent/worker.go index 6e6b79d2..4619de96 100644 --- a/internal/subagent/worker.go +++ b/internal/subagent/worker.go @@ -15,6 +15,7 @@ type worker struct { role Role policy RolePolicy engine Engine + execCtx ExecutionContext state State task Task budget Budget @@ -30,8 +31,21 @@ type worker struct { const traceWindowSize = 16 +// WorkerOption 定义 Worker 的可选注入项。 +type WorkerOption func(*worker) + +// withExecutionContext 注入 Worker 运行期上下文。 +func withExecutionContext(execCtx ExecutionContext) WorkerOption { + return func(w *worker) { + if w == nil { + return + } + w.execCtx = execCtx + } +} + // NewWorker 根据角色、策略与引擎创建一个 WorkerRuntime 实例。 -func NewWorker(role Role, policy RolePolicy, engine Engine) (WorkerRuntime, error) { +func NewWorker(role Role, policy RolePolicy, engine Engine, opts ...WorkerOption) (WorkerRuntime, error) { if !role.Valid() { return nil, errorsf("invalid role %q", role) } @@ -48,12 +62,18 @@ func NewWorker(role Role, policy RolePolicy, engine Engine) (WorkerRuntime, erro engine = defaultEngine{} } - return &worker{ + instance := &worker{ role: role, policy: policy, engine: engine, state: StateIdle, - }, nil + } + for _, opt := range opts { + if opt != nil { + opt(instance) + } + } + return instance, nil } // Start 初始化任务执行上下文并进入运行态。 @@ -129,6 +149,11 @@ func (w *worker) Step(ctx context.Context) (StepResult, error) { Task: w.task, Budget: w.budget, Capability: w.capability, + RunID: strings.TrimSpace(w.task.RunID), + SessionID: strings.TrimSpace(w.task.SessionID), + AgentID: strings.TrimSpace(w.task.AgentID), + Workdir: resolveStepWorkdir(w.task.Workspace), + Executor: w.execCtx.ToolExecutor, StepIndex: w.stepCount + 1, Trace: cloneRecentTrace(w.trace, traceWindowSize), } @@ -258,6 +283,11 @@ func cloneRecentTrace(trace []string, limit int) []string { return append([]string(nil), trace[start:]...) } +// resolveStepWorkdir 解析单步执行所使用的工作目录。 +func resolveStepWorkdir(workspace string) string { + return strings.TrimSpace(workspace) +} + // Stop 主动终止运行中的 worker，并按终止原因映射最终状态。 func (w *worker) Stop(reason StopReason) error { if w == nil { From 0b97dac1b71c086db89e8e9ce7ebe3cfe8e5b95e Mon Sep 17 00:00:00 2001 From: xgopilot Date: Sat, 18 Apr 2026 08:01:17 +0000 Subject: [PATCH 2/5] fix(subagent): tighten step error handling and simplify loop internals Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com> --- internal/runtime/subagent_engine.go | 114 ++++++++++++++++---- internal/runtime/subagent_engine_test.go | 129 ++++++++++++++++------- 2 files changed, 182 insertions(+), 61 deletions(-) diff --git a/internal/runtime/subagent_engine.go b/internal/runtime/subagent_engine.go index 8f238f5d..b3208b62 100644 --- a/internal/runtime/subagent_engine.go +++ b/internal/runtime/subagent_engine.go @@ -10,6 +10,7 @@ import ( "neo-code/internal/config" "neo-code/internal/partsrender" + "neo-code/internal/provider" "neo-code/internal/provider/streaming" providertypes "neo-code/internal/provider/types" "neo-code/internal/subagent" @@ -21,6 +22,17 @@ const ( subAgentMaxStepTurnsLimit = 12 ) +var errSubAgentRuntimeUnavailable = errors.New("runtime: subagent runtime dependencies unavailable") + +var subAgentOutputRequiredKeys = []string{ + "summary", + "findings", + "patches", + "risks", + "next_actions", + "artifacts", +} + type subAgentOutputJSON struct { Summary string `json:"summary"` Findings []string `json:"findings"` @@ -56,11 +68,18 @@ func (e runtimeSubAgentEngine) RunStep(ctx context.Context, input subagent.StepI runtimeConfig, model, toolTimeout, err := e.resolveSettings() if err != nil { - return fallbackSubAgentStep(input), nil + if errors.Is(err, errSubAgentRuntimeUnavailable) { + return fallbackSubAgentStep(input), nil + } + return subagent.StepOutput{}, err } if input.Executor == nil { return subagent.StepOutput{}, errors.New("runtime: subagent tool executor is nil") } + modelProvider, err := e.buildProvider(ctx, runtimeConfig) + if err != nil { + return subagent.StepOutput{}, err + } allowedTools := resolveAllowedTools(input) toolSpecs, err := input.Executor.ListToolSpecs(ctx, subagent.ToolSpecListInput{ @@ -81,14 +100,12 @@ func (e runtimeSubAgentEngine) RunStep(ctx context.Context, input subagent.StepI maxTurns := resolveSubAgentMaxTurns(input.Budget.MaxSteps) for turn := 1; turn <= maxTurns; turn++ { - outcome, err := e.generateStepMessage(ctx, runtimeConfig, model, systemPrompt, messages, toolSpecs) + outcome, err := e.generateStepMessage(ctx, modelProvider, model, systemPrompt, messages, toolSpecs) if err != nil { return subagent.StepOutput{}, err } assistant := outcome.message - if strings.TrimSpace(assistant.Role) == "" { - assistant.Role = providertypes.RoleAssistant - } + assistant = ensureAssistantRole(assistant) if !assistant.IsEmpty() { messages = append(messages, assistant) } @@ -130,13 +147,27 @@ func (e runtimeSubAgentEngine) RunStep(ctx context.Context, input subagent.StepI return subagent.StepOutput{}, fmt.Errorf("runtime: subagent step exceeded max turns (%d)", maxTurns) } +// ensureAssistantRole 确保模型输出消息具备 assistant 角色，避免后续流程依赖空角色值。 +func ensureAssistantRole(message providertypes.Message) providertypes.Message { + if strings.TrimSpace(message.Role) == "" { + message.Role = providertypes.RoleAssistant + } + return message +} + // resolveSettings 读取当前 runtime 配置，并解析子代理调用 provider 所需参数。 func (e runtimeSubAgentEngine) resolveSettings() (config.ResolvedProviderConfig, string, time.Duration, error) { if e.service == nil || e.service.configManager == nil { - return config.ResolvedProviderConfig{}, "", 0, errors.New("runtime: subagent service or config manager is nil") + return config.ResolvedProviderConfig{}, "", 0, fmt.Errorf( + "%w: service or config manager is nil", + errSubAgentRuntimeUnavailable, + ) } if e.service.providerFactory == nil { - return config.ResolvedProviderConfig{}, "", 0, errors.New("runtime: subagent provider factory is nil") + return config.ResolvedProviderConfig{}, "", 0, fmt.Errorf( + "%w: provider factory is nil", + errSubAgentRuntimeUnavailable, + ) } cfg := e.service.configManager.Get() resolvedProvider, err := config.ResolveSelectedProvider(cfg) @@ -157,19 +188,27 @@ func (e runtimeSubAgentEngine) resolveSettings() (config.ResolvedProviderConfig, return resolvedProvider, model, timeout, nil } +// buildProvider 基于解析后的 provider 配置创建单步内复用的模型实例。 +func (e runtimeSubAgentEngine) buildProvider( + ctx context.Context, + resolvedProvider config.ResolvedProviderConfig, +) (provider.Provider, error) { + modelProvider, err := e.service.providerFactory.Build(ctx, resolvedProvider.ToRuntimeConfig()) + if err != nil { + return nil, fmt.Errorf("runtime: build subagent provider: %w", err) + } + return modelProvider, nil +} + // generateStepMessage 发起一次 provider 调用并返回本轮 assistant 输出。 func (e runtimeSubAgentEngine) generateStepMessage( ctx context.Context, - resolvedProvider config.ResolvedProviderConfig, + modelProvider provider.Provider, model string, systemPrompt string, messages []providertypes.Message, toolSpecs []providertypes.ToolSpec, ) (streamGenerateResult, error) { - modelProvider, err := e.service.providerFactory.Build(ctx, resolvedProvider.ToRuntimeConfig()) - if err != nil { - return streamGenerateResult{}, fmt.Errorf("runtime: build subagent provider: %w", err) - } outcome := generateStreamingMessage(ctx, modelProvider, providertypes.GenerateRequest{ Model: model, SystemPrompt: systemPrompt, @@ -325,16 +364,15 @@ func parseSubAgentOutput(text string) (subagent.Output, error) { }, nil } -// extractSubAgentJSONObject 从文本中提取首个完整 JSON 对象，容忍前后噪声。 +// extractSubAgentJSONObject 从文本中提取最可能的输出 JSON，优先选择包含输出契约字段的对象。 func extractSubAgentJSONObject(text string) (string, error) { - start := strings.Index(text, "{") - if start < 0 { - return "", errors.New("runtime: subagent output does not contain json object") - } depth := 0 inString := false escaped := false - for index := start; index < len(text); index++ { + start := -1 + lastObject := "" + contractObject := "" + for index := 0; index < len(text); index++ { ch := text[index] if inString { if escaped { @@ -354,15 +392,49 @@ func extractSubAgentJSONObject(text string) (string, error) { case '"': inString = true case '{': + if depth == 0 { + start = index + } depth++ case '}': - depth-- if depth == 0 { - return strings.TrimSpace(text[start : index+1]), nil + continue } + depth-- + if depth == 0 && start >= 0 { + candidate := strings.TrimSpace(text[start : index+1]) + lastObject = candidate + if matchesSubAgentOutputContract(candidate) { + contractObject = candidate + } + start = -1 + } + } + } + if contractObject != "" { + return contractObject, nil + } + if lastObject != "" { + return lastObject, nil + } + if strings.Contains(text, "{") { + return "", errors.New("runtime: subagent output contains incomplete json object") + } + return "", errors.New("runtime: subagent output does not contain json object") +} + +// matchesSubAgentOutputContract 判断 JSON 文本是否包含子代理输出契约必需字段。 +func matchesSubAgentOutputContract(text string) bool { + var payload map[string]json.RawMessage + if err := json.Unmarshal([]byte(text), &payload); err != nil { + return false + } + for _, key := range subAgentOutputRequiredKeys { + if _, ok := payload[key]; !ok { + return false } } - return "", errors.New("runtime: subagent output contains incomplete json object") + return true } // renderAssistantText 将 assistant parts 渲染为统一文本，用于 JSON 输出解析。 diff --git a/internal/runtime/subagent_engine_test.go b/internal/runtime/subagent_engine_test.go index d98611d6..67c6ea93 100644 --- a/internal/runtime/subagent_engine_test.go +++ b/internal/runtime/subagent_engine_test.go @@ -2,6 +2,7 @@ package runtime import ( "context" + "errors" "strings" "testing" @@ -10,6 +11,8 @@ import ( "neo-code/internal/tools" ) +const subAgentDonePayload = `{"summary":"done","findings":["f1"],"patches":["p1"],"risks":["r1"],"next_actions":["n1"],"artifacts":["a1"]}` + func TestRuntimeSubAgentEngineRunStepToolLoopSuccess(t *testing.T) { t.Parallel() @@ -29,7 +32,7 @@ func TestRuntimeSubAgentEngineRunStepToolLoopSuccess(t *testing.T) { Message: providertypes.Message{ Role: providertypes.RoleAssistant, Parts: []providertypes.ContentPart{ - providertypes.NewTextPart(`{"summary":"done","findings":["f1"],"patches":["p1"],"risks":["r1"],"next_actions":["n1"],"artifacts":["a1"]}`), + providertypes.NewTextPart(subAgentDonePayload), }, }, FinishReason: "stop", @@ -48,7 +51,8 @@ func TestRuntimeSubAgentEngineRunStepToolLoopSuccess(t *testing.T) { }, nil }, } - service := NewWithFactory(manager, toolManager, newMemoryStore(), &scriptedProviderFactory{provider: providerImpl}, nil) + providerFactory := &scriptedProviderFactory{provider: providerImpl} + service := NewWithFactory(manager, toolManager, newMemoryStore(), providerFactory, nil) policy, err := subagent.DefaultRolePolicy(subagent.RoleCoder) if err != nil { t.Fatalf("DefaultRolePolicy() error = %v", err) @@ -57,20 +61,17 @@ func TestRuntimeSubAgentEngineRunStepToolLoopSuccess(t *testing.T) { policy.MaxToolCallsPerStep = 2 engine := runtimeSubAgentEngine{service: service, role: subagent.RoleCoder, policy: policy} - output, err := engine.RunStep(context.Background(), subagent.StepInput{ - Role: subagent.RoleCoder, - Policy: policy, - Task: subagent.Task{ID: "task-1", Goal: "read file and summarize"}, - Budget: subagent.Budget{MaxSteps: 4}, - Capability: subagent.Capability{ - AllowedTools: []string{"filesystem_read_file"}, - }, - RunID: "run-subagent-step-success", - SessionID: "session-subagent-step-success", - AgentID: "subagent:task-1", - Workdir: t.TempDir(), - Executor: newSubAgentRuntimeToolExecutor(service), + stepInput := newRuntimeSubAgentStepInput(t, service, policy, subagent.Task{ + ID: "task-1", + Goal: "read file and summarize", }) + stepInput.Budget = subagent.Budget{MaxSteps: 4} + stepInput.Capability = subagent.Capability{AllowedTools: []string{"filesystem_read_file"}} + stepInput.RunID = "run-subagent-step-success" + stepInput.SessionID = "session-subagent-step-success" + stepInput.AgentID = "subagent:task-1" + + output, err := engine.RunStep(context.Background(), stepInput) if err != nil { t.Fatalf("RunStep() error = %v", err) } @@ -86,6 +87,9 @@ func TestRuntimeSubAgentEngineRunStepToolLoopSuccess(t *testing.T) { if providerImpl.callCount != 2 { t.Fatalf("provider calls = %d, want 2", providerImpl.callCount) } + if providerFactory.calls != 1 { + t.Fatalf("provider build calls = %d, want 1", providerFactory.calls) + } events := collectRuntimeEvents(service.Events()) assertEventSequence(t, events, []EventType{EventSubAgentToolCallStarted, EventSubAgentToolCallResult}) @@ -129,20 +133,17 @@ func TestRuntimeSubAgentEngineRunStepCapabilityDenied(t *testing.T) { policy.MaxToolCallsPerStep = 2 engine := runtimeSubAgentEngine{service: service, role: subagent.RoleCoder, policy: policy} - stepOutput, err := engine.RunStep(context.Background(), subagent.StepInput{ - Role: subagent.RoleCoder, - Policy: policy, - Task: subagent.Task{ID: "task-cap-deny", Goal: "execute bash"}, - Budget: subagent.Budget{MaxSteps: 4}, - Capability: subagent.Capability{ - AllowedTools: []string{"filesystem_read_file"}, - }, - RunID: "run-subagent-cap-deny", - SessionID: "session-subagent-cap-deny", - AgentID: "subagent:task-cap-deny", - Workdir: t.TempDir(), - Executor: newSubAgentRuntimeToolExecutor(service), + stepInput := newRuntimeSubAgentStepInput(t, service, policy, subagent.Task{ + ID: "task-cap-deny", + Goal: "execute bash", }) + stepInput.Budget = subagent.Budget{MaxSteps: 4} + stepInput.Capability = subagent.Capability{AllowedTools: []string{"filesystem_read_file"}} + stepInput.RunID = "run-subagent-cap-deny" + stepInput.SessionID = "session-subagent-cap-deny" + stepInput.AgentID = "subagent:task-cap-deny" + + stepOutput, err := engine.RunStep(context.Background(), stepInput) if err != nil { t.Fatalf("RunStep() error = %v", err) } @@ -168,7 +169,7 @@ func TestRuntimeSubAgentEngineRunStepRequiredModeWithoutToolFails(t *testing.T) Message: providertypes.Message{ Role: providertypes.RoleAssistant, Parts: []providertypes.ContentPart{ - providertypes.NewTextPart(`{"summary":"done","findings":["f1"],"patches":["p1"],"risks":["r1"],"next_actions":["n1"],"artifacts":["a1"]}`), + providertypes.NewTextPart(subAgentDonePayload), }, }, FinishReason: "stop", @@ -187,18 +188,17 @@ func TestRuntimeSubAgentEngineRunStepRequiredModeWithoutToolFails(t *testing.T) policy.MaxToolCallsPerStep = 1 engine := runtimeSubAgentEngine{service: service, role: subagent.RoleCoder, policy: policy} - _, err = engine.RunStep(context.Background(), subagent.StepInput{ - Role: subagent.RoleCoder, - Policy: policy, - Task: subagent.Task{ID: "task-required", Goal: "must call tool"}, - Budget: subagent.Budget{MaxSteps: 2}, - Capability: subagent.Capability{AllowedTools: []string{"filesystem_read_file"}}, - RunID: "run-subagent-required", - SessionID: "session-subagent-required", - AgentID: "subagent:task-required", - Workdir: t.TempDir(), - Executor: newSubAgentRuntimeToolExecutor(service), + stepInput := newRuntimeSubAgentStepInput(t, service, policy, subagent.Task{ + ID: "task-required", + Goal: "must call tool", }) + stepInput.Budget = subagent.Budget{MaxSteps: 2} + stepInput.Capability = subagent.Capability{AllowedTools: []string{"filesystem_read_file"}} + stepInput.RunID = "run-subagent-required" + stepInput.SessionID = "session-subagent-required" + stepInput.AgentID = "subagent:task-required" + + _, err = engine.RunStep(context.Background(), stepInput) if err == nil || !strings.Contains(err.Error(), "requires at least one tool call") { t.Fatalf("expected required-mode error, got %v", err) } @@ -232,6 +232,32 @@ func TestRuntimeSubAgentEngineFallbackWhenRuntimeUnavailable(t *testing.T) { } } +func TestRuntimeSubAgentEngineRunStepProviderBuildFailureReturnsError(t *testing.T) { + t.Parallel() + + manager := newRuntimeConfigManager(t) + policy, err := subagent.DefaultRolePolicy(subagent.RoleReviewer) + if err != nil { + t.Fatalf("DefaultRolePolicy() error = %v", err) + } + engine := runtimeSubAgentEngine{ + service: NewWithFactory(manager, &stubToolManager{}, newMemoryStore(), &scriptedProviderFactory{ + err: errors.New("provider init failed"), + }, nil), + role: subagent.RoleReviewer, + policy: policy, + } + _, err = engine.RunStep(context.Background(), subagent.StepInput{ + Role: subagent.RoleReviewer, + Policy: policy, + Task: subagent.Task{ID: "task-provider-fail", Goal: "review"}, + Executor: newSubAgentRuntimeToolExecutor(engine.service), + }) + if err == nil || !strings.Contains(err.Error(), "build subagent provider") { + t.Fatalf("expected build provider error, got %v", err) + } +} + func TestParseSubAgentOutput(t *testing.T) { t.Parallel() @@ -253,6 +279,13 @@ func TestParseSubAgentOutput(t *testing.T) { input: `summary only`, wantErr: true, }, + { + name: "pick object with output contract keys", + input: strings.Join([]string{ + `{"example":true}`, + `{"summary":"s","findings":["f"],"patches":["p"],"risks":["r"],"next_actions":["n"],"artifacts":["a"]}`, + }, "\n"), + }, } for _, tt := range tests { @@ -306,3 +339,19 @@ func assertSubAgentToolEventPayload( } t.Fatalf("event %q not found", eventType) } + +func newRuntimeSubAgentStepInput( + t *testing.T, + service *Service, + policy subagent.RolePolicy, + task subagent.Task, +) subagent.StepInput { + t.Helper() + return subagent.StepInput{ + Role: policy.Role, + Policy: policy, + Task: task, + Workdir: t.TempDir(), + Executor: newSubAgentRuntimeToolExecutor(service), + } +} From 54c1a38398f5d4f7b0f59baa0f77264ab44a773a Mon Sep 17 00:00:00 2001 From: xgopilot Date: Sat, 18 Apr 2026 08:49:30 +0000 Subject: [PATCH 3/5] refactor(subagent): simplify runtime and worker flow without behavior changes Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com> --- internal/runtime/subagent_engine.go | 15 ++--- internal/runtime/subagent_run.go | 76 +++++++++++----------- internal/runtime/subagent_tool_executor.go | 54 +++++++++------ internal/subagent/factory.go | 15 +++-- internal/subagent/worker.go | 26 +++++--- 5 files changed, 107 insertions(+), 79 deletions(-) diff --git a/internal/runtime/subagent_engine.go b/internal/runtime/subagent_engine.go index b3208b62..eeb07be4 100644 --- a/internal/runtime/subagent_engine.go +++ b/internal/runtime/subagent_engine.go @@ -158,16 +158,10 @@ func ensureAssistantRole(message providertypes.Message) providertypes.Message { // resolveSettings 读取当前 runtime 配置，并解析子代理调用 provider 所需参数。 func (e runtimeSubAgentEngine) resolveSettings() (config.ResolvedProviderConfig, string, time.Duration, error) { if e.service == nil || e.service.configManager == nil { - return config.ResolvedProviderConfig{}, "", 0, fmt.Errorf( - "%w: service or config manager is nil", - errSubAgentRuntimeUnavailable, - ) + return config.ResolvedProviderConfig{}, "", 0, runtimeUnavailableError("service or config manager is nil") } if e.service.providerFactory == nil { - return config.ResolvedProviderConfig{}, "", 0, fmt.Errorf( - "%w: provider factory is nil", - errSubAgentRuntimeUnavailable, - ) + return config.ResolvedProviderConfig{}, "", 0, runtimeUnavailableError("provider factory is nil") } cfg := e.service.configManager.Get() resolvedProvider, err := config.ResolveSelectedProvider(cfg) @@ -188,6 +182,11 @@ func (e runtimeSubAgentEngine) resolveSettings() (config.ResolvedProviderConfig, return resolvedProvider, model, timeout, nil } +// runtimeUnavailableError 封装 runtime 依赖缺失错误，保持错误类型与消息结构一致。 +func runtimeUnavailableError(detail string) error { + return fmt.Errorf("%w: %s", errSubAgentRuntimeUnavailable, strings.TrimSpace(detail)) +} + // buildProvider 基于解析后的 provider 配置创建单步内复用的模型实例。 func (e runtimeSubAgentEngine) buildProvider( ctx context.Context, diff --git a/internal/runtime/subagent_run.go b/internal/runtime/subagent_run.go index a38efad3..3e51e9ae 100644 --- a/internal/runtime/subagent_run.go +++ b/internal/runtime/subagent_run.go @@ -48,22 +48,12 @@ func (s *Service) RunSubAgentTask(ctx context.Context, input SubAgentTaskInput) factory := s.SubAgentFactory() worker, err := factory.Create(input.Role) if err != nil { - _ = s.emit(ctx, EventSubAgentFailed, input.RunID, input.SessionID, SubAgentEventPayload{ - Role: input.Role, - TaskID: input.Task.ID, - State: subagent.StateFailed, - Error: err.Error(), - }) + emitSubAgentFailed(s, ctx, input.RunID, input.SessionID, input.Role, input.Task.ID, err) return subagent.Result{}, err } if err := worker.Start(task, input.Budget, input.Capability); err != nil { - _ = s.emit(ctx, EventSubAgentFailed, input.RunID, input.SessionID, SubAgentEventPayload{ - Role: input.Role, - TaskID: input.Task.ID, - State: subagent.StateFailed, - Error: err.Error(), - }) + emitSubAgentFailed(s, ctx, input.RunID, input.SessionID, input.Role, input.Task.ID, err) return subagent.Result{}, err } @@ -85,13 +75,7 @@ func (s *Service) RunSubAgentTask(ctx context.Context, input SubAgentTaskInput) _ = worker.Stop(subagent.StopReasonTimeout) result, resultErr := worker.Result() if resultErr != nil { - result = subagent.Result{ - Role: input.Role, - TaskID: task.ID, - State: subagent.StateFailed, - StopReason: subagent.StopReasonTimeout, - Error: errorText(stepErr), - } + result = fallbackSubAgentResult(input.Role, task.ID, subagent.StateFailed, subagent.StopReasonTimeout, stepErr) } emitSubAgentTerminal(s, ctx, input, result) return result, stepErr @@ -100,13 +84,7 @@ func (s *Service) RunSubAgentTask(ctx context.Context, input SubAgentTaskInput) _ = worker.Stop(subagent.StopReasonCanceled) result, resultErr := worker.Result() if resultErr != nil { - result = subagent.Result{ - Role: input.Role, - TaskID: task.ID, - State: subagent.StateCanceled, - StopReason: subagent.StopReasonCanceled, - Error: errorText(stepErr), - } + result = fallbackSubAgentResult(input.Role, task.ID, subagent.StateCanceled, subagent.StopReasonCanceled, stepErr) } emitSubAgentTerminal(s, ctx, input, result) return result, stepErr @@ -114,12 +92,7 @@ func (s *Service) RunSubAgentTask(ctx context.Context, input SubAgentTaskInput) result, resultErr := worker.Result() if resultErr != nil { - _ = s.emit(ctx, EventSubAgentFailed, input.RunID, input.SessionID, SubAgentEventPayload{ - Role: input.Role, - TaskID: task.ID, - State: subagent.StateFailed, - Error: stepErr.Error(), - }) + emitSubAgentFailed(s, ctx, input.RunID, input.SessionID, input.Role, task.ID, stepErr) return subagent.Result{}, stepErr } emitSubAgentTerminal(s, ctx, input, result) @@ -132,12 +105,7 @@ func (s *Service) RunSubAgentTask(ctx context.Context, input SubAgentTaskInput) result, err := worker.Result() if err != nil { - _ = s.emit(ctx, EventSubAgentFailed, input.RunID, input.SessionID, SubAgentEventPayload{ - Role: input.Role, - TaskID: task.ID, - State: subagent.StateFailed, - Error: err.Error(), - }) + emitSubAgentFailed(s, ctx, input.RunID, input.SessionID, input.Role, task.ID, err) return subagent.Result{}, err } emitSubAgentTerminal(s, ctx, input, result) @@ -148,6 +116,27 @@ func (s *Service) RunSubAgentTask(ctx context.Context, input SubAgentTaskInput) } } +// emitSubAgentFailed 统一发射子代理失败事件，避免重复构造相同载荷。 +func emitSubAgentFailed( + s *Service, + ctx context.Context, + runID string, + sessionID string, + role subagent.Role, + taskID string, + err error, +) { + if s == nil { + return + } + _ = s.emit(ctx, EventSubAgentFailed, runID, sessionID, SubAgentEventPayload{ + Role: role, + TaskID: taskID, + State: subagent.StateFailed, + Error: errorText(err), + }) +} + // emitSubAgentProgress 非阻塞发射进度事件，避免慢消费者反压执行路径。 func emitSubAgentProgress( s *Service, @@ -202,6 +191,17 @@ func emitSubAgentTerminal(s *Service, ctx context.Context, input SubAgentTaskInp } } +// fallbackSubAgentResult 在 worker 结果不可用时构造保底终态，确保调用方拿到稳定字段。 +func fallbackSubAgentResult(role subagent.Role, taskID string, state subagent.State, reason subagent.StopReason, err error) subagent.Result { + return subagent.Result{ + Role: role, + TaskID: taskID, + State: state, + StopReason: reason, + Error: errorText(err), + } +} + // errorText 将 error 安全转换为事件可用文本。 func errorText(err error) string { if err == nil { diff --git a/internal/runtime/subagent_tool_executor.go b/internal/runtime/subagent_tool_executor.go index 961fc223..23214724 100644 --- a/internal/runtime/subagent_tool_executor.go +++ b/internal/runtime/subagent_tool_executor.go @@ -11,9 +11,11 @@ import ( "neo-code/internal/tools" ) -const subAgentToolDecisionPending = "pending" -const stringPermissionDecisionAsk = "ask" -const defaultSubAgentToolTimeout = 20 * time.Second +const ( + subAgentToolDecisionPending = "pending" + stringPermissionDecisionAsk = "ask" + defaultSubAgentToolTimeout = 20 * time.Second +) // subAgentRuntimeToolExecutor 将 subagent 工具调用桥接到 runtime 的统一执行链路。 type subAgentRuntimeToolExecutor struct { @@ -56,23 +58,29 @@ func (e *subAgentRuntimeToolExecutor) ExecuteTool( if timeout <= 0 { timeout = defaultSubAgentToolTimeout } + runID := strings.TrimSpace(input.RunID) + sessionID := strings.TrimSpace(input.SessionID) + taskID := strings.TrimSpace(input.TaskID) + agentID := strings.TrimSpace(input.AgentID) + workdir := strings.TrimSpace(input.Workdir) + callName := strings.TrimSpace(input.Call.Name) payload := SubAgentToolCallEventPayload{ Role: input.Role, - TaskID: strings.TrimSpace(input.TaskID), - ToolName: strings.TrimSpace(input.Call.Name), + TaskID: taskID, + ToolName: callName, Decision: subAgentToolDecisionPending, ElapsedMS: 0, } - e.emit(input.RunID, input.SessionID, EventSubAgentToolCallStarted, payload) + e.emit(runID, sessionID, EventSubAgentToolCallStarted, payload) result, execErr := e.service.executeToolCallWithPermission(ctx, permissionExecutionInput{ - RunID: strings.TrimSpace(input.RunID), - SessionID: strings.TrimSpace(input.SessionID), - TaskID: strings.TrimSpace(input.TaskID), - AgentID: strings.TrimSpace(input.AgentID), + RunID: runID, + SessionID: sessionID, + TaskID: taskID, + AgentID: agentID, Call: input.Call, - Workdir: strings.TrimSpace(input.Workdir), + Workdir: workdir, ToolTimeout: timeout, }) @@ -91,14 +99,8 @@ func (e *subAgentRuntimeToolExecutor) ExecuteTool( output.Name = strings.TrimSpace(result.Name) } - decision := permissionDecisionAllow + decision := resolveToolExecutionDecision(execErr) if execErr != nil { - var permissionErr *tools.PermissionDecisionError - if errors.As(execErr, &permissionErr) { - decision = permissionErr.Decision() - } else { - decision = "error" - } output.Decision = decision if strings.TrimSpace(output.Content) == "" { output.Content = strings.TrimSpace(execErr.Error()) @@ -108,7 +110,7 @@ func (e *subAgentRuntimeToolExecutor) ExecuteTool( eventPayload := SubAgentToolCallEventPayload{ Role: input.Role, - TaskID: strings.TrimSpace(input.TaskID), + TaskID: taskID, ToolName: output.Name, Decision: decision, ElapsedMS: elapsedMilliseconds(startedAt), @@ -122,10 +124,22 @@ func (e *subAgentRuntimeToolExecutor) ExecuteTool( if strings.EqualFold(decision, permissionDecisionDeny) || strings.EqualFold(decision, stringPermissionDecisionAsk) { eventType = EventSubAgentToolCallDenied } - e.emit(input.RunID, input.SessionID, eventType, eventPayload) + e.emit(runID, sessionID, eventType, eventPayload) return output, execErr } +// resolveToolExecutionDecision 根据工具执行错误映射统一的权限决策结果。 +func resolveToolExecutionDecision(execErr error) string { + if execErr == nil { + return permissionDecisionAllow + } + var permissionErr *tools.PermissionDecisionError + if errors.As(execErr, &permissionErr) { + return permissionErr.Decision() + } + return "error" +} + // emit 发出子代理工具调用事件，失败路径按 best-effort 忽略。 func (e *subAgentRuntimeToolExecutor) emit(runID string, sessionID string, eventType EventType, payload SubAgentToolCallEventPayload) { if e == nil || e.service == nil { diff --git a/internal/subagent/factory.go b/internal/subagent/factory.go index 6f79044d..30424fcb 100644 --- a/internal/subagent/factory.go +++ b/internal/subagent/factory.go @@ -22,11 +22,7 @@ type WorkerFactory struct { // NewWorkerFactory 创建 WorkerFactory；当 builder 为空时使用默认引擎。 func NewWorkerFactory(builder EngineBuilder, opts ...FactoryOption) *WorkerFactory { factory := &WorkerFactory{builder: builder} - for _, opt := range opts { - if opt != nil { - opt(factory) - } - } + applyFactoryOptions(factory, opts...) return factory } @@ -50,6 +46,15 @@ func WithToolExecutor(executor ToolExecutor) FactoryOption { } } +// applyFactoryOptions 按顺序应用工厂选项，忽略空选项以简化调用方处理。 +func applyFactoryOptions(factory *WorkerFactory, opts ...FactoryOption) { + for _, opt := range opts { + if opt != nil { + opt(factory) + } + } +} + // Create 基于角色创建对应策略与执行引擎的 WorkerRuntime。 func (f *WorkerFactory) Create(role Role) (WorkerRuntime, error) { policy, err := DefaultRolePolicy(role) diff --git a/internal/subagent/worker.go b/internal/subagent/worker.go index 4619de96..c3ffc728 100644 --- a/internal/subagent/worker.go +++ b/internal/subagent/worker.go @@ -135,12 +135,12 @@ func (w *worker) Step(ctx context.Context) (StepResult, error) { if w.budget.Timeout > 0 && time.Since(w.startedAt) >= w.budget.Timeout { result := w.finishLocked(StateFailed, StopReasonTimeout, Output{}, errorsf("worker timeout")) w.mu.Unlock() - return StepResult{State: result.State, Done: true, Step: result.StepCount}, nil + return terminalStepResult(result, ""), nil } if w.stepCount >= w.budget.MaxSteps { result := w.finishLocked(StateFailed, StopReasonMaxSteps, Output{}, errorsf("worker reached max steps")) w.mu.Unlock() - return StepResult{State: result.State, Done: true, Step: result.StepCount}, nil + return terminalStepResult(result, ""), nil } input := StepInput{ @@ -165,16 +165,16 @@ func (w *worker) Step(ctx context.Context) (StepResult, error) { if errors.Is(err, context.DeadlineExceeded) { result := w.finishLocked(StateFailed, StopReasonTimeout, Output{}, err) w.mu.Unlock() - return StepResult{State: result.State, Done: true, Step: result.StepCount}, err + return terminalStepResult(result, ""), err } if errors.Is(err, context.Canceled) { result := w.finishLocked(StateCanceled, StopReasonCanceled, Output{}, nil) w.mu.Unlock() - return StepResult{State: result.State, Done: true, Step: result.StepCount}, err + return terminalStepResult(result, ""), err } result := w.finishLocked(StateFailed, StopReasonError, Output{}, err) w.mu.Unlock() - return StepResult{State: result.State, Done: true, Step: result.StepCount}, err + return terminalStepResult(result, ""), err } w.mu.Lock() @@ -192,15 +192,15 @@ func (w *worker) Step(ctx context.Context) (StepResult, error) { if stepOutput.Done { if err := validateOutputContract(w.policy, stepOutput.Output); err != nil { result := w.finishLocked(StateFailed, StopReasonError, Output{}, err) - return StepResult{State: result.State, Done: true, Step: result.StepCount, Delta: delta}, err + return terminalStepResult(result, delta), err } result := w.finishLocked(StateSucceeded, StopReasonCompleted, stepOutput.Output, nil) - return StepResult{State: result.State, Done: true, Step: result.StepCount, Delta: delta}, nil + return terminalStepResult(result, delta), nil } if w.stepCount >= w.budget.MaxSteps { result := w.finishLocked(StateFailed, StopReasonMaxSteps, Output{}, errorsf("worker reached max steps")) - return StepResult{State: result.State, Done: true, Step: result.StepCount, Delta: delta}, nil + return terminalStepResult(result, delta), nil } return StepResult{ @@ -211,6 +211,16 @@ func (w *worker) Step(ctx context.Context) (StepResult, error) { }, nil } +// terminalStepResult 将 worker 终态结果映射为单步返回，确保终态字段构造一致。 +func terminalStepResult(result Result, delta string) StepResult { + return StepResult{ + State: result.State, + Done: true, + Step: result.StepCount, + Delta: delta, + } +} + // bindCapabilityToPolicy 将 capability 约束在角色策略允许的工具集合内。 func bindCapabilityToPolicy(capability Capability, policy RolePolicy) (Capability, error) { allowedPolicyTools := dedupeAndTrim(policy.AllowedTools) From 778e8c4e2e44bf41ade8e8aa2bdca946116fd4d6 Mon Sep 17 00:00:00 2001 From: xgopilot Date: Sat, 18 Apr 2026 09:33:29 +0000 Subject: [PATCH 4/5] fix(subagent): close runtime risk gaps and raise coverage Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com> --- internal/runtime/subagent_engine.go | 50 ++-- internal/runtime/subagent_engine_test.go | 246 +++++++++++++++++- internal/runtime/subagent_helpers_test.go | 160 ++++++++++++ .../runtime/subagent_more_branches_test.go | 183 +++++++++++++ internal/runtime/subagent_run.go | 3 - internal/runtime/subagent_run_test.go | 45 ++++ internal/runtime/subagent_tool_executor.go | 20 +- .../runtime/subagent_tool_executor_test.go | 119 ++++++++- internal/subagent/factory_test.go | 70 +++++ .../scheduler_types_additional_test.go | 26 ++ internal/subagent/types_test.go | 23 ++ internal/subagent/util_test.go | 24 ++ internal/subagent/worker_test.go | 35 +++ 13 files changed, 954 insertions(+), 50 deletions(-) create mode 100644 internal/runtime/subagent_helpers_test.go create mode 100644 internal/runtime/subagent_more_branches_test.go create mode 100644 internal/subagent/factory_test.go create mode 100644 internal/subagent/scheduler_types_additional_test.go create mode 100644 internal/subagent/util_test.go diff --git a/internal/runtime/subagent_engine.go b/internal/runtime/subagent_engine.go index eeb07be4..73175e93 100644 --- a/internal/runtime/subagent_engine.go +++ b/internal/runtime/subagent_engine.go @@ -69,7 +69,7 @@ func (e runtimeSubAgentEngine) RunStep(ctx context.Context, input subagent.StepI runtimeConfig, model, toolTimeout, err := e.resolveSettings() if err != nil { if errors.Is(err, errSubAgentRuntimeUnavailable) { - return fallbackSubAgentStep(input), nil + return subagent.StepOutput{}, err } return subagent.StepOutput{}, err } @@ -221,6 +221,7 @@ func (e runtimeSubAgentEngine) generateStepMessage( } // executeSubAgentToolCallBatch 顺序执行本轮工具调用并转换为后续模型输入消息。 +// 返回值中的计数仅统计真正执行成功且被允许的工具调用次数。 func executeSubAgentToolCallBatch( ctx context.Context, service *Service, @@ -243,8 +244,7 @@ func executeSubAgentToolCallBatch( if !toolAllowed(allowedTools, normalizedCall.Name) { denied := subagentCapabilityDeniedResult(stepInput, normalizedCall) results = append(results, denied) - emitCapabilityDeniedEvent(service, stepInput, normalizedCall.Name) - executed++ + emitCapabilityDeniedEvent(ctx, service, stepInput, normalizedCall.Name) continue } @@ -262,8 +262,13 @@ func executeSubAgentToolCallBatch( if execErr != nil && strings.TrimSpace(message.Parts[0].Text) == "" { message.Parts[0] = providertypes.NewTextPart(strings.TrimSpace(execErr.Error())) } + if execErr != nil && !isRecoverableSubAgentToolError(execErr) { + return results, executed, execErr + } results = append(results, message) - executed++ + if execErr == nil { + executed++ + } } return results, executed, nil } @@ -469,15 +474,24 @@ func normalizeToolAllowlist(items []string) map[string]struct{} { return set } -// toolAllowed 判断工具名是否在 allowlist 内；allowlist 为空时默认放行。 +// toolAllowed 判断工具名是否在 allowlist 内；allowlist 为空时默认拒绝。 func toolAllowed(allowlist map[string]struct{}, toolName string) bool { if len(allowlist) == 0 { - return true + return false } _, ok := allowlist[strings.ToLower(strings.TrimSpace(toolName))] return ok } +// isRecoverableSubAgentToolError 判断工具调用错误是否可回灌给模型继续推理。 +func isRecoverableSubAgentToolError(err error) bool { + if err == nil { + return true + } + var permissionErr *tools.PermissionDecisionError + return errors.As(err, &permissionErr) +} + // subagentCapabilityDeniedResult 构造 capability 越权时回灌给模型的标准 tool 消息。 func subagentCapabilityDeniedResult(input subagent.StepInput, call providertypes.ToolCall) providertypes.Message { content := tools.FormatError(call.Name, "capability denied", "tool is not allowed in current subagent capability") @@ -491,11 +505,11 @@ func subagentCapabilityDeniedResult(input subagent.StepInput, call providertypes } // emitCapabilityDeniedEvent 发射 capability 越权拒绝事件。 -func emitCapabilityDeniedEvent(service *Service, input subagent.StepInput, toolName string) { +func emitCapabilityDeniedEvent(ctx context.Context, service *Service, input subagent.StepInput, toolName string) { if service == nil { return } - _ = service.emit(context.Background(), EventSubAgentToolCallDenied, input.RunID, input.SessionID, SubAgentToolCallEventPayload{ + _ = service.emit(ctx, EventSubAgentToolCallDenied, input.RunID, input.SessionID, SubAgentToolCallEventPayload{ Role: input.Role, TaskID: input.Task.ID, ToolName: strings.TrimSpace(toolName), @@ -528,26 +542,6 @@ func subAgentToolResultToMessage(call providertypes.ToolCall, result subagent.To } } -// fallbackSubAgentStep 在 runtime 依赖不可用时返回可验证的保底输出，避免子代理直接崩溃。 -func fallbackSubAgentStep(input subagent.StepInput) subagent.StepOutput { - summary := strings.TrimSpace(input.Task.ExpectedOutput) - if summary == "" { - summary = strings.TrimSpace(input.Task.Goal) - } - return subagent.StepOutput{ - Delta: "subagent fallback engine completed", - Done: true, - Output: subagent.Output{ - Summary: summary, - Findings: []string{"fallback-engine: runtime dependencies unavailable"}, - Patches: []string{"fallback-engine: no code changes"}, - Risks: []string{"fallback-engine: no provider/tool verification"}, - NextActions: []string{"initialize runtime config/provider to enable tool-call loop"}, - Artifacts: []string{"subagent-fallback"}, - }, - } -} - // streamingHooksForSubAgent 返回子代理生成阶段使用的默认流式钩子。 func streamingHooksForSubAgent() streaming.Hooks { return streaming.Hooks{} diff --git a/internal/runtime/subagent_engine_test.go b/internal/runtime/subagent_engine_test.go index 67c6ea93..f253af26 100644 --- a/internal/runtime/subagent_engine_test.go +++ b/internal/runtime/subagent_engine_test.go @@ -5,6 +5,7 @@ import ( "errors" "strings" "testing" + "time" providertypes "neo-code/internal/provider/types" "neo-code/internal/subagent" @@ -159,6 +160,59 @@ func TestRuntimeSubAgentEngineRunStepCapabilityDenied(t *testing.T) { assertSubAgentToolEventPayload(t, events, EventSubAgentToolCallDenied, "bash", permissionDecisionDeny, false) } +func TestRuntimeSubAgentEngineRequiredModeCapabilityDeniedDoesNotSatisfyRequirement(t *testing.T) { + t.Parallel() + + manager := newRuntimeConfigManager(t) + providerImpl := &scriptedProvider{ + responses: []scriptedResponse{ + { + Message: providertypes.Message{ + Role: providertypes.RoleAssistant, + ToolCalls: []providertypes.ToolCall{ + {ID: "call-bash", Name: "bash", Arguments: `{"command":"echo hi"}`}, + }, + }, + FinishReason: "tool_calls", + }, + { + Message: providertypes.Message{ + Role: providertypes.RoleAssistant, + Parts: []providertypes.ContentPart{ + providertypes.NewTextPart(subAgentDonePayload), + }, + }, + FinishReason: "stop", + }, + }, + } + toolManager := &stubToolManager{ + specs: []providertypes.ToolSpec{{Name: "bash", Schema: map[string]any{"type": "object"}}}, + } + service := NewWithFactory(manager, toolManager, newMemoryStore(), &scriptedProviderFactory{provider: providerImpl}, nil) + policy, err := subagent.DefaultRolePolicy(subagent.RoleCoder) + if err != nil { + t.Fatalf("DefaultRolePolicy() error = %v", err) + } + policy.ToolUseMode = subagent.ToolUseModeRequired + policy.MaxToolCallsPerStep = 2 + engine := runtimeSubAgentEngine{service: service, role: subagent.RoleCoder, policy: policy} + + stepInput := newRuntimeSubAgentStepInput(t, service, policy, subagent.Task{ + ID: "task-required-denied", + Goal: "must call allowed tool", + }) + stepInput.Budget = subagent.Budget{MaxSteps: 4} + stepInput.Capability = subagent.Capability{AllowedTools: []string{"filesystem_read_file"}} + stepInput.RunID = "run-subagent-required-denied" + stepInput.SessionID = "session-subagent-required-denied" + + _, err = engine.RunStep(context.Background(), stepInput) + if err == nil || !strings.Contains(err.Error(), "requires at least one tool call") { + t.Fatalf("expected required-mode error, got %v", err) + } +} + func TestRuntimeSubAgentEngineRunStepRequiredModeWithoutToolFails(t *testing.T) { t.Parallel() @@ -204,7 +258,7 @@ func TestRuntimeSubAgentEngineRunStepRequiredModeWithoutToolFails(t *testing.T) } } -func TestRuntimeSubAgentEngineFallbackWhenRuntimeUnavailable(t *testing.T) { +func TestRuntimeSubAgentEngineUnavailableDepsShouldFailByDefault(t *testing.T) { t.Parallel() policy, err := subagent.DefaultRolePolicy(subagent.RoleReviewer) @@ -221,14 +275,8 @@ func TestRuntimeSubAgentEngineFallbackWhenRuntimeUnavailable(t *testing.T) { Policy: policy, Task: subagent.Task{ID: "task-fallback", Goal: "review"}, }) - if err != nil { - t.Fatalf("RunStep() error = %v", err) - } - if !step.Done { - t.Fatalf("expected fallback step done") - } - if step.Output.Summary != "review" { - t.Fatalf("summary = %q, want %q", step.Output.Summary, "review") + if err == nil || !errors.Is(err, errSubAgentRuntimeUnavailable) { + t.Fatalf("expected runtime unavailable error, got output=%+v err=%v", step, err) } } @@ -258,6 +306,133 @@ func TestRuntimeSubAgentEngineRunStepProviderBuildFailureReturnsError(t *testing } } +func TestRuntimeSubAgentEngineDisabledModeRejectsAnyToolCall(t *testing.T) { + t.Parallel() + + manager := newRuntimeConfigManager(t) + providerImpl := &scriptedProvider{ + responses: []scriptedResponse{ + { + Message: providertypes.Message{ + Role: providertypes.RoleAssistant, + ToolCalls: []providertypes.ToolCall{ + {ID: "call-1", Name: "filesystem_read_file", Arguments: `{"path":"README.md"}`}, + }, + }, + FinishReason: "tool_calls", + }, + }, + } + service := NewWithFactory( + manager, + &stubToolManager{specs: []providertypes.ToolSpec{{Name: "filesystem_read_file"}}}, + newMemoryStore(), + &scriptedProviderFactory{provider: providerImpl}, + nil, + ) + policy, err := subagent.DefaultRolePolicy(subagent.RoleCoder) + if err != nil { + t.Fatalf("DefaultRolePolicy() error = %v", err) + } + policy.ToolUseMode = subagent.ToolUseModeDisabled + engine := runtimeSubAgentEngine{service: service, role: subagent.RoleCoder, policy: policy} + stepInput := newRuntimeSubAgentStepInput(t, service, policy, subagent.Task{ID: "task-disabled", Goal: "no tools"}) + + _, err = engine.RunStep(context.Background(), stepInput) + if err == nil || !strings.Contains(err.Error(), "tool_use_mode is disabled") { + t.Fatalf("expected disabled mode error, got %v", err) + } +} + +func TestRuntimeSubAgentEngineMaxToolCallsExceeded(t *testing.T) { + t.Parallel() + + manager := newRuntimeConfigManager(t) + providerImpl := &scriptedProvider{ + responses: []scriptedResponse{ + { + Message: providertypes.Message{ + Role: providertypes.RoleAssistant, + ToolCalls: []providertypes.ToolCall{ + {ID: "call-1", Name: "filesystem_read_file", Arguments: `{"path":"README.md"}`}, + }, + }, + FinishReason: "tool_calls", + }, + { + Message: providertypes.Message{ + Role: providertypes.RoleAssistant, + ToolCalls: []providertypes.ToolCall{ + {ID: "call-2", Name: "filesystem_read_file", Arguments: `{"path":"README.md"}`}, + }, + }, + FinishReason: "tool_calls", + }, + }, + } + toolManager := &stubToolManager{ + specs: []providertypes.ToolSpec{{Name: "filesystem_read_file", Schema: map[string]any{"type": "object"}}}, + executeFn: func(ctx context.Context, input tools.ToolCallInput) (tools.ToolResult, error) { + return tools.ToolResult{ToolCallID: input.ID, Name: input.Name, Content: "ok"}, nil + }, + } + service := NewWithFactory(manager, toolManager, newMemoryStore(), &scriptedProviderFactory{provider: providerImpl}, nil) + policy, err := subagent.DefaultRolePolicy(subagent.RoleCoder) + if err != nil { + t.Fatalf("DefaultRolePolicy() error = %v", err) + } + policy.MaxToolCallsPerStep = 1 + engine := runtimeSubAgentEngine{service: service, role: subagent.RoleCoder, policy: policy} + stepInput := newRuntimeSubAgentStepInput(t, service, policy, subagent.Task{ID: "task-max-calls", Goal: "call twice"}) + stepInput.Budget = subagent.Budget{MaxSteps: 4} + stepInput.Capability = subagent.Capability{AllowedTools: []string{"filesystem_read_file"}} + + _, err = engine.RunStep(context.Background(), stepInput) + if err == nil || !strings.Contains(err.Error(), "max_tool_calls_per_step exceeded") { + t.Fatalf("expected max tool calls exceeded error, got %v", err) + } +} + +func TestRuntimeSubAgentEngineAbortOnNonRecoverableToolError(t *testing.T) { + t.Parallel() + + manager := newRuntimeConfigManager(t) + providerImpl := &scriptedProvider{ + responses: []scriptedResponse{ + { + Message: providertypes.Message{ + Role: providertypes.RoleAssistant, + ToolCalls: []providertypes.ToolCall{ + {ID: "call-1", Name: "filesystem_read_file", Arguments: `{"path":"README.md"}`}, + }, + }, + FinishReason: "tool_calls", + }, + { + Message: providertypes.Message{ + Role: providertypes.RoleAssistant, + Parts: []providertypes.ContentPart{providertypes.NewTextPart(subAgentDonePayload)}, + }, + FinishReason: "stop", + }, + }, + } + service := NewWithFactory(manager, &stubToolManager{}, newMemoryStore(), &scriptedProviderFactory{provider: providerImpl}, nil) + policy, err := subagent.DefaultRolePolicy(subagent.RoleCoder) + if err != nil { + t.Fatalf("DefaultRolePolicy() error = %v", err) + } + engine := runtimeSubAgentEngine{service: service, role: subagent.RoleCoder, policy: policy} + stepInput := newRuntimeSubAgentStepInput(t, service, policy, subagent.Task{ID: "task-non-recoverable", Goal: "tool fail"}) + stepInput.Capability = subagent.Capability{AllowedTools: []string{"filesystem_read_file"}} + stepInput.Executor = failingToolExecutor{err: errors.New("manager unavailable")} + + _, err = engine.RunStep(context.Background(), stepInput) + if err == nil || !strings.Contains(err.Error(), "manager unavailable") { + t.Fatalf("expected non-recoverable tool error, got %v", err) + } +} + func TestParseSubAgentOutput(t *testing.T) { t.Parallel() @@ -309,6 +484,32 @@ func TestParseSubAgentOutput(t *testing.T) { } } +func TestEmitCapabilityDeniedEventRespectsContextCancellation(t *testing.T) { + t.Parallel() + + service := &Service{events: make(chan RuntimeEvent, 1)} + service.events <- RuntimeEvent{Type: EventSubAgentProgress} + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + done := make(chan struct{}, 1) + go func() { + emitCapabilityDeniedEvent(ctx, service, subagent.StepInput{ + RunID: "run-cap-denied-canceled", + SessionID: "session-cap-denied-canceled", + Role: subagent.RoleCoder, + Task: subagent.Task{ID: "task-cap-denied-canceled"}, + }, "bash") + done <- struct{}{} + }() + + select { + case <-done: + case <-time.After(2 * time.Second): + t.Fatalf("emitCapabilityDeniedEvent() blocked when context is canceled") + } +} + func assertSubAgentToolEventPayload( t *testing.T, events []RuntimeEvent, @@ -355,3 +556,30 @@ func newRuntimeSubAgentStepInput( Executor: newSubAgentRuntimeToolExecutor(service), } } + +type failingToolExecutor struct { + err error +} + +func (f failingToolExecutor) ListToolSpecs( + ctx context.Context, + input subagent.ToolSpecListInput, +) ([]providertypes.ToolSpec, error) { + _ = ctx + _ = input + return []providertypes.ToolSpec{{Name: "filesystem_read_file", Schema: map[string]any{"type": "object"}}}, nil +} + +func (f failingToolExecutor) ExecuteTool( + ctx context.Context, + input subagent.ToolExecutionInput, +) (subagent.ToolExecutionResult, error) { + _ = ctx + _ = input + return subagent.ToolExecutionResult{ + Name: "filesystem_read_file", + Content: "", + Decision: "error", + IsError: true, + }, f.err +} diff --git a/internal/runtime/subagent_helpers_test.go b/internal/runtime/subagent_helpers_test.go new file mode 100644 index 00000000..b28743bf --- /dev/null +++ b/internal/runtime/subagent_helpers_test.go @@ -0,0 +1,160 @@ +package runtime + +import ( + "context" + "errors" + "testing" + "time" + + providertypes "neo-code/internal/provider/types" + "neo-code/internal/subagent" +) + +func TestSubAgentEngineHelperFunctions(t *testing.T) { + t.Parallel() + + assistant := ensureAssistantRole(providertypes.Message{}) + if assistant.Role != providertypes.RoleAssistant { + t.Fatalf("role = %q, want assistant", assistant.Role) + } + explicit := ensureAssistantRole(providertypes.Message{Role: providertypes.RoleUser}) + if explicit.Role != providertypes.RoleUser { + t.Fatalf("existing role should be preserved") + } + + if got := resolveSubAgentMaxTurns(0); got != subAgentMaxStepTurnsDefault { + t.Fatalf("resolveSubAgentMaxTurns(0) = %d", got) + } + if got := resolveSubAgentMaxTurns(99); got != subAgentMaxStepTurnsLimit { + t.Fatalf("resolveSubAgentMaxTurns(99) = %d", got) + } + if got := resolveSubAgentMaxTurns(3); got != 3 { + t.Fatalf("resolveSubAgentMaxTurns(3) = %d", got) + } + + if got := effectiveMaxToolCallsPerStep(0); got != subAgentMaxStepTurnsDefault { + t.Fatalf("effectiveMaxToolCallsPerStep(0) = %d", got) + } + if got := effectiveMaxToolCallsPerStep(2); got != 2 { + t.Fatalf("effectiveMaxToolCallsPerStep(2) = %d", got) + } + + allowlist := normalizeToolAllowlist([]string{" Bash ", "bash", "filesystem_read_file"}) + if len(allowlist) != 2 { + t.Fatalf("normalizeToolAllowlist size = %d, want 2", len(allowlist)) + } + if toolAllowed(nil, "bash") { + t.Fatalf("empty allowlist should deny") + } + if !toolAllowed(allowlist, "BASH") { + t.Fatalf("allowlist should match case-insensitive") + } + + call := normalizeSubAgentToolCall(providertypes.ToolCall{Name: " bash ", Arguments: " {}", ID: ""}, 1) + if call.ID == "" || call.Name != "bash" || call.Arguments != "{}" { + t.Fatalf("normalizeSubAgentToolCall() = %+v", call) + } + + if !isRecoverableSubAgentToolError(nil) { + t.Fatalf("nil error should be recoverable") + } + if isRecoverableSubAgentToolError(errors.New("boom")) { + t.Fatalf("generic error should not be recoverable") + } + if !isRecoverableSubAgentToolError(permissionDecisionDenyError(t)) { + t.Fatalf("permission decision error should be recoverable") + } +} + +func TestBuildSubAgentInitialMessagesAndOutputParserEdges(t *testing.T) { + t.Parallel() + + messages := buildSubAgentInitialMessages(subagent.StepInput{ + Task: subagent.Task{ + ID: "task-init", + Goal: "goal", + ExpectedOutput: "expected", + ContextSlice: subagent.TaskContextSlice{ + TaskID: "task-init", + Goal: "context", + }, + }, + Workdir: "/tmp/workdir", + Trace: []string{" one ", "", "two"}, + }) + if len(messages) != 1 { + t.Fatalf("len(messages) = %d, want 1", len(messages)) + } + if text := messages[0].Parts[0].Text; text == "" { + t.Fatalf("expected non-empty initial message") + } + + if _, err := extractSubAgentJSONObject("{\"summary\":"); err == nil { + t.Fatalf("expected incomplete json error") + } + if _, err := extractSubAgentJSONObject("no json"); err == nil { + t.Fatalf("expected missing json error") + } +} + +func TestRuntimeSubAgentResolveSettingsAndToolExecutorEdges(t *testing.T) { + t.Parallel() + + engine := runtimeSubAgentEngine{} + if _, _, _, err := engine.resolveSettings(); err == nil || !errors.Is(err, errSubAgentRuntimeUnavailable) { + t.Fatalf("expected runtime unavailable error, got %v", err) + } + + service := &Service{configManager: newRuntimeConfigManager(t)} + engine = runtimeSubAgentEngine{service: service} + if _, _, _, err := engine.resolveSettings(); err == nil || !errors.Is(err, errSubAgentRuntimeUnavailable) { + t.Fatalf("expected provider factory unavailable error, got %v", err) + } + + executor := newSubAgentRuntimeToolExecutor(nil) + if _, err := executor.ListToolSpecs(context.Background(), subagent.ToolSpecListInput{}); err == nil { + t.Fatalf("expected unavailable executor error") + } +} + +func TestRuntimeSubAgentGenerateStepMessageError(t *testing.T) { + t.Parallel() + + engine := runtimeSubAgentEngine{} + outcome, err := engine.generateStepMessage( + context.Background(), + &scriptedProvider{ + chatFn: func(ctx context.Context, req providertypes.GenerateRequest, events chan<- providertypes.StreamEvent) error { + _ = ctx + _ = req + _ = events + return errors.New("provider error") + }, + }, + "model", + "prompt", + nil, + nil, + ) + if err == nil || outcome.err != nil { + t.Fatalf("expected wrapped provider error, outcome=%+v err=%v", outcome, err) + } +} + +func TestSubAgentToolExecutorUtilityFunctions(t *testing.T) { + t.Parallel() + + if !toolResultTruncated(map[string]any{"truncated": "TRUE"}) { + t.Fatalf("string truncated flag should be recognized") + } + if toolResultTruncated(map[string]any{"truncated": 1}) { + t.Fatalf("unsupported truncated type should be false") + } + + if got := elapsedMilliseconds(time.Time{}); got != 0 { + t.Fatalf("zero start elapsed = %d, want 0", got) + } + if got := elapsedMilliseconds(time.Now().Add(2 * time.Second)); got != 0 { + t.Fatalf("future start elapsed = %d, want 0", got) + } +} diff --git a/internal/runtime/subagent_more_branches_test.go b/internal/runtime/subagent_more_branches_test.go new file mode 100644 index 00000000..f51187d4 --- /dev/null +++ b/internal/runtime/subagent_more_branches_test.go @@ -0,0 +1,183 @@ +package runtime + +import ( + "context" + "errors" + "testing" + + "neo-code/internal/config" + providertypes "neo-code/internal/provider/types" + "neo-code/internal/subagent" +) + +type markerFactory struct{} + +func (markerFactory) Create(role subagent.Role) (subagent.WorkerRuntime, error) { + _ = role + return nil, errors.New("unused") +} + +func TestSubAgentFactoryRegistryBranches(t *testing.T) { + t.Parallel() + + registry := &subAgentFactoryRegistry{ + factory: make(map[*Service]subagent.Factory), + tracked: make(map[*Service]struct{}), + } + service := &Service{} + + registry.ensureTracked(nil) + registry.ensureTracked(service) + registry.ensureTracked(service) + if len(registry.tracked) != 1 { + t.Fatalf("tracked size = %d, want 1", len(registry.tracked)) + } + + if _, ok := registry.get(nil); ok { + t.Fatalf("get(nil) should return not found") + } + registry.set(nil, markerFactory{}) + registry.set(service, markerFactory{}) + if _, ok := registry.get(service); !ok { + t.Fatalf("expected factory set/get to succeed") + } +} + +func TestRunSubAgentTaskInputValidationBranches(t *testing.T) { + t.Parallel() + + service := NewWithFactory(nil, nil, nil, nil, nil) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + if _, err := service.RunSubAgentTask(ctx, SubAgentTaskInput{}); !errors.Is(err, context.Canceled) { + t.Fatalf("expected context canceled, got %v", err) + } + + if _, err := service.RunSubAgentTask(context.Background(), SubAgentTaskInput{ + Role: subagent.RoleCoder, + Task: subagent.Task{ID: "t", Goal: "g"}, + }); err == nil { + t.Fatalf("expected run id required error") + } + + if _, err := service.RunSubAgentTask(context.Background(), SubAgentTaskInput{ + RunID: "run-invalid-role", + Role: subagent.Role("invalid"), + Task: subagent.Task{ID: "t", Goal: "g"}, + }); err == nil { + t.Fatalf("expected invalid role error") + } + + if _, err := service.RunSubAgentTask(context.Background(), SubAgentTaskInput{ + RunID: "run-invalid-task", + Role: subagent.RoleCoder, + Task: subagent.Task{ID: "", Goal: ""}, + }); err == nil { + t.Fatalf("expected invalid task error") + } +} + +func TestSubAgentResultErrorBranches(t *testing.T) { + t.Parallel() + + if err := subAgentResultError(subagent.Result{Error: "explicit error"}); err == nil || err.Error() != "explicit error" { + t.Fatalf("expected explicit error passthrough, got %v", err) + } + if err := subAgentResultError(subagent.Result{State: subagent.StateFailed, StopReason: subagent.StopReasonError}); err == nil { + t.Fatalf("expected synthesized fallback error") + } +} + +func TestSubAgentFactoryNilReceiverBranches(t *testing.T) { + t.Parallel() + + var nilService *Service + nilService.SetSubAgentFactory(markerFactory{}) + if nilService.SubAgentFactory() == nil { + t.Fatalf("nil receiver SubAgentFactory should return default factory") + } +} + +func TestEmitSubAgentFailedNilServiceNoPanic(t *testing.T) { + t.Parallel() + + emitSubAgentFailed(nil, context.Background(), "run", "session", subagent.RoleCoder, "task", errors.New("boom")) +} + +func TestSubAgentRuntimeToolExecutorListToolSpecsError(t *testing.T) { + t.Parallel() + + service := NewWithFactory( + newRuntimeConfigManager(t), + &stubToolManager{listErr: errors.New("list failed")}, + newMemoryStore(), + &scriptedProviderFactory{provider: &scriptedProvider{}}, + nil, + ) + executor := newSubAgentRuntimeToolExecutor(service) + if _, err := executor.ListToolSpecs(context.Background(), subagent.ToolSpecListInput{ + SessionID: "session", + Role: subagent.RoleCoder, + AllowedTools: []string{"bash"}, + }); err == nil { + t.Fatalf("expected list specs error") + } +} + +func TestSubAgentRuntimeToolExecutorEmitNilService(t *testing.T) { + t.Parallel() + + executor := &subAgentRuntimeToolExecutor{} + executor.emit(context.Background(), "run", "session", EventSubAgentToolCallStarted, SubAgentToolCallEventPayload{ + Role: subagent.RoleCoder, + TaskID: "task", + ToolName: "bash", + }) +} + +func TestRuntimeSubAgentResolveSettingsModelFallbackAndEmptyModel(t *testing.T) { + t.Parallel() + + manager := newRuntimeConfigManager(t) + if err := manager.Update(context.Background(), func(cfg *config.Config) error { + cfg.CurrentModel = "" + for i := range cfg.Providers { + cfg.Providers[i].Model = "model-from-provider" + } + return nil + }); err != nil { + t.Fatalf("manager.Update() error = %v", err) + } + + engine := runtimeSubAgentEngine{ + service: &Service{ + configManager: manager, + providerFactory: &scriptedProviderFactory{provider: &scriptedProvider{}}, + }, + } + _, model, _, err := engine.resolveSettings() + if err != nil { + t.Fatalf("resolveSettings() error = %v", err) + } + if model != "model-from-provider" { + t.Fatalf("model = %q, want model-from-provider", model) + } +} + +func TestSubAgentToolResultToMessageFallbackName(t *testing.T) { + t.Parallel() + + msg := subAgentToolResultToMessage(providertypes.ToolCall{ID: "call-1", Name: "bash"}, subagent.ToolExecutionResult{ + Name: "", + Content: "ok", + Decision: permissionDecisionAllow, + }) + if msg.ToolCallID != "call-1" { + t.Fatalf("tool call id = %q, want call-1", msg.ToolCallID) + } + toolName := msg.ToolMetadata["tool_name"] + if toolName != "bash" { + t.Fatalf("tool_name metadata = %q, want bash", toolName) + } +} diff --git a/internal/runtime/subagent_run.go b/internal/runtime/subagent_run.go index 3e51e9ae..ab38bfba 100644 --- a/internal/runtime/subagent_run.go +++ b/internal/runtime/subagent_run.go @@ -220,9 +220,6 @@ func subAgentResultError(result subagent.Result) error { // resolveSubAgentExecutionAgentID 生成子代理执行身份，供权限链路透传审计。 func resolveSubAgentExecutionAgentID(input SubAgentTaskInput) string { - if agentID := strings.TrimSpace(input.AgentID); agentID != "" { - return agentID - } role := strings.TrimSpace(string(input.Role)) taskID := strings.TrimSpace(input.Task.ID) if role == "" { diff --git a/internal/runtime/subagent_run_test.go b/internal/runtime/subagent_run_test.go index 81770a56..8526e481 100644 --- a/internal/runtime/subagent_run_test.go +++ b/internal/runtime/subagent_run_test.go @@ -286,6 +286,51 @@ func TestServiceRunSubAgentTaskFailureFlows(t *testing.T) { }) } +func TestResolveSubAgentExecutionAgentID(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + input SubAgentTaskInput + want string + }{ + { + name: "ignore untrusted external agent id", + input: SubAgentTaskInput{ + AgentID: "forged-agent", + Role: subagent.RoleCoder, + Task: subagent.Task{ID: "task-1"}, + }, + want: "coder:task-1", + }, + { + name: "empty task id falls back to role", + input: SubAgentTaskInput{ + Role: subagent.RoleReviewer, + }, + want: "reviewer", + }, + { + name: "empty role and task id falls back to subagent", + input: SubAgentTaskInput{ + Role: subagent.Role(""), + }, + want: "subagent", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := resolveSubAgentExecutionAgentID(tt.input) + if got != tt.want { + t.Fatalf("resolveSubAgentExecutionAgentID() = %q, want %q", got, tt.want) + } + }) + } +} + type stubSubAgentFactory struct { create func(role subagent.Role) (subagent.WorkerRuntime, error) } diff --git a/internal/runtime/subagent_tool_executor.go b/internal/runtime/subagent_tool_executor.go index 23214724..4d14b8d8 100644 --- a/internal/runtime/subagent_tool_executor.go +++ b/internal/runtime/subagent_tool_executor.go @@ -72,7 +72,7 @@ func (e *subAgentRuntimeToolExecutor) ExecuteTool( Decision: subAgentToolDecisionPending, ElapsedMS: 0, } - e.emit(runID, sessionID, EventSubAgentToolCallStarted, payload) + e.emit(ctx, runID, sessionID, EventSubAgentToolCallStarted, payload) result, execErr := e.service.executeToolCallWithPermission(ctx, permissionExecutionInput{ RunID: runID, @@ -124,7 +124,7 @@ func (e *subAgentRuntimeToolExecutor) ExecuteTool( if strings.EqualFold(decision, permissionDecisionDeny) || strings.EqualFold(decision, stringPermissionDecisionAsk) { eventType = EventSubAgentToolCallDenied } - e.emit(runID, sessionID, eventType, eventPayload) + e.emit(ctx, runID, sessionID, eventType, eventPayload) return output, execErr } @@ -141,23 +141,27 @@ func resolveToolExecutionDecision(execErr error) string { } // emit 发出子代理工具调用事件，失败路径按 best-effort 忽略。 -func (e *subAgentRuntimeToolExecutor) emit(runID string, sessionID string, eventType EventType, payload SubAgentToolCallEventPayload) { +func (e *subAgentRuntimeToolExecutor) emit( + ctx context.Context, + runID string, + sessionID string, + eventType EventType, + payload SubAgentToolCallEventPayload, +) { if e == nil || e.service == nil { return } - _ = e.service.emit(context.Background(), eventType, strings.TrimSpace(runID), strings.TrimSpace(sessionID), payload) + _ = e.service.emit(ctx, eventType, strings.TrimSpace(runID), strings.TrimSpace(sessionID), payload) } -// filterToolSpecsByAllowlist 按工具名白名单过滤 schema 列表，白名单为空时返回全量。 +// filterToolSpecsByAllowlist 按工具名白名单过滤 schema 列表，白名单为空时默认拒绝全部。 func filterToolSpecsByAllowlist(specs []providertypes.ToolSpec, allowlist []string) []providertypes.ToolSpec { if len(specs) == 0 { return nil } normalizedAllowlist := normalizeAllowlist(allowlist) if len(normalizedAllowlist) == 0 { - result := make([]providertypes.ToolSpec, len(specs)) - copy(result, specs) - return result + return nil } filtered := make([]providertypes.ToolSpec, 0, len(specs)) for _, spec := range specs { diff --git a/internal/runtime/subagent_tool_executor_test.go b/internal/runtime/subagent_tool_executor_test.go index 416f1f23..8efa021f 100644 --- a/internal/runtime/subagent_tool_executor_test.go +++ b/internal/runtime/subagent_tool_executor_test.go @@ -3,6 +3,7 @@ package runtime import ( "context" "errors" + "strings" "testing" "time" @@ -34,10 +35,10 @@ func TestSubAgentRuntimeToolExecutorListToolSpecs(t *testing.T) { allow []string wantSize int }{ - {name: "no allowlist", allow: nil, wantSize: 2}, + {name: "no allowlist", allow: nil, wantSize: 0}, {name: "single allowlist", allow: []string{"bash"}, wantSize: 1}, {name: "case-insensitive allowlist", allow: []string{"FILESYSTEM_READ_FILE"}, wantSize: 1}, - {name: "empty allowlist behaves as full set", allow: []string{""}, wantSize: 2}, + {name: "empty allowlist denies all", allow: []string{""}, wantSize: 0}, } for _, tt := range tests { @@ -161,4 +162,118 @@ func TestSubAgentRuntimeToolExecutorExecuteToolEvents(t *testing.T) { assertEventSequence(t, events, []EventType{EventSubAgentToolCallStarted, EventSubAgentToolCallDenied}) assertSubAgentToolEventPayload(t, events, EventSubAgentToolCallDenied, "bash", string(security.DecisionDeny), false) }) + + t.Run("non-permission error should include elapsed and error payload", func(t *testing.T) { + t.Parallel() + + service := NewWithFactory( + newRuntimeConfigManager(t), + &stubToolManager{ + executeFn: func(ctx context.Context, input tools.ToolCallInput) (tools.ToolResult, error) { + _ = ctx + _ = input + return tools.ToolResult{}, errors.New("tool manager down") + }, + }, + newMemoryStore(), + &scriptedProviderFactory{provider: &scriptedProvider{}}, + nil, + ) + executor := newSubAgentRuntimeToolExecutor(service) + _, execErr := executor.ExecuteTool(context.Background(), subagent.ToolExecutionInput{ + RunID: "run-subagent-tool-error", + SessionID: "session-subagent-tool-error", + TaskID: "task-subagent-tool-error", + Role: subagent.RoleCoder, + AgentID: "subagent:error", + Workdir: t.TempDir(), + Timeout: 2 * time.Second, + Call: providertypes.ToolCall{ + ID: "call-error", + Name: "filesystem_read_file", + Arguments: `{"path":"README.md"}`, + }, + }) + if execErr == nil { + t.Fatalf("expected execution error") + } + + events := collectRuntimeEvents(service.Events()) + assertEventSequence(t, events, []EventType{EventSubAgentToolCallStarted, EventSubAgentToolCallResult}) + for _, evt := range events { + if evt.Type != EventSubAgentToolCallResult { + continue + } + payload, ok := evt.Payload.(SubAgentToolCallEventPayload) + if !ok { + t.Fatalf("payload type = %T, want SubAgentToolCallEventPayload", evt.Payload) + } + if payload.ElapsedMS < 0 { + t.Fatalf("elapsed_ms = %d, want >= 0", payload.ElapsedMS) + } + if !strings.Contains(payload.Error, "tool manager down") { + t.Fatalf("error = %q, want contain tool manager down", payload.Error) + } + return + } + t.Fatalf("result event not found") + }) +} + +func TestSubAgentToolEventEmitRespectsContextCancellation(t *testing.T) { + t.Parallel() + + service := NewWithFactory( + newRuntimeConfigManager(t), + &stubToolManager{ + executeFn: func(ctx context.Context, input tools.ToolCallInput) (tools.ToolResult, error) { + _ = input + if err := ctx.Err(); err != nil { + return tools.ToolResult{}, err + } + return tools.ToolResult{ + ToolCallID: input.ID, + Name: input.Name, + Content: "ok", + }, nil + }, + }, + newMemoryStore(), + &scriptedProviderFactory{provider: &scriptedProvider{}}, + nil, + ) + service.events = make(chan RuntimeEvent, 1) + service.events <- RuntimeEvent{Type: EventSubAgentProgress} + executor := newSubAgentRuntimeToolExecutor(service) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + done := make(chan error, 1) + go func() { + _, err := executor.ExecuteTool(ctx, subagent.ToolExecutionInput{ + RunID: "run-subagent-tool-canceled", + SessionID: "session-subagent-tool-canceled", + TaskID: "task-subagent-tool-canceled", + Role: subagent.RoleCoder, + AgentID: "subagent:canceled", + Workdir: t.TempDir(), + Timeout: 2 * time.Second, + Call: providertypes.ToolCall{ + ID: "call-canceled", + Name: "filesystem_read_file", + Arguments: `{"path":"README.md"}`, + }, + }) + done <- err + }() + + select { + case err := <-done: + if err == nil || !errors.Is(err, context.Canceled) { + t.Fatalf("expected context canceled error, got %v", err) + } + case <-time.After(2 * time.Second): + t.Fatalf("ExecuteTool() blocked when event channel is full and context canceled") + } } diff --git a/internal/subagent/factory_test.go b/internal/subagent/factory_test.go new file mode 100644 index 00000000..622ab216 --- /dev/null +++ b/internal/subagent/factory_test.go @@ -0,0 +1,70 @@ +package subagent + +import ( + "context" + "testing" +) + +func TestApplyFactoryOptionsAndExecutionContext(t *testing.T) { + t.Parallel() + + policy, err := DefaultRolePolicy(RoleCoder) + if err != nil { + t.Fatalf("DefaultRolePolicy() error = %v", err) + } + + executor := noopToolExecutor{} + var capturedExecutor ToolExecutor + factory := NewWorkerFactory(func(role Role, policy RolePolicy) Engine { + _ = role + _ = policy + return EngineFunc(func(ctx context.Context, input StepInput) (StepOutput, error) { + _ = ctx + capturedExecutor = input.Executor + return StepOutput{ + Done: true, + Output: Output{ + Summary: "ok", + Findings: []string{"f1"}, + Patches: []string{"p1"}, + Risks: []string{"r1"}, + NextActions: []string{"n1"}, + Artifacts: []string{"a1"}, + }, + }, nil + }) + }, nil, WithExecutionContext(ExecutionContext{ToolExecutor: executor})) + worker, err := factory.Create(RoleCoder) + if err != nil { + t.Fatalf("Create() error = %v", err) + } + if err := worker.Start(Task{ID: "task-factory-ctx", Goal: "goal"}, policy.DefaultBudget, Capability{}); err != nil { + t.Fatalf("Start() error = %v", err) + } + if _, err := worker.Step(context.Background()); err != nil { + t.Fatalf("Step() error = %v", err) + } + if capturedExecutor == nil { + t.Fatalf("expected execution context tool executor to be injected") + } +} + +func TestNewWorkerFactoryNilBuilderUsesDefaultEngine(t *testing.T) { + t.Parallel() + + factory := NewWorkerFactory(nil) + worker, err := factory.Create(RoleReviewer) + if err != nil { + t.Fatalf("Create() error = %v", err) + } + if err := worker.Start(Task{ID: "task-default-engine", Goal: "review"}, Budget{MaxSteps: 1}, Capability{}); err != nil { + t.Fatalf("Start() error = %v", err) + } + step, err := worker.Step(context.Background()) + if err != nil { + t.Fatalf("Step() error = %v", err) + } + if !step.Done { + t.Fatalf("default engine should finish in one step") + } +} diff --git a/internal/subagent/scheduler_types_additional_test.go b/internal/subagent/scheduler_types_additional_test.go new file mode 100644 index 00000000..61c25534 --- /dev/null +++ b/internal/subagent/scheduler_types_additional_test.go @@ -0,0 +1,26 @@ +package subagent + +import ( + "testing" + "time" +) + +func TestDefaultRetryBackoffWithBounds(t *testing.T) { + t.Parallel() + + backoff := defaultRetryBackoffWithBounds(0, 0) + if got := backoff(0); got != 0 { + t.Fatalf("attempt=0 delay = %v, want 0", got) + } + if got := backoff(1); got != time.Second { + t.Fatalf("attempt=1 delay = %v, want %v", got, time.Second) + } + if got := backoff(10); got != 30*time.Second { + t.Fatalf("attempt=10 delay = %v, want %v", got, 30*time.Second) + } + + bounded := defaultRetryBackoffWithBounds(5*time.Second, 2*time.Second) + if got := bounded(2); got != 5*time.Second { + t.Fatalf("max 0") + } +} diff --git a/internal/subagent/util_test.go b/internal/subagent/util_test.go new file mode 100644 index 00000000..80f449cc --- /dev/null +++ b/internal/subagent/util_test.go @@ -0,0 +1,24 @@ +package subagent + +import "testing" + +func TestDedupeAndTrim(t *testing.T) { + t.Parallel() + + if got := dedupeAndTrim(nil); got != nil { + t.Fatalf("dedupeAndTrim(nil) = %#v, want nil", got) + } + if got := dedupeAndTrim([]string{" ", "\t"}); got != nil { + t.Fatalf("dedupeAndTrim(empty) = %#v, want nil", got) + } + got := dedupeAndTrim([]string{" Bash ", "bash", "FS", "fs", "Go "}) + want := []string{"Bash", "FS", "Go"} + if len(got) != len(want) { + t.Fatalf("len(got)=%d, want %d", len(got), len(want)) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("got[%d]=%q, want %q", i, got[i], want[i]) + } + } +} diff --git a/internal/subagent/worker_test.go b/internal/subagent/worker_test.go index 85bd91b1..0b3f6d7d 100644 --- a/internal/subagent/worker_test.go +++ b/internal/subagent/worker_test.go @@ -259,6 +259,41 @@ func TestWorkerRejectsInvalidOutputContract(t *testing.T) { } } +func TestValidateDefaultWorkspacePath(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + workspace string + wantErr bool + }{ + {name: "current directory", workspace: ".", wantErr: true}, + {name: "filesystem root", workspace: string(filepath.Separator), wantErr: true}, + {name: "normal path", workspace: filepath.Join(string(filepath.Separator), "tmp", "workspace"), wantErr: false}, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := validateDefaultWorkspacePath(tt.workspace) + if tt.wantErr && err == nil { + t.Fatalf("expected error for workspace=%q", tt.workspace) + } + if !tt.wantErr && err != nil { + t.Fatalf("unexpected error for workspace=%q: %v", tt.workspace, err) + } + }) + } +} + +func TestWithExecutionContextOnNilWorker(t *testing.T) { + t.Parallel() + + opt := withExecutionContext(ExecutionContext{ToolExecutor: noopToolExecutor{}}) + opt(nil) +} + func TestWorkerStartCapabilityPolicyGuard(t *testing.T) { t.Parallel() From 82031ec33b03fd2dc33ab3a9d29269dd5473891a Mon Sep 17 00:00:00 2001 From: xgopilot Date: Sat, 18 Apr 2026 10:34:13 +0000 Subject: [PATCH 5/5] fix(subagent): align permission rejection and policy limits semantics Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: Cai-Tang-www <106404101+Cai-Tang-www@users.noreply.github.com> --- internal/runtime/subagent_engine.go | 25 +++- internal/runtime/subagent_engine_test.go | 111 +++++++++++++++++- internal/runtime/subagent_helpers_test.go | 19 ++- internal/runtime/subagent_tool_executor.go | 3 + .../runtime/subagent_tool_executor_test.go | 49 ++++++++ internal/subagent/execution_context_test.go | 1 + internal/subagent/factory_test.go | 48 ++++++++ internal/subagent/policy.go | 4 +- internal/subagent/policy_test.go | 62 +++++++--- internal/subagent/worker_test.go | 24 ++-- 10 files changed, 309 insertions(+), 37 deletions(-) diff --git a/internal/runtime/subagent_engine.go b/internal/runtime/subagent_engine.go index 73175e93..5494160d 100644 --- a/internal/runtime/subagent_engine.go +++ b/internal/runtime/subagent_engine.go @@ -97,7 +97,7 @@ func (e runtimeSubAgentEngine) RunStep(ctx context.Context, input subagent.StepI systemPrompt := buildSubAgentSystemPrompt(input.Policy, allowedTools) messages := buildSubAgentInitialMessages(input) totalToolCalls := 0 - maxTurns := resolveSubAgentMaxTurns(input.Budget.MaxSteps) + maxTurns := resolveSubAgentMaxTurns(input.Policy.DefaultBudget.MaxSteps) for turn := 1; turn <= maxTurns; turn++ { outcome, err := e.generateStepMessage(ctx, modelProvider, model, systemPrompt, messages, toolSpecs) @@ -306,6 +306,7 @@ func buildSubAgentInitialMessages(input subagent.StepInput) []providertypes.Mess // buildSubAgentSystemPrompt 构建子代理策略提示词，约束工具决策和输出契约。 func buildSubAgentSystemPrompt(policy subagent.RolePolicy, allowedTools []string) string { + maxToolCallsPerStep := effectiveMaxToolCallsPerStep(policy.MaxToolCallsPerStep) lines := []string{strings.TrimSpace(policy.SystemPrompt)} lines = append(lines, "你是子代理执行引擎的一部分，必须根据任务目标自主决定是否调用工具。", @@ -313,7 +314,7 @@ func buildSubAgentSystemPrompt(policy subagent.RolePolicy, allowedTools []string "工具失败后优先换参数或换工具，若仍失败则在输出中明确风险与后续动作。", "最终输出必须是 JSON 对象，且必须包含键：summary, findings, patches, risks, next_actions, artifacts。", fmt.Sprintf("tool_use_mode: %s", policy.ToolUseMode), - fmt.Sprintf("max_tool_calls_per_step: %d", policy.MaxToolCallsPerStep), + fmt.Sprintf("max_tool_calls_per_step: %d", maxToolCallsPerStep), ) if len(allowedTools) > 0 { lines = append(lines, "allowed_tools: "+strings.Join(allowedTools, ", ")) @@ -340,10 +341,10 @@ func resolveSubAgentMaxTurns(maxSteps int) int { return maxSteps } -// effectiveMaxToolCallsPerStep 解析每步工具调用上限，并为零值回填默认值。 +// effectiveMaxToolCallsPerStep 解析每步工具调用上限，非法或未配置值按 0 处理（即不允许调用）。 func effectiveMaxToolCallsPerStep(limit int) int { if limit <= 0 { - return subAgentMaxStepTurnsDefault + return 0 } return limit } @@ -489,7 +490,21 @@ func isRecoverableSubAgentToolError(err error) bool { return true } var permissionErr *tools.PermissionDecisionError - return errors.As(err, &permissionErr) + if errors.As(err, &permissionErr) { + return true + } + return isSubAgentPermissionDeniedError(err) +} + +// isSubAgentPermissionDeniedError 判断错误是否属于权限拒绝语义（含 ask->reject 的文本错误）。 +func isSubAgentPermissionDeniedError(err error) bool { + if err == nil { + return false + } + if errors.Is(err, tools.ErrPermissionDenied) { + return true + } + return strings.EqualFold(strings.TrimSpace(err.Error()), permissionRejectedErrorMessage) } // subagentCapabilityDeniedResult 构造 capability 越权时回灌给模型的标准 tool 消息。 diff --git a/internal/runtime/subagent_engine_test.go b/internal/runtime/subagent_engine_test.go index f253af26..329f66e3 100644 --- a/internal/runtime/subagent_engine_test.go +++ b/internal/runtime/subagent_engine_test.go @@ -433,6 +433,108 @@ func TestRuntimeSubAgentEngineAbortOnNonRecoverableToolError(t *testing.T) { } } +func TestRuntimeSubAgentEngineAskRejectIsRecoverable(t *testing.T) { + t.Parallel() + + manager := newRuntimeConfigManager(t) + providerImpl := &scriptedProvider{ + responses: []scriptedResponse{ + { + Message: providertypes.Message{ + Role: providertypes.RoleAssistant, + ToolCalls: []providertypes.ToolCall{ + {ID: "call-1", Name: "filesystem_read_file", Arguments: `{"path":"README.md"}`}, + }, + }, + FinishReason: "tool_calls", + }, + { + Message: providertypes.Message{ + Role: providertypes.RoleAssistant, + Parts: []providertypes.ContentPart{providertypes.NewTextPart(subAgentDonePayload)}, + }, + FinishReason: "stop", + }, + }, + } + service := NewWithFactory(manager, &stubToolManager{}, newMemoryStore(), &scriptedProviderFactory{provider: providerImpl}, nil) + policy, err := subagent.DefaultRolePolicy(subagent.RoleCoder) + if err != nil { + t.Fatalf("DefaultRolePolicy() error = %v", err) + } + engine := runtimeSubAgentEngine{service: service, role: subagent.RoleCoder, policy: policy} + stepInput := newRuntimeSubAgentStepInput(t, service, policy, subagent.Task{ID: "task-ask-reject", Goal: "tool reject should continue"}) + stepInput.Capability = subagent.Capability{AllowedTools: []string{"filesystem_read_file"}} + stepInput.Executor = failingToolExecutor{ + err: errors.New(permissionRejectedErrorMessage), + decision: permissionDecisionDeny, + } + + output, err := engine.RunStep(context.Background(), stepInput) + if err != nil { + t.Fatalf("RunStep() error = %v", err) + } + if !output.Done { + t.Fatalf("expected step done") + } + if output.Output.Summary != "done" { + t.Fatalf("summary = %q, want done", output.Output.Summary) + } +} + +func TestRuntimeSubAgentEngineLowWorkerBudgetStillAllowsToolClosure(t *testing.T) { + t.Parallel() + + manager := newRuntimeConfigManager(t) + providerImpl := &scriptedProvider{ + responses: []scriptedResponse{ + { + Message: providertypes.Message{ + Role: providertypes.RoleAssistant, + ToolCalls: []providertypes.ToolCall{ + {ID: "call-1", Name: "filesystem_read_file", Arguments: `{"path":"README.md"}`}, + }, + }, + FinishReason: "tool_calls", + }, + { + Message: providertypes.Message{ + Role: providertypes.RoleAssistant, + Parts: []providertypes.ContentPart{providertypes.NewTextPart(subAgentDonePayload)}, + }, + FinishReason: "stop", + }, + }, + } + toolManager := &stubToolManager{ + specs: []providertypes.ToolSpec{{Name: "filesystem_read_file", Schema: map[string]any{"type": "object"}}}, + executeFn: func(ctx context.Context, input tools.ToolCallInput) (tools.ToolResult, error) { + _ = ctx + return tools.ToolResult{ToolCallID: input.ID, Name: input.Name, Content: "ok"}, nil + }, + } + service := NewWithFactory(manager, toolManager, newMemoryStore(), &scriptedProviderFactory{provider: providerImpl}, nil) + policy, err := subagent.DefaultRolePolicy(subagent.RoleCoder) + if err != nil { + t.Fatalf("DefaultRolePolicy() error = %v", err) + } + engine := runtimeSubAgentEngine{service: service, role: subagent.RoleCoder, policy: policy} + stepInput := newRuntimeSubAgentStepInput(t, service, policy, subagent.Task{ID: "task-low-budget", Goal: "tool closure with low outer budget"}) + stepInput.Budget = subagent.Budget{MaxSteps: 1} + stepInput.Capability = subagent.Capability{AllowedTools: []string{"filesystem_read_file"}} + + output, err := engine.RunStep(context.Background(), stepInput) + if err != nil { + t.Fatalf("RunStep() error = %v", err) + } + if !output.Done { + t.Fatalf("expected step done") + } + if providerImpl.callCount != 2 { + t.Fatalf("provider calls = %d, want 2", providerImpl.callCount) + } +} + func TestParseSubAgentOutput(t *testing.T) { t.Parallel() @@ -558,7 +660,8 @@ func newRuntimeSubAgentStepInput( } type failingToolExecutor struct { - err error + err error + decision string } func (f failingToolExecutor) ListToolSpecs( @@ -576,10 +679,14 @@ func (f failingToolExecutor) ExecuteTool( ) (subagent.ToolExecutionResult, error) { _ = ctx _ = input + decision := strings.TrimSpace(f.decision) + if decision == "" { + decision = "error" + } return subagent.ToolExecutionResult{ Name: "filesystem_read_file", Content: "", - Decision: "error", + Decision: decision, IsError: true, }, f.err } diff --git a/internal/runtime/subagent_helpers_test.go b/internal/runtime/subagent_helpers_test.go index b28743bf..fc59057c 100644 --- a/internal/runtime/subagent_helpers_test.go +++ b/internal/runtime/subagent_helpers_test.go @@ -3,11 +3,13 @@ package runtime import ( "context" "errors" + "fmt" "testing" "time" providertypes "neo-code/internal/provider/types" "neo-code/internal/subagent" + "neo-code/internal/tools" ) func TestSubAgentEngineHelperFunctions(t *testing.T) { @@ -32,8 +34,8 @@ func TestSubAgentEngineHelperFunctions(t *testing.T) { t.Fatalf("resolveSubAgentMaxTurns(3) = %d", got) } - if got := effectiveMaxToolCallsPerStep(0); got != subAgentMaxStepTurnsDefault { - t.Fatalf("effectiveMaxToolCallsPerStep(0) = %d", got) + if got := effectiveMaxToolCallsPerStep(0); got != 0 { + t.Fatalf("effectiveMaxToolCallsPerStep(0) = %d, want 0", got) } if got := effectiveMaxToolCallsPerStep(2); got != 2 { t.Fatalf("effectiveMaxToolCallsPerStep(2) = %d", got) @@ -64,6 +66,15 @@ func TestSubAgentEngineHelperFunctions(t *testing.T) { if !isRecoverableSubAgentToolError(permissionDecisionDenyError(t)) { t.Fatalf("permission decision error should be recoverable") } + if !isRecoverableSubAgentToolError(fmt.Errorf("wrapped: %w", tools.ErrPermissionDenied)) { + t.Fatalf("wrapped permission denied should be recoverable") + } + if !isSubAgentPermissionDeniedError(errors.New(permissionRejectedErrorMessage)) { + t.Fatalf("permission rejected message should be recognized") + } + if isSubAgentPermissionDeniedError(errors.New("other error")) { + t.Fatalf("non-permission error should not be recognized as denied") + } } func TestBuildSubAgentInitialMessagesAndOutputParserEdges(t *testing.T) { @@ -144,6 +155,10 @@ func TestRuntimeSubAgentGenerateStepMessageError(t *testing.T) { func TestSubAgentToolExecutorUtilityFunctions(t *testing.T) { t.Parallel() + if filtered := filterToolSpecsByAllowlist(nil, []string{"bash"}); len(filtered) != 0 { + t.Fatalf("expected empty specs when input is nil") + } + if !toolResultTruncated(map[string]any{"truncated": "TRUE"}) { t.Fatalf("string truncated flag should be recognized") } diff --git a/internal/runtime/subagent_tool_executor.go b/internal/runtime/subagent_tool_executor.go index 4d14b8d8..573c2fbe 100644 --- a/internal/runtime/subagent_tool_executor.go +++ b/internal/runtime/subagent_tool_executor.go @@ -137,6 +137,9 @@ func resolveToolExecutionDecision(execErr error) string { if errors.As(execErr, &permissionErr) { return permissionErr.Decision() } + if isSubAgentPermissionDeniedError(execErr) { + return permissionDecisionDeny + } return "error" } diff --git a/internal/runtime/subagent_tool_executor_test.go b/internal/runtime/subagent_tool_executor_test.go index 8efa021f..df61229f 100644 --- a/internal/runtime/subagent_tool_executor_test.go +++ b/internal/runtime/subagent_tool_executor_test.go @@ -39,6 +39,7 @@ func TestSubAgentRuntimeToolExecutorListToolSpecs(t *testing.T) { {name: "single allowlist", allow: []string{"bash"}, wantSize: 1}, {name: "case-insensitive allowlist", allow: []string{"FILESYSTEM_READ_FILE"}, wantSize: 1}, {name: "empty allowlist denies all", allow: []string{""}, wantSize: 0}, + {name: "unknown tool allowlist", allow: []string{"webfetch"}, wantSize: 0}, } for _, tt := range tests { @@ -163,6 +164,54 @@ func TestSubAgentRuntimeToolExecutorExecuteToolEvents(t *testing.T) { assertSubAgentToolEventPayload(t, events, EventSubAgentToolCallDenied, "bash", string(security.DecisionDeny), false) }) + t.Run("permission reject message should emit denied", func(t *testing.T) { + t.Parallel() + + service := NewWithFactory( + newRuntimeConfigManager(t), + &stubToolManager{ + executeFn: func(ctx context.Context, input tools.ToolCallInput) (tools.ToolResult, error) { + _ = ctx + return tools.ToolResult{ + ToolCallID: input.ID, + Name: input.Name, + Content: "permission rejected", + IsError: true, + }, errors.New(permissionRejectedErrorMessage) + }, + }, + newMemoryStore(), + &scriptedProviderFactory{provider: &scriptedProvider{}}, + nil, + ) + executor := newSubAgentRuntimeToolExecutor(service) + + result, execErr := executor.ExecuteTool(context.Background(), subagent.ToolExecutionInput{ + RunID: "run-subagent-tool-reject", + SessionID: "session-subagent-tool-reject", + TaskID: "task-subagent-tool-reject", + Role: subagent.RoleCoder, + AgentID: "subagent:reject", + Workdir: t.TempDir(), + Timeout: 2 * time.Second, + Call: providertypes.ToolCall{ + ID: "call-reject", + Name: "filesystem_read_file", + Arguments: `{"path":"README.md"}`, + }, + }) + if execErr == nil || !strings.Contains(execErr.Error(), "permission rejected by user") { + t.Fatalf("expected permission rejected error, got %v", execErr) + } + if result.Decision != permissionDecisionDeny { + t.Fatalf("decision = %q, want %q", result.Decision, permissionDecisionDeny) + } + + events := collectRuntimeEvents(service.Events()) + assertEventSequence(t, events, []EventType{EventSubAgentToolCallStarted, EventSubAgentToolCallDenied}) + assertSubAgentToolEventPayload(t, events, EventSubAgentToolCallDenied, "filesystem_read_file", permissionDecisionDeny, false) + }) + t.Run("non-permission error should include elapsed and error payload", func(t *testing.T) { t.Parallel() diff --git a/internal/subagent/execution_context_test.go b/internal/subagent/execution_context_test.go index 7a07bc73..e4274917 100644 --- a/internal/subagent/execution_context_test.go +++ b/internal/subagent/execution_context_test.go @@ -105,6 +105,7 @@ func TestRolePolicyToolDefaultsAndValidation(t *testing.T) { {name: "valid required", mode: ToolUseModeRequired, limit: 2, wantErr: false}, {name: "valid disabled", mode: ToolUseModeDisabled, limit: 3, wantErr: false}, {name: "invalid mode", mode: ToolUseMode("unknown"), limit: 1, wantErr: true}, + {name: "invalid zero limit", mode: ToolUseModeAuto, limit: 0, wantErr: true}, {name: "invalid negative limit", mode: ToolUseModeAuto, limit: -1, wantErr: true}, } diff --git a/internal/subagent/factory_test.go b/internal/subagent/factory_test.go index 622ab216..4b8e1304 100644 --- a/internal/subagent/factory_test.go +++ b/internal/subagent/factory_test.go @@ -68,3 +68,51 @@ func TestNewWorkerFactoryNilBuilderUsesDefaultEngine(t *testing.T) { t.Fatalf("default engine should finish in one step") } } + +func TestWithToolExecutorOptionOverridesExecutionContext(t *testing.T) { + t.Parallel() + + var captured ToolExecutor + factory := NewWorkerFactory(func(role Role, policy RolePolicy) Engine { + _ = role + _ = policy + return EngineFunc(func(ctx context.Context, input StepInput) (StepOutput, error) { + _ = ctx + captured = input.Executor + return StepOutput{ + Done: true, + Output: Output{ + Summary: "ok", + Findings: []string{"f"}, + Patches: []string{"p"}, + Risks: []string{"r"}, + NextActions: []string{"n"}, + Artifacts: []string{"a"}, + }, + }, nil + }) + }, WithExecutionContext(ExecutionContext{}), WithToolExecutor(noopToolExecutor{})) + + worker, err := factory.Create(RoleCoder) + if err != nil { + t.Fatalf("Create() error = %v", err) + } + if err := worker.Start(Task{ID: "task-tool-executor", Goal: "goal"}, Budget{MaxSteps: 1}, Capability{}); err != nil { + t.Fatalf("Start() error = %v", err) + } + if _, err := worker.Step(context.Background()); err != nil { + t.Fatalf("Step() error = %v", err) + } + if captured == nil { + t.Fatalf("expected executor to be injected") + } +} + +func TestFactoryOptionsIgnoreNilReceiver(t *testing.T) { + t.Parallel() + + var nilFactory *WorkerFactory + WithExecutionContext(ExecutionContext{ToolExecutor: noopToolExecutor{}})(nilFactory) + WithToolExecutor(noopToolExecutor{})(nilFactory) + applyFactoryOptions(nilFactory, nil, WithToolExecutor(noopToolExecutor{})) +} diff --git a/internal/subagent/policy.go b/internal/subagent/policy.go index d848391e..ce4d09f1 100644 --- a/internal/subagent/policy.go +++ b/internal/subagent/policy.go @@ -64,8 +64,8 @@ func (p RolePolicy) Validate() error { if !p.ToolUseMode.Valid() { return errorsf("role policy tool use mode %q is invalid", p.ToolUseMode) } - if p.MaxToolCallsPerStep < 0 { - return errorsf("role policy max tool calls per step must not be negative") + if p.MaxToolCallsPerStep <= 0 { + return errorsf("role policy max tool calls per step must be greater than zero") } if _, err := normalizeRequiredSections(p.RequiredSections); err != nil { return err diff --git a/internal/subagent/policy_test.go b/internal/subagent/policy_test.go index 08721d37..efe30669 100644 --- a/internal/subagent/policy_test.go +++ b/internal/subagent/policy_test.go @@ -61,47 +61,73 @@ func TestRolePolicyValidate(t *testing.T) { { name: "valid", policy: RolePolicy{ - Role: RoleResearcher, - SystemPrompt: "prompt", - AllowedTools: []string{"filesystem_grep"}, - RequiredSections: []string{"summary"}, + Role: RoleResearcher, + SystemPrompt: "prompt", + AllowedTools: []string{"filesystem_grep"}, + MaxToolCallsPerStep: 1, + RequiredSections: []string{"summary"}, }, }, { name: "empty prompt", policy: RolePolicy{ - Role: RoleResearcher, - AllowedTools: []string{"filesystem_grep"}, - RequiredSections: []string{"summary"}, + Role: RoleResearcher, + AllowedTools: []string{"filesystem_grep"}, + MaxToolCallsPerStep: 1, + RequiredSections: []string{"summary"}, }, wantErr: true, }, { name: "empty tools", policy: RolePolicy{ - Role: RoleResearcher, - SystemPrompt: "prompt", - RequiredSections: []string{"summary"}, + Role: RoleResearcher, + SystemPrompt: "prompt", + MaxToolCallsPerStep: 1, + RequiredSections: []string{"summary"}, + }, + wantErr: true, + }, + { + name: "empty required sections", + policy: RolePolicy{ + Role: RoleResearcher, + SystemPrompt: "prompt", + AllowedTools: []string{"filesystem_grep"}, + MaxToolCallsPerStep: 1, }, wantErr: true, }, { name: "invalid role", policy: RolePolicy{ - Role: Role("x"), - SystemPrompt: "prompt", - AllowedTools: []string{"filesystem_grep"}, - RequiredSections: []string{"summary"}, + Role: Role("x"), + SystemPrompt: "prompt", + AllowedTools: []string{"filesystem_grep"}, + MaxToolCallsPerStep: 1, + RequiredSections: []string{"summary"}, }, wantErr: true, }, { name: "unsupported required section", policy: RolePolicy{ - Role: RoleResearcher, - SystemPrompt: "prompt", - AllowedTools: []string{"filesystem_grep"}, - RequiredSections: []string{"unknown_section"}, + Role: RoleResearcher, + SystemPrompt: "prompt", + AllowedTools: []string{"filesystem_grep"}, + MaxToolCallsPerStep: 1, + RequiredSections: []string{"unknown_section"}, + }, + wantErr: true, + }, + { + name: "non-positive max tool calls", + policy: RolePolicy{ + Role: RoleResearcher, + SystemPrompt: "prompt", + AllowedTools: []string{"filesystem_grep"}, + MaxToolCallsPerStep: 0, + RequiredSections: []string{"summary"}, }, wantErr: true, }, diff --git a/internal/subagent/worker_test.go b/internal/subagent/worker_test.go index 0b3f6d7d..7e307cea 100644 --- a/internal/subagent/worker_test.go +++ b/internal/subagent/worker_test.go @@ -494,7 +494,13 @@ func TestWorkerNilAndValidationBranches(t *testing.T) { if _, err := NewWorker(Role("bad"), RolePolicy{}, nil); err == nil { t.Fatalf("expected invalid role error") } - if _, err := NewWorker(RoleCoder, RolePolicy{Role: RoleReviewer, SystemPrompt: "p", AllowedTools: []string{"bash"}, RequiredSections: []string{"summary"}}, nil); err == nil { + if _, err := NewWorker(RoleCoder, RolePolicy{ + Role: RoleReviewer, + SystemPrompt: "p", + AllowedTools: []string{"bash"}, + MaxToolCallsPerStep: 1, + RequiredSections: []string{"summary"}, + }, nil); err == nil { t.Fatalf("expected role mismatch error") } @@ -667,9 +673,10 @@ func TestWorkerAdditionalUncoveredBranches(t *testing.T) { t.Run("new worker fills empty policy role", func(t *testing.T) { t.Parallel() policy := RolePolicy{ - SystemPrompt: "review", - AllowedTools: []string{"bash"}, - RequiredSections: []string{"summary"}, + SystemPrompt: "review", + AllowedTools: []string{"bash"}, + MaxToolCallsPerStep: 1, + RequiredSections: []string{"summary"}, } wr, err := NewWorker(RoleReviewer, policy, nil) if err != nil { @@ -683,10 +690,11 @@ func TestWorkerAdditionalUncoveredBranches(t *testing.T) { t.Run("new worker policy validate error", func(t *testing.T) { t.Parallel() _, err := NewWorker(RoleReviewer, RolePolicy{ - Role: RoleReviewer, - SystemPrompt: "", - AllowedTools: []string{"bash"}, - RequiredSections: []string{"summary"}, + Role: RoleReviewer, + SystemPrompt: "", + AllowedTools: []string{"bash"}, + MaxToolCallsPerStep: 1, + RequiredSections: []string{"summary"}, }, nil) if err == nil { t.Fatalf("expected policy validate error")