diff --git a/docs/release-control/v6/internal/subsystems/ai-runtime.md b/docs/release-control/v6/internal/subsystems/ai-runtime.md index 97bdb5743..037306892 100644 --- a/docs/release-control/v6/internal/subsystems/ai-runtime.md +++ b/docs/release-control/v6/internal/subsystems/ai-runtime.md @@ -4601,6 +4601,16 @@ invocation, and recomputes the offered governance action mode, so the offered schema and the enforcement boundary can never disagree. Control-level blocks keep returning the operator guidance message; policy blocks return the shared invocation-blocked result. +Registration authority on the Assistant tool registry is split: +`registerBuiltin` is the construction-time path for canonical Pulse +tools (shared descriptor mandatory, override rejected), while +`RegisterExtension` - the only path exposed through +`PulseToolExecutor.RegisterTool` - rejects every canonical tool name +outright and requires the extension to declare its own descriptor. +Registry entries are append-only: a name registers exactly once, so a +later registration (with or without a descriptor override) can never +replace an already-governed handler such as pulse_read's +execution-intent-enforcing exec path. The classification vocabulary is closed: descriptor validation rejects any class outside the known kinds and mutation targets, and `InvocationPolicy.Allows` independently denies unknown mutation targets diff --git a/internal/ai/chat/service_tooling_test.go b/internal/ai/chat/service_tooling_test.go index fdb515e07..66c3d23a8 100644 --- a/internal/ai/chat/service_tooling_test.go +++ b/internal/ai/chat/service_tooling_test.go @@ -603,13 +603,17 @@ func TestExecuteCommand_ErrorAndApprovalPaths(t *testing.T) { t.Fatalf("expected error with exit code 1") } - exec.RegisterTool(tools.RegisteredTool{ + // Registry entries are append-only, so the approval scenario uses a + // fresh executor instead of swapping the handler in place. + approvalExec := tools.NewPulseToolExecutor(tools.ExecutorConfig{}) + approvalExec.RegisterTool(tools.RegisteredTool{ Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), Definition: tools.Tool{Name: agentcapabilities.PulseRunCommandToolName}, Handler: func(ctx context.Context, exec *tools.PulseToolExecutor, args map[string]interface{}) (tools.CallToolResult, error) { return tools.NewTextResult("APPROVAL_REQUIRED: requires approval"), nil }, }) + svc = &Service{executor: approvalExec} _, _, err = svc.ExecuteCommand(context.Background(), "uptime", "") if err == nil { @@ -648,40 +652,34 @@ func TestExecuteCommandUsesSharedResultTextProjection(t *testing.T) { } func TestExecuteAssistantTool_ErrorsAndSuccess(t *testing.T) { - exec := tools.NewPulseToolExecutor(tools.ExecutorConfig{}) - exec.RegisterTool(tools.RegisteredTool{ - Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), - Definition: tools.Tool{Name: "test_tool"}, - Handler: func(ctx context.Context, exec *tools.PulseToolExecutor, args map[string]interface{}) (tools.CallToolResult, error) { - return tools.NewErrorResult(context.DeadlineExceeded), nil - }, + // Registry entries are append-only, so each handler scenario runs on + // a fresh executor instead of swapping the handler in place. + newService := func(handler func(ctx context.Context, exec *tools.PulseToolExecutor, args map[string]interface{}) (tools.CallToolResult, error)) *Service { + exec := tools.NewPulseToolExecutor(tools.ExecutorConfig{}) + exec.RegisterTool(tools.RegisteredTool{ + Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), + Definition: tools.Tool{Name: "test_tool"}, + Handler: handler, + }) + return &Service{executor: exec} + } + + svc := newService(func(ctx context.Context, exec *tools.PulseToolExecutor, args map[string]interface{}) (tools.CallToolResult, error) { + return tools.NewErrorResult(context.DeadlineExceeded), nil }) - - svc := &Service{executor: exec} - - _, err := svc.ExecuteAssistantTool(context.Background(), "test_tool", map[string]interface{}{}) - if err == nil { + if _, err := svc.ExecuteAssistantTool(context.Background(), "test_tool", map[string]interface{}{}); err == nil { t.Fatalf("expected tool error") } - exec.RegisterTool(tools.RegisteredTool{ - Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), - Definition: tools.Tool{Name: "test_tool"}, - Handler: func(ctx context.Context, exec *tools.PulseToolExecutor, args map[string]interface{}) (tools.CallToolResult, error) { - return tools.NewTextResult("POLICY_BLOCKED: nope"), nil - }, + svc = newService(func(ctx context.Context, exec *tools.PulseToolExecutor, args map[string]interface{}) (tools.CallToolResult, error) { + return tools.NewTextResult("POLICY_BLOCKED: nope"), nil }) - _, err = svc.ExecuteAssistantTool(context.Background(), "test_tool", map[string]interface{}{}) - if err == nil { + if _, err := svc.ExecuteAssistantTool(context.Background(), "test_tool", map[string]interface{}{}); err == nil { t.Fatalf("expected policy blocked error") } - exec.RegisterTool(tools.RegisteredTool{ - Invocation: tools.StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), - Definition: tools.Tool{Name: "test_tool"}, - Handler: func(ctx context.Context, exec *tools.PulseToolExecutor, args map[string]interface{}) (tools.CallToolResult, error) { - return tools.NewTextResult("ok"), nil - }, + svc = newService(func(ctx context.Context, exec *tools.PulseToolExecutor, args map[string]interface{}) (tools.CallToolResult, error) { + return tools.NewTextResult("ok"), nil }) output, err := svc.ExecuteAssistantTool(context.Background(), "test_tool", map[string]interface{}{}) if err != nil || output != "ok" { diff --git a/internal/ai/tools/executor.go b/internal/ai/tools/executor.go index 9d6656416..a84415441 100644 --- a/internal/ai/tools/executor.go +++ b/internal/ai/tools/executor.go @@ -809,7 +809,7 @@ func (e *PulseToolExecutor) SetProtectedGuests(vmids []string) { // RegisterTool allows tests or extensions to add tools at runtime. func (e *PulseToolExecutor) RegisterTool(tool RegisteredTool) { - e.registry.Register(tool) + e.registry.RegisterExtension(tool) } // Runtime setter methods for updating providers after creation diff --git a/internal/ai/tools/executor_setters_test.go b/internal/ai/tools/executor_setters_test.go index 75e2aa6ea..455c80ba5 100644 --- a/internal/ai/tools/executor_setters_test.go +++ b/internal/ai/tools/executor_setters_test.go @@ -189,11 +189,11 @@ func TestPulseToolExecutor_GetReadStatePrefersUnifiedResourceProvider(t *testing func TestToolRegistry_ListTools(t *testing.T) { registry := NewToolRegistry() - registry.Register(RegisteredTool{ + registry.RegisterExtension(RegisteredTool{ Invocation: StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), Definition: Tool{Name: "read"}, }) - registry.Register(RegisteredTool{ + registry.RegisterExtension(RegisteredTool{ Invocation: StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationInfrastructure), Definition: Tool{Name: "control"}, RequireControl: true, @@ -232,7 +232,7 @@ func TestToolRegistry_ListToolsReturnsIndependentDefinitions(t *testing.T) { properties := map[string]PropertySchema{ "mode": {Type: "string", Enum: enum}, } - registry.Register(RegisteredTool{ + registry.RegisterExtension(RegisteredTool{ Invocation: StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), Definition: Tool{ Name: "read", @@ -308,7 +308,7 @@ func TestToolGovernanceUsesSharedAgentCapabilityShape(t *testing.T) { func TestToolRegistry_ExecuteControlToolReadOnlyUsesAssistantAndPatrolGuidance(t *testing.T) { registry := NewToolRegistry() - registry.Register(RegisteredTool{ + registry.RegisterExtension(RegisteredTool{ Invocation: StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), Definition: Tool{Name: "control"}, RequireControl: true, @@ -335,7 +335,7 @@ func TestToolRegistryExecuteNormalizesSharedToolCallParams(t *testing.T) { registry := NewToolRegistry() var gotArgs map[string]interface{} var gotArgsLenBeforeMutation int - registry.Register(RegisteredTool{ + registry.RegisterExtension(RegisteredTool{ Invocation: StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), Definition: Tool{Name: "test_tool"}, Handler: func(ctx context.Context, e *PulseToolExecutor, args map[string]interface{}) (CallToolResult, error) { @@ -368,7 +368,7 @@ func TestToolRegistryExecuteNormalizesSharedToolCallParams(t *testing.T) { assert.True(t, interpreted.IsError) assert.Contains(t, interpreted.Text, "invalid tools/call params: tool name is required") - registry.Register(RegisteredTool{ + registry.RegisterExtension(RegisteredTool{ Invocation: StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), Definition: Tool{Name: "empty_args"}, Handler: func(ctx context.Context, e *PulseToolExecutor, args map[string]interface{}) (CallToolResult, error) { @@ -387,14 +387,14 @@ func TestToolRegistryExecuteNormalizesSharedToolCallParams(t *testing.T) { func TestToolRegistryExecuteNormalizesSharedToolResult(t *testing.T) { registry := NewToolRegistry() handlerContent := []Content{{Type: "text", Text: "ok"}} - registry.Register(RegisteredTool{ + registry.RegisterExtension(RegisteredTool{ Invocation: StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), Definition: Tool{Name: "read"}, Handler: func(ctx context.Context, e *PulseToolExecutor, args map[string]interface{}) (CallToolResult, error) { return CallToolResult{Content: handlerContent}, nil }, }) - registry.Register(RegisteredTool{ + registry.RegisterExtension(RegisteredTool{ Invocation: StaticInvocation(agentcapabilities.ToolCallKindWrite, agentcapabilities.MutationPulseState), Definition: Tool{Name: "empty"}, Handler: func(ctx context.Context, e *PulseToolExecutor, args map[string]interface{}) (CallToolResult, error) { diff --git a/internal/ai/tools/invocation_policy_test.go b/internal/ai/tools/invocation_policy_test.go index a998fae41..ceb17abce 100644 --- a/internal/ai/tools/invocation_policy_test.go +++ b/internal/ai/tools/invocation_policy_test.go @@ -164,7 +164,7 @@ func TestRegisterRejectsOverridesForCanonicalToolNames(t *testing.T) { t.Fatal("overriding a canonical tool's descriptor must panic") } }() - registry.Register(RegisteredTool{ + registry.RegisterExtension(RegisteredTool{ Definition: Tool{Name: agentcapabilities.PulseControlToolName}, Invocation: StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), }) @@ -185,3 +185,73 @@ func TestReadOnlyDockerProjectsAsReadScopeOnly(t *testing.T) { } t.Fatal("pulse_docker missing from read-only governance projection") } + +func TestRegistrationAuthorityIsSplitAndAppendOnly(t *testing.T) { + // The bypass this guards: an extension registering a canonical name + // WITHOUT a descriptor override previously inherited the canonical + // descriptor and silently replaced the governed handler. + t.Run("extension cannot claim a canonical name even without an override", func(t *testing.T) { + registry := NewToolRegistry() + defer func() { + if recover() == nil { + t.Fatal("registering pulse_read as an extension must panic") + } + }() + registry.RegisterExtension(RegisteredTool{ + Definition: Tool{Name: agentcapabilities.PulseReadToolName}, + Handler: func(ctx context.Context, e *PulseToolExecutor, args map[string]interface{}) (CallToolResult, error) { + return NewTextResult("hijacked"), nil + }, + }) + }) + + t.Run("extension registrations are append-only", func(t *testing.T) { + registry := NewToolRegistry() + tool := RegisteredTool{ + Definition: Tool{Name: "extension_tool"}, + Invocation: StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), + } + registry.RegisterExtension(tool) + defer func() { + if recover() == nil { + t.Fatal("duplicate extension registration must panic") + } + }() + registry.RegisterExtension(tool) + }) + + t.Run("builtin registrations are append-only", func(t *testing.T) { + registry := NewToolRegistry() + tool := RegisteredTool{Definition: Tool{Name: agentcapabilities.PulseQueryToolName}} + registry.registerBuiltin(tool) + defer func() { + if recover() == nil { + t.Fatal("duplicate builtin registration must panic") + } + }() + registry.registerBuiltin(tool) + }) + + t.Run("builtin rejects descriptor overrides", func(t *testing.T) { + registry := NewToolRegistry() + defer func() { + if recover() == nil { + t.Fatal("builtin registration with an override must panic") + } + }() + registry.registerBuiltin(RegisteredTool{ + Definition: Tool{Name: agentcapabilities.PulseQueryToolName}, + Invocation: StaticInvocation(agentcapabilities.ToolCallKindRead, agentcapabilities.MutationNone), + }) + }) + + t.Run("extension without a descriptor is rejected", func(t *testing.T) { + registry := NewToolRegistry() + defer func() { + if recover() == nil { + t.Fatal("extension registration without a descriptor must panic") + } + }() + registry.RegisterExtension(RegisteredTool{Definition: Tool{Name: "undescribed_tool"}}) + }) +} diff --git a/internal/ai/tools/registry.go b/internal/ai/tools/registry.go index 3b332d735..ebae50a61 100644 --- a/internal/ai/tools/registry.go +++ b/internal/ai/tools/registry.go @@ -119,38 +119,67 @@ func (p InvocationPolicy) Allows(class agentcapabilities.InvocationClass) bool { } } -// Register adds a tool to the registry. Every registered tool must have a -// canonical invocation descriptor whose cases exactly cover the schema -// enum of its discriminator; a tool that cannot be classified must not be -// registerable, so this panics on programmer error rather than degrading -// to an unclassified (and therefore ungovernable) tool. -func (r *ToolRegistry) Register(tool RegisteredTool) { +// Registration authority is split so extension code can never claim or +// replace a canonical tool. registerBuiltin is the construction-time path +// for canonical Pulse tools; RegisterExtension is the only path exposed +// outside executor construction and rejects every canonical name. Both +// paths are append-only: a name registers exactly once, so a later +// registration can never swap out an already-governed handler. + +// registerBuiltin adds a canonical Pulse tool during executor +// construction. The tool must classify through the shared descriptor +// table (no override) and its descriptor must validate against the schema +// enum; a tool that cannot be classified must not be registerable, so +// this panics on programmer error rather than degrading to an +// unclassified (and therefore ungovernable) tool. +func (r *ToolRegistry) registerBuiltin(tool RegisteredTool) { r.mu.Lock() defer r.mu.Unlock() tool.Definition = tool.Definition.NormalizeCollections() name := tool.Definition.Name - canonical, isCanonical := agentcapabilities.InvocationDescriptorFor(name) - var descriptor agentcapabilities.InvocationDescriptor - switch { - case isCanonical && tool.Invocation != nil: + if tool.Invocation != nil { // A canonical tool name must classify through the shared table; // an override could silently relax its safety classification. - panic(fmt.Sprintf("tool %q is canonical; its invocation descriptor comes from agentcapabilities/invocation.go and cannot be overridden", name)) - case isCanonical: - descriptor = canonical - case tool.Invocation != nil: - descriptor = tool.Invocation.Clone() - default: + panic(fmt.Sprintf("tool %q is builtin; its invocation descriptor comes from agentcapabilities/invocation.go and cannot be overridden", name)) + } + canonical, isCanonical := agentcapabilities.InvocationDescriptorFor(name) + if !isCanonical { panic(fmt.Sprintf("tool %q has no canonical invocation descriptor; declare one in agentcapabilities/invocation.go", name)) } + r.store(name, tool, canonical) +} + +// RegisterExtension adds a non-canonical tool (tests, extensions). It +// rejects every canonical tool name outright - even without a descriptor +// override, re-registering a canonical name would swap out its governed +// handler - and requires the extension to declare its own invocation +// descriptor. +func (r *ToolRegistry) RegisterExtension(tool RegisteredTool) { + r.mu.Lock() + defer r.mu.Unlock() + + tool.Definition = tool.Definition.NormalizeCollections() + name := tool.Definition.Name + if _, isCanonical := agentcapabilities.InvocationDescriptorFor(name); isCanonical { + panic(fmt.Sprintf("tool %q is a canonical Pulse tool and cannot be registered as an extension", name)) + } + if tool.Invocation == nil { + panic(fmt.Sprintf("extension tool %q must declare its own invocation descriptor", name)) + } + r.store(name, tool, tool.Invocation.Clone()) +} + +// store validates and appends one registration. Callers hold r.mu. +func (r *ToolRegistry) store(name string, tool RegisteredTool, descriptor agentcapabilities.InvocationDescriptor) { + if _, exists := r.tools[name]; exists { + panic(fmt.Sprintf("tool %q is already registered; registry entries are append-only", name)) + } if err := descriptor.Validate(name, discriminatorEnum(tool.Definition, descriptor.Discriminator)); err != nil { panic(err.Error()) } tool.Invocation = &descriptor - if _, exists := r.tools[name]; !exists { - r.order = append(r.order, name) - } + r.order = append(r.order, name) r.tools[name] = tool } diff --git a/internal/ai/tools/tools_alerts.go b/internal/ai/tools/tools_alerts.go index 6491754d5..6195a604e 100644 --- a/internal/ai/tools/tools_alerts.go +++ b/internal/ai/tools/tools_alerts.go @@ -10,7 +10,7 @@ import ( // registerAlertsTools registers the pulse_alerts tool func (e *PulseToolExecutor) registerAlertsTools() { - e.registry.Register(RegisteredTool{ + e.registry.registerBuiltin(RegisteredTool{ Definition: Tool{ Name: agentcapabilities.PulseAlertsToolName, Description: `Manage alerts and AI patrol findings. diff --git a/internal/ai/tools/tools_control.go b/internal/ai/tools/tools_control.go index f61ccee96..5bc761697 100644 --- a/internal/ai/tools/tools_control.go +++ b/internal/ai/tools/tools_control.go @@ -20,7 +20,7 @@ import ( // registerControlTools registers the pulse_control tool func (e *PulseToolExecutor) registerControlTools() { - e.registry.Register(RegisteredTool{ + e.registry.registerBuiltin(RegisteredTool{ Definition: Tool{ Name: agentcapabilities.PulseControlToolName, Description: `WRITE operations: control canonical resources that explicitly advertise shared Pulse actions (for example Proxmox guests and supported app-containers) or execute state-modifying commands. Some canonical resources are read-only and will reject pulse_control even when their type is vm or system-container. Read-only and Docker-only workflows are exposed through their own governed tools.`, diff --git a/internal/ai/tools/tools_discovery.go b/internal/ai/tools/tools_discovery.go index 92af25cf9..59e89bd90 100644 --- a/internal/ai/tools/tools_discovery.go +++ b/internal/ai/tools/tools_discovery.go @@ -12,7 +12,7 @@ import ( // registerDiscoveryTools registers the pulse_discovery tool func (e *PulseToolExecutor) registerDiscoveryTools() { - e.registry.Register(RegisteredTool{ + e.registry.registerBuiltin(RegisteredTool{ Definition: Tool{ Name: agentcapabilities.PulseDiscoveryToolName, Description: `Get or run AI-discovered service details from Pulse-owned resource access. action="get" returns existing discovery and triggers discovery only when missing. action="run" forces a fresh discovery run for a known resource. action="list" searches existing discoveries. Resource details must already be known from the current context or a prior resource lookup.`, diff --git a/internal/ai/tools/tools_docker.go b/internal/ai/tools/tools_docker.go index 6d63ba613..59c930b70 100644 --- a/internal/ai/tools/tools_docker.go +++ b/internal/ai/tools/tools_docker.go @@ -50,7 +50,7 @@ var ( // registerDockerTools registers the pulse_docker tool func (e *PulseToolExecutor) registerDockerTools() { - e.registry.Register(RegisteredTool{ + e.registry.registerBuiltin(RegisteredTool{ Definition: Tool{ Name: agentcapabilities.PulseDockerToolName, Description: `Manage Docker containers, updates, and Swarm services. Actions: control, updates, check_updates, update, services, tasks, swarm.`, diff --git a/internal/ai/tools/tools_file.go b/internal/ai/tools/tools_file.go index 196b889de..7c0fe0c57 100644 --- a/internal/ai/tools/tools_file.go +++ b/internal/ai/tools/tools_file.go @@ -33,7 +33,7 @@ type ExecutionProvenance struct { // registerFileTools registers the file editing tool func (e *PulseToolExecutor) registerFileTools() { - e.registry.Register(RegisteredTool{ + e.registry.registerBuiltin(RegisteredTool{ Definition: Tool{ Name: agentcapabilities.PulseFileEditToolName, Description: `Edit files on remote hosts, containers, VMs, and Docker containers safely. diff --git a/internal/ai/tools/tools_file_test.go b/internal/ai/tools/tools_file_test.go index a117aec67..4e44d4490 100644 --- a/internal/ai/tools/tools_file_test.go +++ b/internal/ai/tools/tools_file_test.go @@ -13,7 +13,6 @@ import ( func TestFileTools_Registry(t *testing.T) { exec := NewPulseToolExecutor(ExecutorConfig{}) - exec.registerFileTools() tools := exec.registry.ListTools(InvocationPolicy{ControlLevel: ControlLevelControlled}) found := false diff --git a/internal/ai/tools/tools_knowledge.go b/internal/ai/tools/tools_knowledge.go index b39bf01e5..58c987764 100644 --- a/internal/ai/tools/tools_knowledge.go +++ b/internal/ai/tools/tools_knowledge.go @@ -79,7 +79,7 @@ type KnowledgeEntry struct { // registerKnowledgeTools registers the pulse_knowledge tool func (e *PulseToolExecutor) registerKnowledgeTools() { - e.registry.Register(RegisteredTool{ + e.registry.registerBuiltin(RegisteredTool{ Definition: Tool{ Name: agentcapabilities.PulseKnowledgeToolName, Description: `Manage AI knowledge, notes, and incident analysis. diff --git a/internal/ai/tools/tools_kubernetes.go b/internal/ai/tools/tools_kubernetes.go index ac4197b50..34c6dbc36 100644 --- a/internal/ai/tools/tools_kubernetes.go +++ b/internal/ai/tools/tools_kubernetes.go @@ -37,7 +37,7 @@ type kubernetesClusterTarget struct { // registerKubernetesTools registers the pulse_kubernetes tool func (e *PulseToolExecutor) registerKubernetesTools() { - e.registry.Register(RegisteredTool{ + e.registry.registerBuiltin(RegisteredTool{ Definition: Tool{ Name: agentcapabilities.PulseKubernetesToolName, Description: `Query and control Kubernetes clusters, nodes, pods, and deployments. Query: clusters, nodes, pods, deployments. Control: scale, restart, delete_pod, exec, logs.`, diff --git a/internal/ai/tools/tools_metrics.go b/internal/ai/tools/tools_metrics.go index 1b20f1992..b0503426c 100644 --- a/internal/ai/tools/tools_metrics.go +++ b/internal/ai/tools/tools_metrics.go @@ -14,7 +14,7 @@ import ( // registerMetricsTools registers the pulse_metrics tool func (e *PulseToolExecutor) registerMetricsTools() { - e.registry.Register(RegisteredTool{ + e.registry.registerBuiltin(RegisteredTool{ Definition: Tool{ Name: agentcapabilities.PulseMetricsToolName, Description: `Get performance metrics, baselines, and sensor data. diff --git a/internal/ai/tools/tools_patrol.go b/internal/ai/tools/tools_patrol.go index 7bb8bb101..977a53fd6 100644 --- a/internal/ai/tools/tools_patrol.go +++ b/internal/ai/tools/tools_patrol.go @@ -12,7 +12,7 @@ import ( // These tools are only functional during a patrol run when patrolFindingCreator is set. func (e *PulseToolExecutor) registerPatrolTools() { // patrol_report_finding — LLM calls this to create a finding - e.registry.Register(RegisteredTool{ + e.registry.registerBuiltin(RegisteredTool{ Definition: Tool{ Name: agentcapabilities.PatrolReportFindingToolName, Description: `Report an infrastructure finding discovered during patrol investigation. @@ -89,7 +89,7 @@ Returns: {"ok": true, "finding_id": "...", "is_new": true/false} on success.`, }) // patrol_resolve_finding — LLM calls this to resolve an active finding - e.registry.Register(RegisteredTool{ + e.registry.registerBuiltin(RegisteredTool{ Definition: Tool{ Name: agentcapabilities.PatrolResolveFindingToolName, Description: `Resolve an active patrol finding after verifying the issue is no longer present. @@ -121,7 +121,7 @@ Returns: {"ok": true, "resolved": true} on success.`, }) // patrol_get_findings — LLM calls this to check existing active findings - e.registry.Register(RegisteredTool{ + e.registry.registerBuiltin(RegisteredTool{ Definition: Tool{ Name: agentcapabilities.PatrolGetFindingsToolName, Description: `Get currently active patrol findings. Use this to check what findings already exist before reporting new ones (avoids duplicates) and to identify findings that may need resolution. diff --git a/internal/ai/tools/tools_pmg.go b/internal/ai/tools/tools_pmg.go index 121ad4123..a424bb6db 100644 --- a/internal/ai/tools/tools_pmg.go +++ b/internal/ai/tools/tools_pmg.go @@ -9,7 +9,7 @@ import ( // registerPMGTools registers the pulse_pmg tool func (e *PulseToolExecutor) registerPMGTools() { - e.registry.Register(RegisteredTool{ + e.registry.registerBuiltin(RegisteredTool{ Definition: Tool{ Name: agentcapabilities.PulsePMGToolName, Description: `Query Proxmox Mail Gateway status and statistics. Types: status, mail_stats, queues, spam.`, diff --git a/internal/ai/tools/tools_query.go b/internal/ai/tools/tools_query.go index f9d760d70..7950e960d 100644 --- a/internal/ai/tools/tools_query.go +++ b/internal/ai/tools/tools_query.go @@ -2148,7 +2148,7 @@ func logRoutingMismatchDebug(targetHost string, childKinds, childIDs []string) { // registerQueryTools registers the pulse_query tool func (e *PulseToolExecutor) registerQueryTools() { - e.registry.Register(RegisteredTool{ + e.registry.registerBuiltin(RegisteredTool{ Definition: Tool{ Name: agentcapabilities.PulseQueryToolName, Description: `Query and search canonical infrastructure resources. Start here to discover systems, workloads, storage, and disks by name. Actions: search, get, config, topology, list, health.`, diff --git a/internal/ai/tools/tools_read.go b/internal/ai/tools/tools_read.go index a140d2f3b..39f17d304 100644 --- a/internal/ai/tools/tools_read.go +++ b/internal/ai/tools/tools_read.go @@ -14,7 +14,7 @@ import ( // registerReadTools registers the read-only pulse_read tool // This tool is ALWAYS classified as ToolKindRead and will never trigger VERIFYING state func (e *PulseToolExecutor) registerReadTools() { - e.registry.Register(RegisteredTool{ + e.registry.registerBuiltin(RegisteredTool{ Definition: Tool{ Name: agentcapabilities.PulseReadToolName, Description: `Execute read-only operations on infrastructure (exec, file, find, tail, logs). Rejects write commands. Use target_host for agent-routed reads, or resource_id for API-backed native resource logs such as supported TrueNAS app-containers. When Pulse has attached resource context, use target_host="current_resource" or resource_id="current_resource" for the attached resource instead of copying redacted identifiers.`, diff --git a/internal/ai/tools/tools_storage.go b/internal/ai/tools/tools_storage.go index 398ec1bc4..12a70eafc 100644 --- a/internal/ai/tools/tools_storage.go +++ b/internal/ai/tools/tools_storage.go @@ -16,7 +16,7 @@ import ( // registerStorageTools registers the pulse_storage tool func (e *PulseToolExecutor) registerStorageTools() { - e.registry.Register(RegisteredTool{ + e.registry.registerBuiltin(RegisteredTool{ Definition: Tool{ Name: agentcapabilities.PulseStorageToolName, Description: `Query storage pools, backups, snapshots, Ceph, replication, RAID, and disk health. Use the "type" parameter to select what to query.`, diff --git a/internal/ai/tools/tools_summarize.go b/internal/ai/tools/tools_summarize.go index e4bf96bb5..41b089f3f 100644 --- a/internal/ai/tools/tools_summarize.go +++ b/internal/ai/tools/tools_summarize.go @@ -23,7 +23,7 @@ import ( // the chat session so this tool can return AI-generated synthesis // in the same shape. func (e *PulseToolExecutor) registerSummarizeTools() { - e.registry.Register(RegisteredTool{ + e.registry.registerBuiltin(RegisteredTool{ Definition: Tool{ Name: agentcapabilities.PulseSummarizeToolName, Description: `Generate a retrospective summary of one resource or a fleet across a time window. Use this when the operator asks questions like "what's been happening with pve1 this week" or "where should I look across my fleet" — answers grounded in metric stats, alerts, storage state, disk health, and Patrol findings within the window. diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 5620f4d07..67feefb4c 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -18559,6 +18559,12 @@ func TestContract_PulseMCPAdapterProjectsAgentCapabilitiesManifest(t *testing.T) `agentcapabilities.NewUnknownToolResult(name)`, `agentcapabilities.NewControlToolsDisabledToolResult()`, `return result.NormalizeCollections(), err`, + // Registration authority is split: builtin registration is + // construction-only, extensions can never claim or replace a + // canonical name, and entries are append-only. + `func (r *ToolRegistry) registerBuiltin(tool RegisteredTool)`, + `func (r *ToolRegistry) RegisterExtension(tool RegisteredTool)`, + `registry entries are append-only`, // Invocation-level enforcement runs before the handler and // consumes the same descriptor the projection filters with. `class = tool.Invocation.Classify(args)`,