diff --git a/docs/architecture.md b/docs/architecture.md index 9dda416c..03c6a994 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -23,7 +23,7 @@ Implementation status for this revision: | --- | --- | --- | | Product entry and names | Implemented | One V2 React entry; `AgentRunner`, `SubagentRunner`, `RunWorktreeService` and `APIClient` name their responsibilities. | | Retired code | Removed | Unreachable UI, unused Kernel/Planner/Executor/Critic scaffolding, unused tool Schema descriptions and retired execution constructors were deleted in [PR #245](https://github.com/CWNU-Open-Source-Community/Universal-Code/pull/245). Historical readers remain supported. | -| Built-in tools | Unified | [Tool registry](../internal/toolgateway/registry.go) supplies definitions, names, classes, normalization, model membership and Gateway handlers. Runtime capabilities select the advertised subset. | +| Built-in tools | Unified | [Tool registry](../internal/toolgateway/registry.go) supplies definitions, names, classes, normalization, model membership, executor availability and Gateway handlers. Runtime capabilities select the advertised subset. | | Execution checks | Shared | [Dispatch guard](../internal/executionauth/dispatch_guard.go) handles first dispatch and later rechecks for GitHub review, advanced Git, thread Git and PR operations. Business authorization stays with each operation. | | Command binding | Deduplicated | [Command authority](../internal/application/command_runtime_authority.go) validates one loaded binding snapshot. Each actual execution and later recheck reloads current state. | | Migration fixtures | Simplified | [Historical fixtures](../internal/store/test_database_fixture_test.go) build real migration prefixes. [Seed import](../internal/store/historical_fixture_import_test.go) copies compatible test data; the cumulative reverse-DDL chain is gone. | diff --git a/internal/toolgateway/gateway.go b/internal/toolgateway/gateway.go index 84be976c..fec38df7 100644 --- a/internal/toolgateway/gateway.go +++ b/internal/toolgateway/gateway.go @@ -198,45 +198,14 @@ func (g *Gateway) Invoke(ctx context.Context, call ToolCall) (outcome Outcome, r if normalized.InvocationID != "" { return Outcome{}, errors.New("tool invocation id is generated by the gateway") } - if (normalized.Name == WorkItemCreateTool || normalized.Name == NoteCreateTool) && g.structuredMemory == nil { - return Outcome{}, errors.New("structured memory executor is required") - } - if normalized.Name == SpecialistDelegationProposeTool && g.delegationProposals == nil { - return Outcome{}, errors.New("specialist delegation proposal executor is required") - } - if normalized.Name == ChildTaskProposeTool && g.childTaskProposals == nil { - return Outcome{}, errors.New("child task proposal executor is required") - } - if normalized.Name == PlanDeliveryProposeTool && g.planDeliveryProposals == nil { - return Outcome{}, errors.New("Plan/Delivery proposal executor is required") - } - if normalized.Name == DockerSandboxRunProposeTool && - g.dockerSandboxProposals == nil { - return Outcome{}, errors.New("Docker Sandbox proposal executor is required") - } - if normalized.Name == SkillCandidateProposeTool && g.skillCandidates == nil { - return Outcome{}, errors.New("Skill candidate proposal executor is required") - } - if normalized.Name == DebugTerminalTool && g.debugTerminal == nil { - return Outcome{}, errors.New("debug terminal executor is required") - } - if normalized.Name == CommandRuntimeTool && g.commandRuntime == nil { - return Outcome{}, errors.New("command runtime executor is required") - } - if normalized.Name == MCPToolCallTool && g.mcp == nil { - return Outcome{}, errors.New("MCP client executor is required") - } - if IsWebEvidenceTool(normalized.Name) && g.webEvidence == nil { - return Outcome{}, errors.New("web evidence executor is required") - } - if IsBrowserActionTool(normalized.Name) && ((!IsAgentBrowserPayload(normalized.Payload) && g.browserActions == nil) || (IsAgentBrowserPayload(normalized.Payload) && g.agentBrowser == nil)) { - return Outcome{}, errors.New("browser action executor is required") - } - if isAgentCodeTool(normalized.Name) && g.agentCode == nil { - return Outcome{}, errors.New("agent code tool executor is required") + registration, found := lookupTool(normalized.Name) + if !found || registration.invoke == nil { + return Outcome{}, fmt.Errorf("unsupported tool %q", normalized.Name) } - if IsCodeIntelTool(normalized.Name) && g.codeIntel == nil { - return Outcome{}, errors.New("code-intel tool executor is required") + if registration.preflight != nil { + if err := registration.preflight(g, normalized); err != nil { + return Outcome{}, err + } } fallback := waitgraph.External(normalized.RequestedBy) if normalized.AgentID != "" { @@ -250,12 +219,8 @@ func (g *Gateway) Invoke(ctx context.Context, call ToolCall) (outcome Outcome, r return Outcome{}, fmt.Errorf("register synchronous tool dependency: %w", err) } defer releaseWait() - class, ok := ClassForTool(normalized.Name) - if !ok { - return Outcome{}, fmt.Errorf("unsupported tool %q", normalized.Name) - } if g.budgetStore != nil { - usage, err := g.budgetStore.ChargeToolCall(ctx, toolBudgetRequest(normalized, class)) + usage, err := g.budgetStore.ChargeToolCall(ctx, toolBudgetRequest(normalized, registration.definition.Class)) if err != nil { return Outcome{}, err } @@ -264,10 +229,6 @@ func (g *Gateway) Invoke(ctx context.Context, call ToolCall) (outcome Outcome, r normalized.InvocationID = usage.LastCharge } } - registration, found := lookupTool(normalized.Name) - if !found || registration.invoke == nil { - return Outcome{}, fmt.Errorf("unsupported tool %q", normalized.Name) - } return registration.invoke(g, ctx, normalized) } @@ -816,7 +777,7 @@ func validateToolArguments(call ToolCall) error { call.LeaseID == "" { return errors.New("code-intel tools require a structured fenced root Supervisor scope") } - _, _, err := NormalizeCodeIntelPayload(call.Name, call.Payload) + _, err := NormalizeSupervisorToolPayload(call.Name, call.Payload) return err } if isAgentCodeTool(call.Name) { @@ -826,20 +787,10 @@ func validateToolArguments(call ToolCall) error { call.LeaseID == "" { return errors.New("agent code tools require a structured fenced root Supervisor scope") } - _, err := NormalizeAgentCodePayload(call.Name, call.Payload) + _, err := NormalizeSupervisorToolPayload(call.Name, call.Payload) return err } - if call.Name == WorkItemCreateTool || call.Name == NoteCreateTool || - call.Name == SpecialistDelegationProposeTool || - call.Name == ChildTaskProposeTool || - call.Name == PlanDeliveryProposeTool || - call.Name == HostCommandProposeTool || - call.Name == DockerSandboxRunProposeTool || - call.Name == SkillCandidateProposeTool || - call.Name == DebugTerminalTool || - call.Name == CommandRuntimeTool || - call.Name == MCPToolCallTool || IsWebEvidenceTool(call.Name) || - IsBrowserActionTool(call.Name) { + if IsSupervisorTool(call.Name) { if len(call.Arguments) != 0 { return fmt.Errorf("tool %s accepts a JSON payload instead of string arguments", call.Name) } @@ -851,6 +802,8 @@ func validateToolArguments(call ToolCall) error { } switch call.Name { case WorkItemCreateTool: + // Memory admission validates the original fields; its catalog normalizer + // also redacts and revalidates them, which can expand bounded text. _, _, err := decodeWorkItemCreateInput(call.Payload) return err case NoteCreateTool: @@ -860,77 +813,53 @@ func validateToolArguments(call ToolCall) error { if call.RequestedBy != "run_supervisor" || call.AgentID == "" || call.LeaseID == "" { return errors.New("specialist delegation proposals require a fenced root Supervisor") } - _, _, err := normalizeSpecialistDelegationPayload(call.Payload) - return err case ChildTaskProposeTool: if call.RequestedBy != "run_supervisor" || call.AgentID == "" || call.LeaseID == "" { return errors.New("child task proposals require a fenced root Supervisor") } - _, _, err := normalizeChildTaskProposalPayload(call.Payload) - return err case PlanDeliveryProposeTool: if call.RequestedBy != "run_supervisor" || call.AgentID == "" || call.LeaseID == "" { return errors.New("Plan/Delivery proposals require a fenced root Supervisor") } - _, _, err := normalizePlanDeliveryPayload(call.Payload) - return err - case HostCommandProposeTool: - if call.RequestedBy != "run_supervisor" || call.AgentID == "" || call.LeaseID == "" { - return errors.New("host command proposals require a fenced root Supervisor") - } - _, _, err := normalizeHostCommandProposalPayload(call.Payload) - return err case DockerSandboxRunProposeTool: if call.RequestedBy != "run_supervisor" || call.AgentID == "" || call.WorkspaceID == "" || call.LeaseID == "" { return errors.New("Docker Sandbox proposals require a fenced root Supervisor") } - _, _, err := normalizeDockerSandboxProposalPayload(call.Payload) - return err case SkillCandidateProposeTool: if call.RequestedBy != "run_supervisor" || call.AgentID == "" || call.WorkspaceID == "" || call.LeaseID == "" { return errors.New("Skill candidate proposals require a fenced root Supervisor") } - _, _, err := normalizeSkillCandidatePayload(call.Payload) - return err case DebugTerminalTool: if call.RequestedBy != "run_supervisor" || call.AgentID == "" || call.WorkspaceID == "" || call.LeaseID == "" { return errors.New("debug terminal calls require a fenced root Supervisor") } - _, _, err := normalizeDebugTerminalPayload(call.Payload) - return err case CommandRuntimeTool: if call.RequestedBy != "run_supervisor" || call.AgentID == "" || call.WorkspaceID == "" || call.LeaseID == "" { return errors.New("command runtime calls require a fenced root Supervisor") } - _, _, err := normalizeCommandRuntimePayload(call.Payload) - return err case MCPToolCallTool: if call.RequestedBy != "run_supervisor" || call.AgentID == "" || call.WorkspaceID == "" || call.LeaseID == "" { return errors.New("MCP tool calls require a fenced host actor") } - _, _, err := NormalizeMCPToolPayload(call.Payload) - return err case BrowserStatusTool, BrowserNavigateTool, BrowserSnapshotTool, BrowserClickTool, BrowserTypeTool, BrowserScreenshotTool, BrowserScrollTool, BrowserKeyTool: if call.RequestedBy != "run_supervisor" || call.AgentID == "" || call.LeaseID == "" { return errors.New("browser actions require a fenced root Supervisor") } - _, err := NormalizeBrowserActionPayload(call.Name, call.Payload) - return err case WebSearchTool, SourceSearchTool, WebFetchTool, WebCitationTool: if call.RequestedBy != "run_supervisor" || call.AgentID == "" || call.MissionID == "" || call.LeaseID == "" { return errors.New("web evidence calls require a fenced root Supervisor") } - _, err := NormalizeWebEvidencePayload(call.Name, call.Payload) - return err } + _, err := NormalizeSupervisorToolPayload(call.Name, call.Payload) + return err } if len(call.Payload) != 0 || call.OperationKey != "" { return fmt.Errorf("tool %s does not accept a structured payload or operation key", call.Name) diff --git a/internal/toolgateway/registry.go b/internal/toolgateway/registry.go index a5a107c6..92153eff 100644 --- a/internal/toolgateway/registry.go +++ b/internal/toolgateway/registry.go @@ -21,6 +21,7 @@ type toolRegistration struct { defaultCatalog bool planCatalog bool normalize func(ToolName, json.RawMessage) (json.RawMessage, error) + preflight func(*Gateway, ToolCall) error invoke func(*Gateway, context.Context, ToolCall) (Outcome, error) } @@ -48,92 +49,111 @@ func buildToolRegistry() toolRegistry { addSupervisor := func(definitions []ToolDefinition, normalize func(ToolName, json.RawMessage) (json.RawMessage, error), invoke func(*Gateway, context.Context, ToolCall) (Outcome, error), + preflight func(*Gateway, ToolCall) error, typedAction, defaultCatalog bool, ) { for _, definition := range definitions { add(toolRegistration{definition: definition, valid: true, typedAction: typedAction, defaultCatalog: defaultCatalog, planCatalog: defaultCatalog && definition.Name != SkillCandidateProposeTool, - normalize: normalize, invoke: invoke}) + normalize: normalize, preflight: preflight, invoke: invoke}) } } // This order is the existing model catalog order. Dynamic capability // projections narrow or replace these definitions at the application boundary. addSupervisor(StructuredMemoryToolDefinitions(), NormalizeStructuredMemoryPayload, - (*Gateway).invokeStructuredMemory, true, true) + (*Gateway).invokeStructuredMemory, + requireExecutor("structured memory", func(g *Gateway, _ ToolCall) bool { return g.structuredMemory != nil }), true, true) addSupervisor(HistoryRecallToolDefinitions(), NormalizeHistoryRecallPayload, - (*Gateway).invokeHistoryRecall, true, true) + (*Gateway).invokeHistoryRecall, nil, true, true) addSupervisor([]ToolDefinition{SkillReadToolDefinition(nil)}, func(_ ToolName, payload json.RawMessage) (json.RawMessage, error) { _, canonical, err := NormalizeSkillReadPayload(payload) return canonical, err - }, (*Gateway).invokeSkillRead, true, true) + }, (*Gateway).invokeSkillRead, nil, true, true) addSupervisor([]ToolDefinition{specialistDelegationDefinition}, func(_ ToolName, payload json.RawMessage) (json.RawMessage, error) { _, canonical, err := normalizeSpecialistDelegationPayload(payload) return canonical, err - }, (*Gateway).invokeSpecialistDelegation, true, true) + }, (*Gateway).invokeSpecialistDelegation, + requireExecutor("specialist delegation proposal", func(g *Gateway, _ ToolCall) bool { return g.delegationProposals != nil }), true, true) addSupervisor([]ToolDefinition{childTaskProposeDefinition}, func(_ ToolName, payload json.RawMessage) (json.RawMessage, error) { _, canonical, err := normalizeChildTaskProposalPayload(payload) return canonical, err - }, (*Gateway).invokeChildTaskProposal, true, true) + }, (*Gateway).invokeChildTaskProposal, + requireExecutor("child task proposal", func(g *Gateway, _ ToolCall) bool { return g.childTaskProposals != nil }), true, true) addSupervisor([]ToolDefinition{dockerSandboxProposalDefinition}, func(_ ToolName, payload json.RawMessage) (json.RawMessage, error) { _, canonical, err := normalizeDockerSandboxProposalPayload(payload) return canonical, err - }, (*Gateway).invokeDockerSandboxProposal, true, true) + }, (*Gateway).invokeDockerSandboxProposal, + requireExecutor("Docker Sandbox proposal", func(g *Gateway, _ ToolCall) bool { return g.dockerSandboxProposals != nil }), true, true) addSupervisor([]ToolDefinition{skillCandidateProposalDefinition}, func(_ ToolName, payload json.RawMessage) (json.RawMessage, error) { _, canonical, err := normalizeSkillCandidatePayload(payload) return canonical, err - }, (*Gateway).invokeSkillCandidate, false, true) + }, (*Gateway).invokeSkillCandidate, + requireExecutor("Skill candidate proposal", func(g *Gateway, _ ToolCall) bool { return g.skillCandidates != nil }), false, true) addSupervisor([]ToolDefinition{debugTerminalDefinition}, func(_ ToolName, payload json.RawMessage) (json.RawMessage, error) { _, canonical, err := normalizeDebugTerminalPayload(payload) return canonical, err - }, (*Gateway).invokeDebugTerminal, false, true) + }, (*Gateway).invokeDebugTerminal, + requireExecutor("debug terminal", func(g *Gateway, _ ToolCall) bool { return g.debugTerminal != nil }), false, true) addSupervisor([]ToolDefinition{commandRuntimeDefinition}, func(_ ToolName, payload json.RawMessage) (json.RawMessage, error) { _, canonical, err := normalizeCommandRuntimePayload(payload) return canonical, err - }, (*Gateway).invokeCommandRuntime, false, true) + }, (*Gateway).invokeCommandRuntime, + requireExecutor("command runtime", func(g *Gateway, _ ToolCall) bool { return g.commandRuntime != nil }), false, true) addSupervisor([]ToolDefinition{MCPToolDefinition()}, func(_ ToolName, payload json.RawMessage) (json.RawMessage, error) { _, canonical, err := NormalizeMCPToolPayload(payload) return canonical, err - }, (*Gateway).invokeMCP, true, true) + }, (*Gateway).invokeMCP, + requireExecutor("MCP client", func(g *Gateway, _ ToolCall) bool { return g.mcp != nil }), true, true) addSupervisor(WebEvidenceToolDefinitions(), NormalizeWebEvidencePayload, - (*Gateway).invokeWebEvidence, true, true) + (*Gateway).invokeWebEvidence, + requireExecutor("web evidence", func(g *Gateway, _ ToolCall) bool { return g.webEvidence != nil }), true, true) browserInvoke := func(g *Gateway, ctx context.Context, call ToolCall) (Outcome, error) { if IsAgentBrowserPayload(call.Payload) { return g.invokeAgentBrowser(ctx, call) } return g.invokeBrowserAction(ctx, call) } + browserPreflight := requireExecutor("browser action", func(g *Gateway, call ToolCall) bool { + if IsAgentBrowserPayload(call.Payload) { + return g.agentBrowser != nil + } + return g.browserActions != nil + }) addSupervisor(BrowserActionToolDefinitions(), NormalizeBrowserActionPayload, - browserInvoke, true, true) + browserInvoke, browserPreflight, true, true) add(toolRegistration{definition: planDeliveryDefinition, valid: true, typedAction: true, planCatalog: true, normalize: func(_ ToolName, payload json.RawMessage) (json.RawMessage, error) { _, canonical, err := normalizePlanDeliveryPayload(payload) return canonical, err - }, invoke: (*Gateway).invokePlanDelivery}) + }, preflight: requireExecutor("Plan/Delivery proposal", func(g *Gateway, _ ToolCall) bool { return g.planDeliveryProposals != nil }), + invoke: (*Gateway).invokePlanDelivery}) addSupervisor(AgentCodeToolDefinitions(), NormalizeAgentCodePayload, - (*Gateway).invokeAgentCode, true, false) + (*Gateway).invokeAgentCode, + requireExecutor("agent code tool", func(g *Gateway, _ ToolCall) bool { return g.agentCode != nil }), true, false) addSupervisor(CodeIntelToolDefinitions(), func(name ToolName, payload json.RawMessage) (json.RawMessage, error) { _, canonical, err := NormalizeCodeIntelPayload(name, payload) return canonical, err - }, (*Gateway).invokeCodeIntel, true, false) + }, (*Gateway).invokeCodeIntel, + requireExecutor("code-intel tool", func(g *Gateway, _ ToolCall) bool { return g.codeIntel != nil }), true, false) for _, name := range []ToolName{BrowserScrollTool, BrowserKeyTool} { definition, found := AgentBrowserToolDefinition(name) if !found { panic(fmt.Sprintf("missing Agent Browser definition %q", name)) } addSupervisor([]ToolDefinition{definition}, NormalizeBrowserActionPayload, - browserInvoke, true, false) + browserInvoke, browserPreflight, true, false) } // Operator tools are valid gateway names, but are absent from model catalogs. @@ -166,6 +186,15 @@ func buildToolRegistry() toolRegistry { return registry } +func requireExecutor(label string, available func(*Gateway, ToolCall) bool) func(*Gateway, ToolCall) error { + return func(g *Gateway, call ToolCall) error { + if !available(g, call) { + return fmt.Errorf("%s executor is required", label) + } + return nil + } +} + func lookupTool(name ToolName) (toolRegistration, bool) { index, found := registeredTools.byName[name] if !found { diff --git a/internal/toolgateway/registry_test.go b/internal/toolgateway/registry_test.go index fb30e438..6fddeae9 100644 --- a/internal/toolgateway/registry_test.go +++ b/internal/toolgateway/registry_test.go @@ -121,6 +121,49 @@ type registryChargingStore struct { *memoryStore } +func TestGatewayMissingRequiredExecutorDoesNotChargeBudget(t *testing.T) { + capability := testBrowserActionCapabilityContext() + browserCall := ToolCall{Name: BrowserStatusTool, OperationKey: "browser-operation", + RunID: "run-1", SessionID: "session-1", AgentID: "agent-1", + Surface: capability.Surface, Phase: capability.Phase, Role: capability.Role, + Profile: capability.Profile, PermissionMode: capability.PermissionMode, + ModeRevision: capability.ModeRevision, PermissionRevision: capability.PermissionRevision, + CapabilityGeneration: BrowserActionCapabilitySnapshot(capability).Generation, + RequestedBy: "run_supervisor", LeaseID: "lease-1", LeaseGeneration: 1} + legacyCall, agentCall := browserCall, browserCall + legacyCall.Payload = json.RawMessage(`{"version":"browser_status.v1"}`) + agentCall.Payload = json.RawMessage(`{"version":"browser_status.v2"}`) + for _, test := range []struct { + name string + call ToolCall + configure func(*Gateway) *Gateway + wantError string + }{ + {name: "command runtime", call: commandRuntimeToolCall(commandRuntimeValidPayload("Write-Output ok")), + wantError: "command runtime executor is required"}, + {name: "Docker proposal", call: validDockerSandboxToolCall(dockerSandboxProposalPayload), + wantError: "Docker Sandbox proposal executor is required"}, + {name: "legacy browser with only Agent Browser", call: legacyCall, + configure: func(g *Gateway) *Gateway { return g.WithAgentBrowserExecutor(&agentBrowserGatewayProbe{}) }, + wantError: "browser action executor is required"}, + {name: "Agent Browser with only legacy browser", call: agentCall, + configure: func(g *Gateway) *Gateway { return g.WithBrowserActionExecutor(&browserActionExecutorStub{}) }, + wantError: "browser action executor is required"}, + } { + t.Run(test.name, func(t *testing.T) { + store := newTrackedStructuredStore() + gateway := New(store, nil) + if test.configure != nil { + gateway = test.configure(gateway) + } + _, err := gateway.Invoke(t.Context(), test.call) + if err == nil || err.Error() != test.wantError || store.chargeCount() != 0 { + t.Fatalf("missing executor consumed a tool call: err=%v charges=%d", err, store.chargeCount()) + } + }) + } +} + func (s *registryChargingStore) ChargeToolCall(_ context.Context, request toolbudget.ChargeRequest) (toolbudget.Usage, error) { return toolbudget.Usage{Tracked: true, RunID: request.RunID, LastCharge: "tool-charge-" + request.ToolName}, nil diff --git a/internal/toolgateway/structured_memory_test.go b/internal/toolgateway/structured_memory_test.go index 54014e94..2b7b7f0f 100644 --- a/internal/toolgateway/structured_memory_test.go +++ b/internal/toolgateway/structured_memory_test.go @@ -258,3 +258,25 @@ func TestStructuredMemoryPolicyDenialNeverInvokesExecutor(t *testing.T) { executor.noteCalls, store.chargeCount()) } } + +func TestStructuredMemoryAdmissionKeepsOriginalTitleLimit(t *testing.T) { + title := strings.Repeat("x", 222) + " password=12345678" + for name, payload := range map[ToolName]string{ + WorkItemCreateTool: fmt.Sprintf(`{"title":%q}`, title), + NoteCreateTool: fmt.Sprintf(`{"title":%q,"content":"observation"}`, title), + } { + t.Run(string(name), func(t *testing.T) { + store := newTrackedStructuredStore() + executor := &structuredExecutorStub{} + outcome, err := New(store, policy.NewDefaultChecker()).WithStructuredMemoryExecutor(executor). + Invoke(t.Context(), ToolCall{Name: name, Payload: json.RawMessage(payload), + OperationKey: "title-limit", RunID: "run-1", SessionID: "sess-1", RequestedBy: "root"}) + if err != nil || store.chargeCount() != 1 || executor.workCalls+executor.noteCalls != 1 { + t.Fatalf("valid 240-character title was not admitted: err=%v charges=%d", err, store.chargeCount()) + } + if strings.Contains(string(outcome.Call.Payload), "12345678") { + t.Fatal("admitted title was not redacted in the outcome") + } + }) + } +}