From 7996848b640e96603aa16fbf16e7cc883c215f9b Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Wed, 2 Sep 2026 00:19:08 +0100 Subject: [PATCH] Submit advertised lifecycle actions through pulse_control, not prose A paying operator asked the Assistant to reboot five Proxmox VMs matching a name pattern (GitHub #1782, support mail 2026-08-26 and 2026-08-29). The model resolved the VMs and then ended with a report that invented a prerequisite: a QEMU guest agent on 6.3.2, a "discovery binding" on stable 6.4.0. It never planned the action. Three defects made the governed path fail whenever the model did try it, and nothing refused the prose ending when it did not: - pulse_control handed the session-scoped id (vm::) to the action lifecycle, whose registry keys on canonical unified ids, so a Proxmox guest plan could never resolve. - pulse_control gated the action on the legacy per-executor action list, which never carried the canonical "reboot" capability Proxmox guests advertise, so "reboot" was refused as not permitted before planning. - A reference absent from the session context was refused with "resource discovery is required" even when the unified inventory resolved it. pulse_control now binds its target to the canonical unified resource (session alias first, then a unique inventory match, refusing ambiguity with candidate ids and naming the pulse_query recovery on a miss), passes the canonical id to the planner, and answers "not available" only from the resource's current advertised capabilities. The FSM ordering block and the shared operating instructions state that a recoverable block is not a limitation to report, and the instructions require the governed action tool for advertised capabilities. The agentic loop adds a bounded advertised-action gate: when the operator asked for a lifecycle action, pulse_control was offered but never submitted, and a session-resolved resource advertises the action, a tool-free final answer is refused once with the exact per-target calls. Covered by tools and loop unit tests (the #1782 transcript against a scripted provider fails on the previous code with the two exact errors above), a prompt-contract test, and the live eval scenario ProxmoxBulkLifecycleActionScenario. --- .../v6/internal/subsystems/ai-runtime.md | 75 ++++ internal/ai/chat/agentic.go | 47 ++ internal/ai/chat/agentic_action_gate.go | 94 ++++ internal/ai/chat/agentic_action_gate_test.go | 336 +++++++++++++++ internal/ai/chat/fsm.go | 2 +- internal/ai/chat/service.go | 2 + internal/ai/chat/service_tooling_test.go | 11 + internal/ai/chat/types.go | 23 + internal/ai/eval/assertions.go | 26 ++ internal/ai/eval/branchcov0723pm_test.go | 3 +- internal/ai/eval/eval_test.go | 21 + internal/ai/eval/scenarios.go | 52 +++ internal/ai/tools/control_targets.go | 400 +++++++++++++++++ internal/ai/tools/control_targets_test.go | 404 ++++++++++++++++++ internal/ai/tools/tools_control.go | 44 +- 15 files changed, 1524 insertions(+), 16 deletions(-) create mode 100644 internal/ai/chat/agentic_action_gate.go create mode 100644 internal/ai/chat/agentic_action_gate_test.go create mode 100644 internal/ai/tools/control_targets.go create mode 100644 internal/ai/tools/control_targets_test.go diff --git a/docs/release-control/v6/internal/subsystems/ai-runtime.md b/docs/release-control/v6/internal/subsystems/ai-runtime.md index 132852209..3058e60ac 100644 --- a/docs/release-control/v6/internal/subsystems/ai-runtime.md +++ b/docs/release-control/v6/internal/subsystems/ai-runtime.md @@ -7796,3 +7796,78 @@ Menu-opening controls remain buttons, and the mobile bar continues to own `--pulse-mobile-nav-height`; Assistant overlay containment and action approval boundaries are unchanged. Mobile navigation and AppLayout tests pin the route and focus behavior. + +### Advertised lifecycle actions are submitted, never narrated + +When an operator asks the Assistant to perform a lifecycle action (start, +stop, shutdown, reboot/restart) and a canonical resource the session has +resolved advertises that capability, the Assistant submits `pulse_control` +for each target and lets the shared action lifecycle decide availability +through planning, approval, execution, and verification. It may report a +limitation only from a tool result in the current turn; an assumed +prerequisite (QEMU guest agent, "discovery binding", "session state") or a +manual `qm`/`pct` instruction for an action Pulse offers is a contract +violation (GitHub issue #1782). Three structural guarantees enforce this: + +- `pulse_control` binds its `resource_id` to the canonical unified resource. + Session context is consulted first; a reference absent from the session + that resolves uniquely in the unified inventory is registered and planned, + an ambiguous name is refused with the candidate canonical ids, and a + lookup miss names the exact `pulse_query` recovery call. The plan request + carries the canonical unified id (never the session-scoped + `kind:host:uid`), and the legacy per-executor action list is not a gate: + whether the action exists is the action lifecycle's decision from the + resource's advertised capabilities (`internal/ai/tools/control_targets.go`). + A capability the resource does not advertise comes back as + `ACTION_NOT_ALLOWED` tool evidence listing the currently advertised + capabilities. +- Recoverable ordering blocks (the RESOLVING FSM state, a strict-resolution + miss) name the read-only step to take first and state that they are not a + limitation to report; the shared operating instructions say the same and + require the governed action tool for advertised capabilities. +- The agentic loop's advertised-action gate refuses, once per run, a + tool-free final answer when the operator's message requests a lifecycle + action, `pulse_control` was offered, no `pulse_control` call reached + execution, and at least one session-resolved resource currently advertises + the action. The refusal is a provider-only user-role correction naming the + exact calls per target; it fails open on the next prose answer so a model + with tool-evidenced reasons not to act is never livelocked + (`internal/ai/chat/agentic_action_gate.go`). + +Proofs: `internal/ai/tools/control_targets_test.go`, +`internal/ai/chat/agentic_action_gate_test.go` (the #1782 transcript against +a scripted provider), `internal/agentcapabilities/governance_prompt_test.go`, +and the live eval `ProxmoxBulkLifecycleActionScenario` in +`internal/ai/eval/scenarios.go`. + +### Alert-mirroring findings fold under the alert; flapping collapses to one row + +Real-time alerts own down, threshold, and age conditions, but Patrol's +deterministic watchers and the model can still emit a finding for the same +resource and condition. `internal/ai/findings_alert_mirror.go` is the single +matcher: a finding mirrors an alert when it carries the alert's identifier or +when both name the same canonical resource and the same condition class +(down, restart loop, container health, disk capacity, memory, CPU, backup age, +snapshot age, temperature). Unknown alert types and free-form findings never +match. `PatrolService.reconcileAlertMirrors` runs at the end of every real and +demo patrol cycle against the unscoped active-alert snapshot and stamps +`mirrors_alert_id` and `mirrors_alert_type` on unresolved findings through +`FindingsStore.StampAlertMirrors`, clearing the stamp once the alert resolves. +The stamp is a derived read-model annotation: it is not persisted and is not a +reason to drop the finding. Surfaces demote stamped findings under the alert +instead of listing the same problem twice. + +Flapping is owned by the existing storm throttler rather than a parallel +mechanism. `FindingsStore.recordTransitionLifecycleLocked` routes every +regressed, resolved, and auto_resolved transition through +`findingStormThrottler.observeFlapLocked`, which keeps a 24-hour sliding window +per finding (hydrated from the persisted lifecycle on first sight so a restart +does not forget). At four or more transitions the finding carries `flapping` +(count, window, first and latest transition) and the store maintains one +`flapping` lifecycle row whose metadata carries the count, updating it in place +instead of appending a row per transition; below the threshold the ordinary +rows return and the label clears. Regression counters keep counting while +collapsed. `internal/ai/findings_flapping_test.go`, +`internal/ai/findings_storm_throttler_test.go`, and +`internal/ai/findings_alert_mirror_test.go` pin the threshold, the collapse, +the hydration, and the matcher. diff --git a/internal/ai/chat/agentic.go b/internal/ai/chat/agentic.go index 2e60cb7d7..b95ce69a4 100644 --- a/internal/ai/chat/agentic.go +++ b/internal/ai/chat/agentic.go @@ -972,6 +972,12 @@ func (a *AgenticLoop) executeWithTools(ctx context.Context, sessionID string, me lookGateToolAttempted := false lookGateBlocks := 0 + // Advertised-action gate state; see maxAdvertisedActionGateBlocks. Only a + // pulse_control call that reached execution counts: a call the FSM refused + // for ordering has not been submitted yet. + controlToolExecutedThisRun := false + advertisedActionGateBlocks := 0 + // Track where each turn's messages begin in providerMessages for compaction. // We keep the last N turns' tool results in full; older ones get compacted. const compactionKeepTurns = 2 // Keep last 2 turns' tool results in full (KA preserves key facts) @@ -1778,6 +1784,41 @@ agenticLoop: } } + // === ADVERTISED-ACTION GATE: an action request ends in pulse_control, not prose === + // The field failure this pins: the operator asks to reboot N guests, + // the model resolves them, then writes a report with "next steps" + // and an invented prerequisite instead of submitting the governed + // action. When the resolved targets advertise the requested + // capability and pulse_control was offered but never submitted, + // refuse the prose ending once and steer to the exact calls. + if !textOnlySafetyBrake && + !controlToolExecutedThisRun && + advertisedActionGateBlocks < maxAdvertisedActionGateBlocks && + !isPatrolDetectionExecution(a.currentExecutionProfile()) && + !isPatrolInvestigationExecution(a.currentExecutionProfile()) && + a.executor != nil && + providerToolOffered(tools, agentcapabilities.PulseControlToolName) { + if action, ok := requestedLifecycleAction(latestUserRequest(messages)); ok { + if targets := a.executor.SessionTargetsAdvertisingAction(action); len(targets) > 0 { + advertisedActionGateBlocks++ + gatePrompt := buildAdvertisedActionGatePrompt(action, targets) + log.Warn(). + Str("session_id", sessionID). + Str("requested_action", action). + Int("advertised_targets", len(targets)). + Int("gate_blocks", advertisedActionGateBlocks). + Msg("[AgenticLoop] Refused prose-only ending for an advertised action request (advertised-action gate)") + // The premature prose stays in the transcript (it was + // already streamed); the correction is a provider-only + // user-role anchor so the next turn can submit the action. + providerMessages = appendFSMVerificationPrompt(providerMessages, gatePrompt) + currentTurnStartIndex = len(providerMessages) + turn++ + continue + } + } + } + // === FSM ENFORCEMENT GATE 2: Check if final answer is allowed === a.mu.Lock() fsm := a.sessionFSM @@ -2274,6 +2315,12 @@ agenticLoop: pendingExec = append(pendingExec, pendingToolExec{tc: tc, toolKind: toolKind}) // A real tool attempt satisfies the look-before-asking gate. lookGateToolAttempted = true + if tc.Name == agentcapabilities.PulseControlToolName { + // A submitted governed action satisfies the advertised-action + // gate whatever the plan outcome: a real boundary from this + // call is evidence the model may report. + controlToolExecutedThisRun = true + } } // --- Phase 2: Execute pending tools --- diff --git a/internal/ai/chat/agentic_action_gate.go b/internal/ai/chat/agentic_action_gate.go new file mode 100644 index 000000000..3d406f61f --- /dev/null +++ b/internal/ai/chat/agentic_action_gate.go @@ -0,0 +1,94 @@ +package chat + +import ( + "fmt" + "regexp" + "strings" + + "github.com/rcourtman/pulse-go-rewrite/internal/ai/providers" + "github.com/rcourtman/pulse-go-rewrite/internal/ai/tools" +) + +// maxAdvertisedActionGateBlocks bounds the advertised-action gate. When the +// user asked for a lifecycle action, the session has resolved resources that +// advertise it, pulse_control was offered, and the model still ends the run +// with prose instead of a governed call, the gate refuses that final answer +// once and steers the model to submit pulse_control for each target. It fails +// open after this many refusals so a model that has a genuine reason not to +// act (which it must then state from tool evidence) cannot livelock. +const maxAdvertisedActionGateBlocks = 1 + +// lifecycleRequestPatterns maps operator phrasing to the canonical lifecycle +// verb pulse_control accepts. Order matters: "restart" must resolve before the +// bare "start" pattern is considered, and the reboot/restart pair is folded +// to one verb because the action lifecycle treats them as synonyms. +var lifecycleRequestPatterns = []struct { + action string + pattern *regexp.Regexp +}{ + {action: "reboot", pattern: regexp.MustCompile(`\b(reboot|restart|power[- ]?cycle|bounce)\b`)}, + {action: "shutdown", pattern: regexp.MustCompile(`\b(shut ?down|power[- ]?off|halt)\b`)}, + {action: "stop", pattern: regexp.MustCompile(`\bstop\b`)}, + {action: "start", pattern: regexp.MustCompile(`\b(start|boot|power[- ]?on|bring up|spin up)\b`)}, +} + +// interrogativeLead matches messages that ask about an action rather than +// request one ("why did X reboot?", "is it safe to stop Y?"). Those must keep +// their investigative answer; the gate only applies to action requests. +var interrogativeLead = regexp.MustCompile(`^\s*(why|what|when|where|who|how|did|was|were|is|are|has|have|should|do|does|which|whether)\b`) + +// requestedLifecycleAction reports the canonical lifecycle verb an operator +// message asks Pulse to perform, if any. +func requestedLifecycleAction(userText string) (string, bool) { + text := strings.ToLower(strings.TrimSpace(userText)) + if text == "" || interrogativeLead.MatchString(text) { + return "", false + } + for _, candidate := range lifecycleRequestPatterns { + if candidate.pattern.MatchString(text) { + return candidate.action, true + } + } + return "", false +} + +// latestUserRequest returns the most recent operator message in the run's +// input transcript, ignoring tool-result carriers that share the user role. +func latestUserRequest(messages []Message) string { + for i := len(messages) - 1; i >= 0; i-- { + msg := messages[i] + if msg.Role != "user" || msg.ToolResult != nil { + continue + } + if content := strings.TrimSpace(msg.Content); content != "" { + return content + } + } + return "" +} + +func providerToolOffered(offered []providers.Tool, name string) bool { + for _, tool := range offered { + if strings.TrimSpace(tool.Name) == name { + return true + } + } + return false +} + +// buildAdvertisedActionGatePrompt is the user-role correction injected when +// the gate refuses a prose-only ending. It names the exact calls to make and +// forbids the invented-prerequisite failure mode seen in the field. +func buildAdvertisedActionGatePrompt(action string, targets []tools.AdvertisedActionTarget) string { + lines := make([]string, 0, len(targets)) + for _, target := range targets { + lines = append(lines, fmt.Sprintf("- pulse_control {\"type\":\"resource\",\"resource_id\":%q,\"action\":%q} (%s %s)", target.CanonicalID, target.Capability, target.Kind, target.Name)) + } + noun := "resource advertises" + if len(targets) != 1 { + noun = "resources advertise" + } + return fmt.Sprintf(`BLOCKED: the user asked you to %s, and %d resolved %s that capability right now. Pulse offers pulse_control for exactly this, so a final answer that narrates next steps, manual commands, or prerequisites is not acceptable. Submit the governed action for each target now, one call per target, using the canonical resource id: +%s +Pulse owns planning, approval, execution, and verification from there; the user approves in Pulse, not by running commands. If a tool result in this turn reported a real boundary for a target, quote that exact result for that target instead. Do not invent prerequisites such as discovery, session or context binding, or guest-agent availability.`, action, len(targets), noun, strings.Join(lines, "\n")) +} diff --git a/internal/ai/chat/agentic_action_gate_test.go b/internal/ai/chat/agentic_action_gate_test.go new file mode 100644 index 000000000..1b78f6f97 --- /dev/null +++ b/internal/ai/chat/agentic_action_gate_test.go @@ -0,0 +1,336 @@ +package chat + +// Regression transcript for GitHub issue #1782. A paying operator asked the +// Assistant (Gemini, Controlled mode) to reboot five Windows VMs matching a +// name pattern. The model resolved all five with pulse_query, then ended the +// run with a markdown report ("Next steps", an invented prerequisite) and +// never submitted pulse_control. The expected behaviour is one governed +// pulse_control plan per target, each awaiting approval in Pulse. +// +// The scripted provider below reproduces the field transcript turn by turn. +// On a build without the advertised-action gate the run ends at turn 2 with +// the report and zero pulse_control calls, which is the failing assertion. + +import ( + "context" + "encoding/json" + "fmt" + "strings" + "sync" + "testing" + + "github.com/rcourtman/pulse-go-rewrite/internal/ai/providers" + "github.com/rcourtman/pulse-go-rewrite/internal/ai/tools" + "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" + "github.com/stretchr/testify/require" +) + +const gateTestReport = `## Summary +I found five Windows VMs matching "win": win-01, win-02, win-03, win-04, win-05. + +## Limitation +The reboot could not be scheduled because these VMs are not yet bound to a discovery session in the current context. + +## Next steps +1. Run a discovery for the VMs. +2. Ask me again and I will reboot them.` + +func gateTestProxmoxVM(name string, vmid int) unifiedresources.Resource { + capabilities := []unifiedresources.ResourceCapability{} + for _, operation := range []string{"shutdown", "reboot", "stop"} { + capabilities = append(capabilities, unifiedresources.ResourceCapability{ + Name: operation, + Type: unifiedresources.CapabilityTypeCommon, + Description: "Proxmox VM lifecycle " + operation, + MinimumApprovalLevel: unifiedresources.ApprovalAdmin, + Platform: "qemu", + InternalHandler: "proxmox.vm.lifecycle", + }) + } + return unifiedresources.Resource{ + ID: "vm-pve-" + name, + Type: unifiedresources.ResourceTypeVM, + Name: name, + Status: unifiedresources.StatusOnline, + ParentName: "pve", + Capabilities: capabilities, + Proxmox: &unifiedresources.ProxmoxData{ + SourceID: fmt.Sprintf("pve:pve:%d", vmid), + NodeName: "pve", + Instance: "pve", + VMID: vmid, + }, + } +} + +type gateTestPlanner struct { + mu sync.Mutex + requests []unifiedresources.ActionRequest +} + +func (p *gateTestPlanner) PlanTypedAction(_ context.Context, _ string, req unifiedresources.ActionRequest) (*unifiedresources.ActionPlan, error) { + p.mu.Lock() + defer p.mu.Unlock() + p.requests = append(p.requests, req) + return &unifiedresources.ActionPlan{ + ActionID: fmt.Sprintf("action-%d", len(p.requests)), + RequestID: req.RequestID, + Allowed: true, + RequiresApproval: true, + ApprovalPolicy: unifiedresources.ApprovalAdmin, + PlanHash: "hash", + }, nil +} + +func (p *gateTestPlanner) snapshot() []unifiedresources.ActionRequest { + p.mu.Lock() + defer p.mu.Unlock() + return append([]unifiedresources.ActionRequest(nil), p.requests...) +} + +func newGateTestExecutor(t *testing.T, planner tools.TypedActionPlanner, vms ...unifiedresources.Resource) *tools.PulseToolExecutor { + t.Helper() + exec := tools.NewPulseToolExecutor(tools.ExecutorConfig{ + StateProvider: &mockStateProvider{}, + AgentServer: &mockAgentServer{}, + UnifiedResourceProvider: plainTextResourceTestProvider(vms...), + TypedActionPlanner: planner, + ControlLevel: tools.ControlLevelControlled, + }) + exec.SetResolvedContext(NewResolvedContext("gate-session")) + return exec +} + +func gateTestControlCalls(vms []unifiedresources.Resource, action string) []providers.ToolCall { + calls := make([]providers.ToolCall, 0, len(vms)) + for i, vm := range vms { + calls = append(calls, providers.ToolCall{ + ID: fmt.Sprintf("c-%d", i+1), + Name: "pulse_control", + Input: map[string]interface{}{ + "type": "resource", + "resource_id": vm.ID, + "action": action, + }, + }) + } + return calls +} + +func gateTestContainsBlock(req providers.ChatRequest) bool { + for _, msg := range req.Messages { + if msg.Role == "user" && strings.Contains(msg.Content, "BLOCKED: the user asked you to reboot") { + return true + } + } + return false +} + +// TestAgenticLoop_BulkLifecycleRequestEndsInPulseControlPlans is the #1782 +// transcript: resolve five VMs, try to end with a report, and prove the run +// instead submits one governed plan per target before answering. +func TestAgenticLoop_BulkLifecycleRequestEndsInPulseControlPlans(t *testing.T) { + vms := []unifiedresources.Resource{ + gateTestProxmoxVM("win-01", 101), + gateTestProxmoxVM("win-02", 102), + gateTestProxmoxVM("win-03", 103), + gateTestProxmoxVM("win-04", 104), + gateTestProxmoxVM("win-05", 105), + } + planner := &gateTestPlanner{} + exec := newGateTestExecutor(t, planner, vms...) + + var ( + mu sync.Mutex + turn int + blockSeenAtTurn int + requestsPerTurn []providers.ChatRequest + reportStreamed bool + finalAnswerTurns int + ) + provider := &stubStreamingProvider{} + provider.chatStream = func(ctx context.Context, req providers.ChatRequest, callback providers.StreamCallback) error { + mu.Lock() + turn++ + current := turn + requestsPerTurn = append(requestsPerTurn, req) + if gateTestContainsBlock(req) && blockSeenAtTurn == 0 { + blockSeenAtTurn = current + } + mu.Unlock() + + switch current { + case 1: + // The model resolves the targets exactly as in the field. + callback(providers.StreamEvent{Type: "done", Data: providers.DoneEvent{ + ToolCalls: []providers.ToolCall{{ + ID: "q-1", + Name: "pulse_query", + Input: map[string]interface{}{"action": "search", "query": "win", "type": "vm"}, + }}, + }}) + case 2: + // The field failure: a report with an invented prerequisite. + mu.Lock() + reportStreamed = true + mu.Unlock() + callback(providers.StreamEvent{Type: "content", Data: providers.ContentEvent{Text: gateTestReport}}) + callback(providers.StreamEvent{Type: "done", Data: providers.DoneEvent{}}) + case 3: + // Steered by the gate, the model submits one plan per target. + require.True(t, gateTestContainsBlock(req), "turn 3 must carry the advertised-action correction") + callback(providers.StreamEvent{Type: "done", Data: providers.DoneEvent{ToolCalls: gateTestControlCalls(vms, "reboot")}}) + case 4: + // Post-write verification read demanded by the FSM. + callback(providers.StreamEvent{Type: "done", Data: providers.DoneEvent{ + ToolCalls: []providers.ToolCall{{ + ID: "q-2", + Name: "pulse_query", + Input: map[string]interface{}{"action": "search", "query": "win", "type": "vm"}, + }}, + }}) + default: + mu.Lock() + finalAnswerTurns++ + mu.Unlock() + callback(providers.StreamEvent{Type: "content", Data: providers.ContentEvent{Text: "Planned a reboot for all five VMs; approve them in Pulse to proceed."}}) + callback(providers.StreamEvent{Type: "done", Data: providers.DoneEvent{}}) + } + return nil + } + + loop := NewAgenticLoop(provider, exec, "base prompt") + loop.SetSessionFSM(NewSessionFSM()) + + messages, err := loop.ExecuteWithTools( + context.Background(), + "gate-session", + []Message{{Role: "user", Content: "Reboot all my Windows VMs whose name starts with win-. There should be five of them."}}, + nil, + func(StreamEvent) {}, + ) + require.NoError(t, err) + + planned := map[string]bool{} + for _, msg := range messages { + if msg.ToolResult != nil && msg.ToolResult.ToolUseID == "q-1" { + require.False(t, msg.ToolResult.IsError, "resolution query must succeed: %s", msg.ToolResult.Content) + require.Contains(t, msg.ToolResult.Content, "win-05", "resolution query must list every target: %s", msg.ToolResult.Content) + } + if msg.ToolResult == nil || !strings.HasPrefix(msg.ToolResult.ToolUseID, "c-") { + continue + } + require.False(t, msg.ToolResult.IsError, "pulse_control must plan, not error: %s", msg.ToolResult.Content) + var payload map[string]any + require.NoError(t, json.Unmarshal([]byte(msg.ToolResult.Content), &payload), msg.ToolResult.Content) + require.Equal(t, true, payload["planned"], payload) + require.Equal(t, true, payload["requires_approval"], "Controlled mode plans wait for approval: %v", payload) + require.Equal(t, "reboot", payload["capability"], payload) + planned[fmt.Sprint(payload["resource_id"])] = true + } + require.Len(t, planned, len(vms), "one governed plan per resolved target; got %v", planned) + + requests := planner.snapshot() + require.Len(t, requests, len(vms)) + for _, req := range requests { + require.True(t, strings.HasPrefix(req.ResourceID, "vm-pve-win-0"), "plans must carry the canonical unified id, got %q", req.ResourceID) + require.Equal(t, "reboot", req.CapabilityName) + } + + mu.Lock() + defer mu.Unlock() + require.True(t, reportStreamed, "the scripted field report must have been produced") + require.Equal(t, 3, blockSeenAtTurn, "the gate must refuse the report once and steer the very next turn") + require.Equal(t, 1, finalAnswerTurns, "after planning and verifying, the answer is accepted") + require.Len(t, requestsPerTurn, 5) + require.True(t, hasFinalAssistantText(messages)) +} + +// TestAgenticLoop_AdvertisedActionGateFailsOpenAfterOneRefusal pins the +// bounded escape hatch: a model that still answers in prose after the single +// correction is not livelocked, and the run ends with its answer. +func TestAgenticLoop_AdvertisedActionGateFailsOpenAfterOneRefusal(t *testing.T) { + vms := []unifiedresources.Resource{gateTestProxmoxVM("win-01", 101)} + planner := &gateTestPlanner{} + exec := newGateTestExecutor(t, planner, vms...) + + turn := 0 + provider := &stubStreamingProvider{} + provider.chatStream = func(ctx context.Context, req providers.ChatRequest, callback providers.StreamCallback) error { + turn++ + if turn == 1 { + callback(providers.StreamEvent{Type: "done", Data: providers.DoneEvent{ + ToolCalls: []providers.ToolCall{{ID: "q-1", Name: "pulse_query", Input: map[string]interface{}{"action": "search", "query": "win"}}}, + }}) + return nil + } + callback(providers.StreamEvent{Type: "content", Data: providers.ContentEvent{Text: "I will not reboot win-01: its console shows an in-progress Windows update (pulse_read evidence above)."}}) + callback(providers.StreamEvent{Type: "done", Data: providers.DoneEvent{}}) + return nil + } + + loop := NewAgenticLoop(provider, exec, "base prompt") + loop.SetSessionFSM(NewSessionFSM()) + messages, err := loop.ExecuteWithTools(context.Background(), "gate-failopen", []Message{{Role: "user", Content: "please reboot win-01"}}, nil, func(StreamEvent) {}) + require.NoError(t, err) + require.Equal(t, 3, turn, "query, refused prose, accepted prose") + require.Empty(t, planner.snapshot(), "the gate steers; it never submits on the model's behalf") + require.True(t, hasFinalAssistantText(messages)) +} + +// TestAgenticLoop_AdvertisedActionGateLeavesQuestionsAlone pins that an +// operator asking *about* a lifecycle event keeps a normal investigative +// answer: the gate only applies to action requests. +func TestAgenticLoop_AdvertisedActionGateLeavesQuestionsAlone(t *testing.T) { + vms := []unifiedresources.Resource{gateTestProxmoxVM("win-01", 101)} + planner := &gateTestPlanner{} + exec := newGateTestExecutor(t, planner, vms...) + + turn := 0 + provider := &stubStreamingProvider{} + provider.chatStream = func(ctx context.Context, req providers.ChatRequest, callback providers.StreamCallback) error { + turn++ + require.False(t, gateTestContainsBlock(req), "a question must never trip the advertised-action gate") + if turn == 1 { + callback(providers.StreamEvent{Type: "done", Data: providers.DoneEvent{ + ToolCalls: []providers.ToolCall{{ID: "q-1", Name: "pulse_query", Input: map[string]interface{}{"action": "search", "query": "win"}}}, + }}) + return nil + } + callback(providers.StreamEvent{Type: "content", Data: providers.ContentEvent{Text: "win-01 is running; nothing in the current state explains a reboot."}}) + callback(providers.StreamEvent{Type: "done", Data: providers.DoneEvent{}}) + return nil + } + + loop := NewAgenticLoop(provider, exec, "base prompt") + loop.SetSessionFSM(NewSessionFSM()) + _, err := loop.ExecuteWithTools(context.Background(), "gate-question", []Message{{Role: "user", Content: "Why did win-01 reboot last night?"}}, nil, func(StreamEvent) {}) + require.NoError(t, err) + require.Equal(t, 2, turn) + require.Empty(t, planner.snapshot()) +} + +func TestRequestedLifecycleAction(t *testing.T) { + cases := []struct { + text string + action string + ok bool + }{ + {"Reboot all my Windows VMs matching win-*", "reboot", true}, + {"can you restart the five win VMs?", "reboot", true}, + {"Please power-cycle win-01", "reboot", true}, + {"shut down win-02 gracefully", "shutdown", true}, + {"stop win-03 now", "stop", true}, + {"start win-04 again", "start", true}, + {"Why did win-01 reboot last night?", "", false}, + {"Is it safe to stop win-02?", "", false}, + {"Should I restart win-03?", "", false}, + {"how is my infrastructure doing?", "", false}, + {"", "", false}, + } + for _, tc := range cases { + action, ok := requestedLifecycleAction(tc.text) + require.Equal(t, tc.ok, ok, tc.text) + require.Equal(t, tc.action, action, tc.text) + } +} diff --git a/internal/ai/chat/fsm.go b/internal/ai/chat/fsm.go index edf37250b..d971ede7d 100644 --- a/internal/ai/chat/fsm.go +++ b/internal/ai/chat/fsm.go @@ -154,7 +154,7 @@ func (fsm *SessionFSM) CanExecuteTool(kind ToolKind, toolName string) error { State: fsm.State, ToolName: toolName, ToolKind: kind, - Reason: "POLICY_BLOCKED: state-changing tools require a validated target/resource context. Establish the target from current context or available tools, then retry only if the requested action still fits.", + Reason: "POLICY_BLOCKED: no resource has been resolved in this session yet, so this action cannot be bound to a canonical target. This is an ordering rule, not a missing prerequisite and not a limitation to report to the user: resolve the target with a read-only call first (for example pulse_query action=search query=), then retry this exact call.", Recoverable: true, } } diff --git a/internal/ai/chat/service.go b/internal/ai/chat/service.go index a28b9a117..aad96c31b 100644 --- a/internal/ai/chat/service.go +++ b/internal/ai/chat/service.go @@ -3861,6 +3861,8 @@ func (s *Service) buildSystemPromptWithToolGovernance(toolGovernance string) str - Not every VM or container supports control. Some API-backed platforms are read-only even when the resource type is "vm" or "system-container". - A Proxmox VM or LXC reboot is a hypervisor lifecycle action and does not require the QEMU guest agent. When the canonical resource advertises the requested lifecycle capability, use pulse_control and let Pulse enforce planning, approval, execution, and verification. - Never redirect the user to manual qm or pct commands when Pulse offers the requested lifecycle capability. Do not claim that a guest agent or capability is unavailable without current tool evidence; if a governed tool reports a real boundary, explain that exact result instead of inventing a prerequisite. +- A recoverable block that names the read-only step to take first (resolve the target, then retry) is an ordering rule, not a boundary: complete that step and retry the same governed call. Never present it to the user as a prerequisite, a session or discovery binding, or a limitation. +- When the user asks for an action and a canonical resource advertises that capability, submit pulse_control for each target and let planning, approval, execution, and verification decide availability. Report a limitation only from a tool result in this turn, never from an assumed prerequisite, and never hand the user manual steps for an action Pulse offers. - Write tools are allowed only when the user explicitly asks you to perform an action. - Status checks and monitoring are read-oriented; do not change state unless the user asked for a state change. - Resolve a missing target yourself before asking: use read-only query/topology tools to see which hosts, VMs, or containers exist. If exactly one plausible target matches the request (a single connected host, or one resource by that name), run read-only diagnostics against it and name the target in your answer instead of asking. diff --git a/internal/ai/chat/service_tooling_test.go b/internal/ai/chat/service_tooling_test.go index 2af0ab0e1..87aac0717 100644 --- a/internal/ai/chat/service_tooling_test.go +++ b/internal/ai/chat/service_tooling_test.go @@ -333,6 +333,17 @@ func TestBuildSystemPrompt_DoesNotClaimGenericVMControl(t *testing.T) { if !strings.Contains(prompt, "Never redirect the user to manual qm or pct commands when Pulse offers the requested lifecycle capability") { t.Fatalf("expected system prompt to keep offered Proxmox lifecycle actions on the governed Pulse path, got %q", prompt) } + for _, want := range []string{ + "is an ordering rule, not a boundary", + "Never present it to the user as a prerequisite, a session or discovery binding, or a limitation.", + "submit pulse_control for each target", + "Report a limitation only from a tool result in this turn", + "never hand the user manual steps for an action Pulse offers", + } { + if !strings.Contains(prompt, want) { + t.Fatalf("expected system prompt to carry the advertised-action contract (#1782) %q", want) + } + } if !strings.Contains(prompt, "Do not claim that a guest agent or capability is unavailable without current tool evidence") { t.Fatalf("expected system prompt to prohibit invented guest-agent limitations, got %q", prompt) } diff --git a/internal/ai/chat/types.go b/internal/ai/chat/types.go index 4dfd8f22a..2f307942c 100644 --- a/internal/ai/chat/types.go +++ b/internal/ai/chat/types.go @@ -1352,6 +1352,29 @@ func (rc *ResolvedContext) GetResolvedResourceByAlias(alias string) (tools.Resol return res, true } +// ListResolvedResources returns every live resource the session has resolved, +// ordered by canonical ID. It does not touch LRU state: enumeration is +// evidence gathering for the advertised-action gate, not user access. +func (rc *ResolvedContext) ListResolvedResources() []tools.ResolvedResourceInfo { + rc.mu.Lock() + defer rc.mu.Unlock() + + rc.evictExpired() + + ids := make([]string, 0, len(rc.ResourcesByID)) + for id, res := range rc.ResourcesByID { + if res != nil { + ids = append(ids, id) + } + } + sort.Strings(ids) + resources := make([]tools.ResolvedResourceInfo, 0, len(ids)) + for _, id := range ids { + resources = append(resources, rc.ResourcesByID[id]) + } + return resources +} + // ValidateResourceID checks if a resource ID exists in this context // and returns the resource if valid func (rc *ResolvedContext) ValidateResourceID(resourceID string) (*ResolvedResource, error) { diff --git a/internal/ai/eval/assertions.go b/internal/ai/eval/assertions.go index 3b8ea3b33..9bd83df9f 100644 --- a/internal/ai/eval/assertions.go +++ b/internal/ai/eval/assertions.go @@ -814,3 +814,29 @@ func min(a, b int) int { } return b } + +// AssertToolCallCountAtLeast checks that a tool was called at least min times +// in the step. Bulk action requests plan one governed action per target, so a +// single call for a five-target request is a failure, not a pass. +func AssertToolCallCountAtLeast(toolName string, min int) Assertion { + return func(result *StepResult) AssertionResult { + count := 0 + for _, tc := range result.ToolCalls { + if tc.Name == toolName { + count++ + } + } + if count < min { + return AssertionResult{ + Name: fmt.Sprintf("ToolCallCountAtLeast(%s, %d)", toolName, min), + Passed: false, + Message: fmt.Sprintf("%s was called %d time(s), want at least %d", toolName, count, min), + } + } + return AssertionResult{ + Name: fmt.Sprintf("ToolCallCountAtLeast(%s, %d)", toolName, min), + Passed: true, + Message: fmt.Sprintf("%s was called %d time(s)", toolName, count), + } + } +} diff --git a/internal/ai/eval/branchcov0723pm_test.go b/internal/ai/eval/branchcov0723pm_test.go index ded125807..7f0c02e53 100644 --- a/internal/ai/eval/branchcov0723pm_test.go +++ b/internal/ai/eval/branchcov0723pm_test.go @@ -60,6 +60,7 @@ func branchcov0723pmScenarioCtors() []struct { {"StrictResolutionScenario", StrictResolutionScenario}, {"StrictResolutionRecoveryScenario", StrictResolutionRecoveryScenario}, {"StrictResolutionBlockScenario", StrictResolutionBlockScenario}, + {"ProxmoxBulkLifecycleActionScenario", ProxmoxBulkLifecycleActionScenario}, {"ApprovalScenario", ApprovalScenario}, {"ApprovalComboScenario", ApprovalComboScenario}, {"ApprovalApproveScenario", ApprovalApproveScenario}, @@ -207,7 +208,7 @@ func TestBranchcov0723pm_ScenarioCatalogParity(t *testing.T) { func TestBranchcov0723pm_ScenarioCatalogInvariants(t *testing.T) { ctors := branchcov0723pmScenarioCtors() - require.Len(t, ctors, 36, "catalog table must list every scenario constructor") + require.Len(t, ctors, 37, "catalog table must list every scenario constructor") seen := make(map[string]string, len(ctors)) diff --git a/internal/ai/eval/eval_test.go b/internal/ai/eval/eval_test.go index e12239f80..df9375ce1 100644 --- a/internal/ai/eval/eval_test.go +++ b/internal/ai/eval/eval_test.go @@ -118,6 +118,7 @@ func TestAllScenarios(t *testing.T) { RoutingValidationScenario(), LogTailingScenario(), DiscoveryScenario(), + ProxmoxBulkLifecycleActionScenario(), } allPassed := true @@ -212,3 +213,23 @@ func TestAllPatrolScenarios(t *testing.T) { t.Fatal("One or more patrol scenarios failed") } } + +// TestProxmoxBulkLifecycleAction runs the issue #1782 regression: a bulk VM +// reboot request must end in governed pulse_control plans, not a report. +// Run with: go test -v ./internal/ai/eval -run TestProxmoxBulkLifecycleAction -live +// Set EVAL_LIFECYCLE_PATTERN and EVAL_LIFECYCLE_TARGET_COUNT for the estate. +func TestProxmoxBulkLifecycleAction(t *testing.T) { + if !*runLiveEval { + t.Skip("Skipping live eval test. Use -live flag to run against live Pulse API") + } + + runner := NewRunner(DefaultConfig()) + scenario := ProxmoxBulkLifecycleActionScenario() + + result := runner.RunScenario(scenario) + runner.PrintSummary(result) + + if !result.Passed { + t.Fatalf("Scenario '%s' failed", scenario.Name) + } +} diff --git a/internal/ai/eval/scenarios.go b/internal/ai/eval/scenarios.go index 582648347..04d4e4eb9 100644 --- a/internal/ai/eval/scenarios.go +++ b/internal/ai/eval/scenarios.go @@ -1650,3 +1650,55 @@ func NonInteractiveGuardrailScenario() Scenario { }, } } + +// ProxmoxBulkLifecycleActionScenario pins GitHub issue #1782: an operator asks +// the Assistant to reboot every Proxmox VM matching a name pattern. The +// Assistant must resolve the targets and then submit one governed +// pulse_control plan per target (approval stays in Pulse), never end with a +// report that invents a prerequisite (QEMU guest agent, "discovery binding", +// "session state") or redirects the operator to manual qm/pct commands. +// +// Targets: EVAL_LIFECYCLE_PATTERN (default "win") selects the VM name pattern; +// EVAL_LIFECYCLE_TARGET_COUNT (default 1) is the minimum number of plans the +// step must produce. Approvals raised in chat are denied so the eval never +// executes a reboot; canonical action plans wait for approval in Pulse and +// are left unapproved. +func ProxmoxBulkLifecycleActionScenario() Scenario { + pattern := envOrDefault("EVAL_LIFECYCLE_PATTERN", "win") + minTargets := 1 + if raw := strings.TrimSpace(os.Getenv("EVAL_LIFECYCLE_TARGET_COUNT")); raw != "" { + if _, err := fmt.Sscanf(raw, "%d", &minTargets); err != nil || minTargets < 1 { + minTargets = 1 + } + } + prompt := fmt.Sprintf("Reboot every Proxmox VM whose name contains %q. Do it through Pulse so I can approve each action; do not tell me to run commands myself.", pattern) + + return Scenario{ + Name: "Proxmox Bulk Lifecycle Action", + Description: "Bulk reboot request ends in one pulse_control plan per resolved VM, not a report with invented prerequisites (issue #1782)", + Steps: []Step{ + { + Name: "Reboot VMs by name pattern", + Prompt: prompt, + ApprovalDecision: ApprovalDeny, + ApprovalReason: "eval deny (lifecycle plans stay unapproved)", + Assertions: []Assertion{ + AssertNoError(), + AssertHasContent(), + AssertToolUsed("pulse_query"), + AssertToolUsed("pulse_control"), + AssertToolSequence([]string{"pulse_query", "pulse_control"}), + AssertToolCallCountAtLeast("pulse_control", minTargets), + AssertAnyToolInputContainsAny("pulse_control", "\"reboot\"", "\"restart\""), + AssertToolOutputContainsAny("pulse_control", "\"planned\":true", "\"planned\": true"), + AssertContentOmitsAll( + "qm reboot", "qm start", "qm stop", "pct reboot", "pct start", "pct stop", + "guest agent", "guest-agent", "qemu-guest-agent", + "discovery binding", "discovery session", "session binding", "session state", + ), + AssertNoResourceIdentityQuestion(), + }, + }, + }, + } +} diff --git a/internal/ai/tools/control_targets.go b/internal/ai/tools/control_targets.go new file mode 100644 index 000000000..7d0d0fe8a --- /dev/null +++ b/internal/ai/tools/control_targets.go @@ -0,0 +1,400 @@ +package tools + +import ( + "errors" + "fmt" + "sort" + "strings" + + "github.com/rcourtman/pulse-go-rewrite/internal/actionplanner" + "github.com/rcourtman/pulse-go-rewrite/internal/agentcapabilities" + "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" +) + +// resolvedResourceLister is the optional session-context extension that lets +// the executor enumerate every resource the session has resolved so far. The +// chat ResolvedContext implements it; narrower test doubles may omit it. +type resolvedResourceLister interface { + ListResolvedResources() []ResolvedResourceInfo +} + +// controlTarget is the canonical binding pulse_control plans against. The +// session entry is the alias index the model saw in earlier query output; the +// canonical resource is the unified-inventory record whose ID the shared +// action lifecycle keys on and whose capability list is the only source of +// truth for "advertised". +type controlTarget struct { + session ResolvedResourceInfo + canonical *unifiedresources.Resource +} + +// canonicalID returns the identifier the action lifecycle registry keys on. +// A session-only binding (no unified provider wired) falls back to the +// session ID so narrow deployments keep working. +func (t controlTarget) canonicalID() string { + if t.canonical != nil { + if id := unifiedresources.CanonicalResourceID(t.canonical.ID); id != "" { + return id + } + } + if t.session != nil { + return unifiedresources.CanonicalResourceID(t.session.GetResourceID()) + } + return "" +} + +func (t controlTarget) displayName() string { + if t.canonical != nil { + if name := strings.TrimSpace(resourceDisplayName(*t.canonical)); name != "" { + return name + } + } + if t.session != nil { + for _, alias := range t.session.GetAliases() { + if alias = strings.TrimSpace(alias); alias != "" { + return alias + } + } + return strings.TrimSpace(t.session.GetResourceID()) + } + return "" +} + +// AdvertisedActionTarget names a session-resolved canonical resource that +// currently advertises a lifecycle capability. The agentic loop uses it to +// refuse a final answer that narrates an advertised action instead of +// submitting it through pulse_control. +type AdvertisedActionTarget struct { + CanonicalID string + Name string + Kind string + Capability string +} + +// lifecycleActionSynonym mirrors the action lifecycle's capability synonym +// table: Proxmox guests advertise "reboot" while container platforms +// advertise "restart", and operators use the words interchangeably. +func lifecycleActionSynonym(action string) (string, bool) { + switch strings.ToLower(strings.TrimSpace(action)) { + case "restart": + return "reboot", true + case "reboot": + return "restart", true + } + return "", false +} + +// advertisedCapabilityNames lists the resource's current capability names in a +// stable order for tool evidence. +func advertisedCapabilityNames(resource unifiedresources.Resource) []string { + names := canonicalCapabilityActions(resource) + sort.Strings(names) + return names +} + +// advertisedActionName reports the capability name the resource advertises +// for a requested lifecycle action, following the same synonym rule the +// action lifecycle applies when it plans. +func advertisedActionName(resource unifiedresources.Resource, action string) (string, bool) { + action = strings.ToLower(strings.TrimSpace(action)) + if action == "" { + return "", false + } + if _, found := actionplanner.FindCapability(resource.Capabilities, action); found { + return action, true + } + if synonym, ok := lifecycleActionSynonym(action); ok { + if _, found := actionplanner.FindCapability(resource.Capabilities, synonym); found { + return synonym, true + } + } + return "", false +} + +var controlCandidateResourceTypes = []unifiedresources.ResourceType{ + unifiedresources.ResourceTypeVM, + unifiedresources.ResourceTypeSystemContainer, + unifiedresources.ResourceTypeAppContainer, + unifiedresources.ResourceTypeAgent, +} + +func controlResourceTypeForKind(kind string) (unifiedresources.ResourceType, bool) { + switch strings.ToLower(strings.TrimSpace(kind)) { + case "vm": + return unifiedresources.ResourceTypeVM, true + case "system-container", "lxc": + return unifiedresources.ResourceTypeSystemContainer, true + case "app-container": + return unifiedresources.ResourceTypeAppContainer, true + case "agent", "node", "docker-host": + return unifiedresources.ResourceTypeAgent, true + } + return "", false +} + +// canonicalResourceForResolved maps a session-resolved resource back to its +// unified-inventory record. It matches on the canonical ID that query tools +// register as an alias, then on provider identity (Proxmox VMID on the same +// node, app-container provider ID on the same host, agent name). It never +// matches on host names, IP addresses, or tags, which the alias list also +// carries: those are lookup conveniences, not identity, and a write must not +// bind to a neighbour by accident. +func canonicalResourceForResolved(provider UnifiedResourceProvider, resolved ResolvedResourceInfo) (unifiedresources.Resource, bool) { + if provider == nil || resolved == nil { + return unifiedresources.Resource{}, false + } + resourceType, ok := controlResourceTypeForKind(firstNonEmptyString(resolved.GetKind(), resolved.GetResourceType())) + if !ok { + return unifiedresources.Resource{}, false + } + candidates := provider.GetByType(resourceType) + for _, alias := range resolved.GetAliases() { + alias = strings.TrimSpace(alias) + if alias == "" { + continue + } + for _, resource := range candidates { + if strings.EqualFold(strings.TrimSpace(resource.ID), alias) { + return resource, true + } + } + } + switch resourceType { + case unifiedresources.ResourceTypeVM, unifiedresources.ResourceTypeSystemContainer: + vmid := resolved.GetVMID() + node := strings.TrimSpace(resolved.GetNode()) + if vmid <= 0 { + return unifiedresources.Resource{}, false + } + for _, resource := range candidates { + if resource.Proxmox == nil || resource.Proxmox.VMID != vmid { + continue + } + if node == "" || strings.EqualFold(strings.TrimSpace(resource.Proxmox.NodeName), node) { + return resource, true + } + } + case unifiedresources.ResourceTypeAppContainer: + providerUID := strings.TrimSpace(resolved.GetProviderUID()) + host := strings.TrimSpace(resolved.GetTargetHost()) + if providerUID == "" { + return unifiedresources.Resource{}, false + } + for _, resource := range candidates { + if !strings.EqualFold(strings.TrimSpace(appContainerProviderID(resource)), providerUID) { + continue + } + if host == "" || strings.EqualFold(strings.TrimSpace(canonicalAppContainerHost(resource)), host) { + return resource, true + } + } + case unifiedresources.ResourceTypeAgent: + for _, resource := range candidates { + name := strings.TrimSpace(resourceDisplayName(resource)) + for _, alias := range resolved.GetAliases() { + if name != "" && strings.EqualFold(name, strings.TrimSpace(alias)) { + return resource, true + } + } + } + } + return unifiedresources.Resource{}, false +} + +// canonicalControlCandidates resolves a model-supplied reference against the +// unified inventory. An exact canonical-ID match wins outright; otherwise every +// control-capable resource whose display name or name equals the reference is +// returned so the caller can refuse an ambiguous write instead of guessing. +func canonicalControlCandidates(provider UnifiedResourceProvider, ref string) []unifiedresources.Resource { + ref = strings.TrimSpace(ref) + if provider == nil || ref == "" { + return nil + } + var byName []unifiedresources.Resource + for _, resourceType := range controlCandidateResourceTypes { + for _, resource := range provider.GetByType(resourceType) { + if strings.EqualFold(strings.TrimSpace(resource.ID), ref) { + return []unifiedresources.Resource{resource} + } + if strings.EqualFold(strings.TrimSpace(resourceDisplayName(resource)), ref) || + strings.EqualFold(strings.TrimSpace(resource.Name), ref) { + byName = append(byName, resource) + } + } + } + return byName +} + +// resolveControlTarget binds a pulse_control reference to a canonical +// resource. Session context is consulted first because it is what the model +// just saw, but a reference that is absent from the session and resolves +// uniquely in the unified inventory is registered and accepted: the inventory +// is Pulse's own discovery, so demanding a second in-session "discovery" step +// is an ordering accident, not a safety property. Capability, approval, and +// execution stay with the shared action lifecycle. +func (e *PulseToolExecutor) resolveControlTarget(ref, action string) (controlTarget, *CallToolResult) { + target := controlTarget{} + if e.resolvedContext != nil { + if res, ok := e.resolvedContext.GetResolvedResourceByAlias(ref); ok && res != nil { + target.session = res + } else if res, ok := e.resolvedContext.GetResolvedResourceByID(ref); ok && res != nil { + target.session = res + } + } + + if e.unifiedResourceProvider != nil { + if target.session != nil { + if resource, ok := canonicalResourceForResolved(e.unifiedResourceProvider, target.session); ok { + target.canonical = &resource + } + } + if target.canonical == nil { + candidates := canonicalControlCandidates(e.unifiedResourceProvider, ref) + switch len(candidates) { + case 0: + case 1: + resource := candidates[0] + target.canonical = &resource + if target.session == nil && e.resolvedContext != nil { + if reg, ok := CanonicalHandoffResourceRegistration(e.unifiedResourceProvider, resource.ID, "", string(unifiedresources.ContractResourceType(resource)), ""); ok { + e.registerResolvedResourceWithExplicitAccess(reg) + if res, ok := e.resolvedContext.GetResolvedResourceByID(resource.ID); ok && res != nil { + target.session = res + } else if res, ok := e.resolvedContext.GetResolvedResourceByAlias(reg.Name); ok && res != nil { + target.session = res + } + } + } + default: + ids := make([]string, 0, len(candidates)) + for _, candidate := range candidates { + ids = append(ids, fmt.Sprintf("%s (%s)", candidate.ID, unifiedresources.ContractResourceType(candidate))) + } + sort.Strings(ids) + result := NewToolResponseResult(NewToolBlockedError( + agentcapabilities.ErrCodeInvalidInput, + fmt.Sprintf("%d canonical resources are named %q; call pulse_control again with one of these canonical resource ids: %s.", len(candidates), ref, strings.Join(ids, ", ")), + map[string]interface{}{ + "resource_id": ref, + "action": action, + "candidates": ids, + "policy_boundary": "Ambiguous target reference. Retry with a canonical resource id from this list; this is a lookup detail, not a missing prerequisite.", + }, + )) + return target, &result + } + } + } + + if target.session == nil && target.canonical == nil { + if isStrictResolutionEnabled() && isWriteAction(action) { + if e.telemetryCallback != nil { + e.telemetryCallback.RecordStrictResolutionBlock("pulse_control", action) + } + strict := &ErrStrictResolution{ + ResourceID: ref, + Action: action, + Message: fmt.Sprintf("No resolved or canonical resource matches %q. Call pulse_query action=search query=%q, then retry pulse_control with a returned name or canonical resource id before performing %q.", ref, ref, action), + } + result := NewToolResponseResult(strict.ToToolResponse()) + return target, &result + } + result := NewToolResponseResult(NewToolBlockedError( + agentcapabilities.ErrCodeNotFound, + fmt.Sprintf("No canonical resource matches %q. Call pulse_query action=search query=%q to list matching resources, then call pulse_control again with a returned name or canonical resource id.", ref, ref), + map[string]interface{}{ + "resource_id": ref, + "action": action, + "policy_boundary": "Target lookup miss. Retry with a name or canonical id returned by pulse_query; this is a lookup detail, not a missing prerequisite, and the user does not need to do anything.", + }, + )) + return target, &result + } + + return target, nil +} + +// controlPlanFailureResult turns a planning error into tool evidence the model +// can report faithfully. A capability the resource does not advertise is a +// real boundary and is described with the resource's current capability list; +// anything else is passed through unchanged. +func controlPlanFailureResult(target controlTarget, action string, err error) CallToolResult { + if err == nil { + return NewErrorResult(fmt.Errorf("canonical action planning failed")) + } + if !errors.Is(err, actionplanner.ErrCapabilityNotFound) { + return NewErrorResult(err) + } + name := target.displayName() + details := map[string]interface{}{ + "resource_id": target.canonicalID(), + "requested_action": action, + "policy_boundary": "The resource does not currently advertise this capability. Report exactly this boundary; do not invent other prerequisites or redirect the user to manual commands.", + } + message := fmt.Sprintf("%q is not permitted on %s: the resource does not currently advertise that capability.", action, name) + if target.canonical != nil { + advertised := advertisedCapabilityNames(*target.canonical) + details["advertised_capabilities"] = advertised + details["status"] = string(target.canonical.Status) + if len(advertised) > 0 { + message = fmt.Sprintf("%q is not permitted on %s: it does not currently advertise that capability; its advertised capabilities right now are %s (status %s).", action, name, strings.Join(advertised, ", "), target.canonical.Status) + } else { + message = fmt.Sprintf("%q is not permitted on %s: it does not currently advertise that capability or any other lifecycle capability (status %s).", action, name, target.canonical.Status) + } + } + return NewToolResponseResult(NewToolBlockedError(agentcapabilities.ErrCodeActionNotAllowed, message, details)) +} + +// SessionTargetsAdvertisingAction lists the session-resolved resources whose +// unified-inventory record currently advertises the requested lifecycle +// action (or its lifecycle synonym). It is the evidence behind the agentic +// loop's advertised-action gate. +func (e *PulseToolExecutor) SessionTargetsAdvertisingAction(action string) []AdvertisedActionTarget { + if e == nil || e.resolvedContext == nil || e.unifiedResourceProvider == nil { + return nil + } + lister, ok := e.resolvedContext.(resolvedResourceLister) + if !ok { + return nil + } + action = strings.ToLower(strings.TrimSpace(action)) + if action == "" { + return nil + } + seen := make(map[string]struct{}) + var targets []AdvertisedActionTarget + for _, resolved := range lister.ListResolvedResources() { + if resolved == nil { + continue + } + resource, ok := canonicalResourceForResolved(e.unifiedResourceProvider, resolved) + if !ok { + continue + } + capability, ok := advertisedActionName(resource, action) + if !ok { + continue + } + id := unifiedresources.CanonicalResourceID(resource.ID) + if id == "" { + continue + } + if _, dup := seen[id]; dup { + continue + } + seen[id] = struct{}{} + targets = append(targets, AdvertisedActionTarget{ + CanonicalID: id, + Name: firstNonEmptyString(strings.TrimSpace(resourceDisplayName(resource)), id), + Kind: string(unifiedresources.ContractResourceType(resource)), + Capability: capability, + }) + } + sort.Slice(targets, func(i, j int) bool { + if targets[i].Name == targets[j].Name { + return targets[i].CanonicalID < targets[j].CanonicalID + } + return targets[i].Name < targets[j].Name + }) + return targets +} diff --git a/internal/ai/tools/control_targets_test.go b/internal/ai/tools/control_targets_test.go new file mode 100644 index 000000000..313f04a7e --- /dev/null +++ b/internal/ai/tools/control_targets_test.go @@ -0,0 +1,404 @@ +package tools + +// Regression coverage for GitHub issue #1782: an operator asks the Assistant +// to reboot Proxmox VMs, the model resolves them, and pulse_control must plan +// the advertised lifecycle action rather than manufacture a "discovery", +// "session binding", or guest-agent prerequisite. Three defects produced that +// behaviour and each is pinned here: +// +// 1. pulse_control handed the session-scoped ID (vm::) to the +// action lifecycle, whose registry keys on canonical unified IDs, so a +// Proxmox guest plan could never resolve. +// 2. pulse_control gated the action on the legacy per-executor action list, +// which never knew the canonical "reboot" capability Proxmox guests +// advertise. +// 3. A reference absent from the session context was refused with a +// "discovery is required" message even when the unified inventory +// resolved it uniquely. + +import ( + "context" + "encoding/json" + "fmt" + "strings" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/actionplanner" + "github.com/rcourtman/pulse-go-rewrite/internal/agentcapabilities" + "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" +) + +func controlTestProxmoxVM(name string, vmid int, node string, running bool) unifiedresources.Resource { + status := unifiedresources.StatusOnline + operations := []string{"shutdown", "reboot", "stop"} + if !running { + status = unifiedresources.StatusOffline + operations = []string{"start"} + } + capabilities := make([]unifiedresources.ResourceCapability, 0, len(operations)) + for _, operation := range operations { + capabilities = append(capabilities, unifiedresources.ResourceCapability{ + Name: operation, + Type: unifiedresources.CapabilityTypeCommon, + Description: "Proxmox VM lifecycle " + operation, + MinimumApprovalLevel: unifiedresources.ApprovalAdmin, + Platform: "qemu", + InternalHandler: "proxmox.vm.lifecycle", + }) + } + return unifiedresources.Resource{ + ID: fmt.Sprintf("vm-%s-%s", node, name), + Type: unifiedresources.ResourceTypeVM, + Name: name, + Status: status, + ParentName: node, + Capabilities: capabilities, + Proxmox: &unifiedresources.ProxmoxData{ + SourceID: fmt.Sprintf("%s:%s:%d", node, node, vmid), + NodeName: node, + Instance: node, + VMID: vmid, + }, + } +} + +// listingResolvedContext adds the optional enumeration surface the +// advertised-action gate consults. +type listingResolvedContext struct { + *mockResolvedContext +} + +func (l *listingResolvedContext) ListResolvedResources() []ResolvedResourceInfo { + out := make([]ResolvedResourceInfo, 0, len(l.resources)) + for _, res := range l.resources { + out = append(out, res) + } + return out +} + +func newControlTestResolvedContext() *listingResolvedContext { + return &listingResolvedContext{mockResolvedContext: &mockResolvedContext{ + resources: make(map[string]ResolvedResourceInfo), + aliases: make(map[string]ResolvedResourceInfo), + lastAccessed: make(map[string]time.Time), + }} +} + +type recordedPlan struct { + requests []unifiedresources.ActionRequest +} + +func (r *recordedPlan) planner(err error) typedActionPlannerFunc { + return func(_ context.Context, _ string, req unifiedresources.ActionRequest) (*unifiedresources.ActionPlan, error) { + r.requests = append(r.requests, req) + if err != nil { + return nil, err + } + return &unifiedresources.ActionPlan{ + ActionID: "action-" + req.RequestID, + RequestID: req.RequestID, + Allowed: true, + RequiresApproval: true, + ApprovalPolicy: unifiedresources.ApprovalAdmin, + PlanHash: "hash-1", + }, nil + } +} + +func decodeControlPayload(t *testing.T, result CallToolResult) map[string]any { + t.Helper() + if len(result.Content) == 0 { + t.Fatalf("expected result content, got %+v", result) + } + var payload map[string]any + if err := json.Unmarshal([]byte(result.Content[0].Text), &payload); err != nil { + t.Fatalf("decode result payload %q: %v", result.Content[0].Text, err) + } + return payload +} + +func decodeControlToolResponse(t *testing.T, result CallToolResult) ToolResponse { + t.Helper() + if len(result.Content) == 0 { + t.Fatalf("expected result content, got %+v", result) + } + var response ToolResponse + if err := json.Unmarshal([]byte(result.Content[0].Text), &response); err != nil { + t.Fatalf("decode tool response %q: %v", result.Content[0].Text, err) + } + return response +} + +func TestExecuteControlResource_PlansAdvertisedRebootAgainstCanonicalID(t *testing.T) { + vm := controlTestProxmoxVM("win-01", 101, "pve", true) + provider := &stubUnifiedResourceProvider{resources: []unifiedresources.Resource{vm}} + resolved := newControlTestResolvedContext() + reg, ok := canonicalGuestRegistration("vm", vm) + if !ok { + t.Fatal("expected canonical guest registration") + } + resolved.AddResolvedResource(reg) + + plans := &recordedPlan{} + executor := NewPulseToolExecutor(ExecutorConfig{ + UnifiedResourceProvider: provider, + ControlLevel: ControlLevelControlled, + TypedActionPlanner: plans.planner(nil), + }) + executor.SetResolvedContext(resolved) + + result, err := executor.executeControl(context.Background(), map[string]interface{}{ + "type": "resource", + "resource_id": "win-01", + "action": "reboot", + }) + if err != nil { + t.Fatalf("executeControl: unexpected error: %v", err) + } + if result.IsError { + t.Fatalf("expected planned result, got error %+v", result) + } + if len(plans.requests) != 1 { + t.Fatalf("expected one plan request, got %d", len(plans.requests)) + } + if plans.requests[0].ResourceID != vm.ID { + t.Fatalf("plan ResourceID = %q, want canonical %q (session-scoped ids never resolve in the lifecycle registry)", plans.requests[0].ResourceID, vm.ID) + } + if plans.requests[0].CapabilityName != "reboot" { + t.Fatalf("plan CapabilityName = %q, want reboot", plans.requests[0].CapabilityName) + } + payload := decodeControlPayload(t, result) + if payload["planned"] != true || payload["requires_approval"] != true { + t.Fatalf("expected planned action awaiting approval, got %+v", payload) + } + if payload["resource_id"] != vm.ID || payload["capability"] != "reboot" || payload["resource_name"] != "win-01" { + t.Fatalf("unexpected plan payload %+v", payload) + } + if message, _ := payload["message"].(string); strings.Contains(strings.ToLower(message), "discover") { + t.Fatalf("plan message must not mention discovery: %q", message) + } +} + +func TestExecuteControlResource_RestartMapsToAdvertisedRebootInPayload(t *testing.T) { + vm := controlTestProxmoxVM("win-02", 102, "pve", true) + provider := &stubUnifiedResourceProvider{resources: []unifiedresources.Resource{vm}} + resolved := newControlTestResolvedContext() + reg, _ := canonicalGuestRegistration("vm", vm) + resolved.AddResolvedResource(reg) + + plans := &recordedPlan{} + executor := NewPulseToolExecutor(ExecutorConfig{ + UnifiedResourceProvider: provider, + ControlLevel: ControlLevelControlled, + TypedActionPlanner: plans.planner(nil), + }) + executor.SetResolvedContext(resolved) + + result, err := executor.executeControl(context.Background(), map[string]interface{}{ + "type": "resource", + "resource_id": vm.ID, + "action": "restart", + }) + if err != nil || result.IsError { + t.Fatalf("expected planned result, got err=%v result=%+v", err, result) + } + payload := decodeControlPayload(t, result) + if payload["requested_action"] != "restart" || payload["capability"] != "reboot" { + t.Fatalf("expected restart to surface the advertised reboot capability, got %+v", payload) + } +} + +func TestExecuteControlResource_ResolvesCanonicalTargetWithoutSessionDiscovery(t *testing.T) { + vm := controlTestProxmoxVM("win-03", 103, "pve", true) + provider := &stubUnifiedResourceProvider{resources: []unifiedresources.Resource{vm}} + resolved := newControlTestResolvedContext() + + plans := &recordedPlan{} + executor := NewPulseToolExecutor(ExecutorConfig{ + UnifiedResourceProvider: provider, + ControlLevel: ControlLevelControlled, + TypedActionPlanner: plans.planner(nil), + }) + executor.SetResolvedContext(resolved) + + result, err := executor.executeControl(context.Background(), map[string]interface{}{ + "type": "resource", + "resource_id": "win-03", + "action": "reboot", + }) + if err != nil { + t.Fatalf("executeControl: unexpected error: %v", err) + } + if result.IsError { + t.Fatalf("a reference that resolves uniquely in the unified inventory must plan, got %+v", result) + } + if len(plans.requests) != 1 || plans.requests[0].ResourceID != vm.ID { + t.Fatalf("expected one plan against %q, got %+v", vm.ID, plans.requests) + } + if _, ok := resolved.GetResolvedResourceByAlias("win-03"); !ok { + t.Fatal("expected the canonical target to be registered in session context after planning") + } +} + +func TestExecuteControlResource_UnknownReferenceNamesTheRecoveryCall(t *testing.T) { + provider := &stubUnifiedResourceProvider{resources: []unifiedresources.Resource{controlTestProxmoxVM("win-04", 104, "pve", true)}} + plans := &recordedPlan{} + executor := NewPulseToolExecutor(ExecutorConfig{ + UnifiedResourceProvider: provider, + ControlLevel: ControlLevelControlled, + TypedActionPlanner: plans.planner(nil), + }) + executor.SetResolvedContext(newControlTestResolvedContext()) + + result, err := executor.executeControl(context.Background(), map[string]interface{}{ + "type": "resource", + "resource_id": "no-such-guest", + "action": "reboot", + }) + if err != nil { + t.Fatalf("executeControl: unexpected error: %v", err) + } + if !result.IsError { + t.Fatalf("expected a blocked result for an unknown reference, got %+v", result) + } + if len(plans.requests) != 0 { + t.Fatalf("an unresolved reference must not reach the planner, got %+v", plans.requests) + } + response := decodeControlToolResponse(t, result) + if response.Error == nil || response.Error.Code != agentcapabilities.ErrCodeNotFound { + t.Fatalf("expected %s, got %+v", agentcapabilities.ErrCodeNotFound, response.Error) + } + if !strings.Contains(response.Error.Message, "pulse_query action=search") { + t.Fatalf("lookup miss must name the exact recovery call, got %q", response.Error.Message) + } + lower := strings.ToLower(response.Error.Message + fmt.Sprint(response.Error.Details["policy_boundary"])) + if strings.Contains(lower, "discovery is required") || strings.Contains(lower, "has not been discovered") { + t.Fatalf("lookup miss must not read as a discovery prerequisite: %q", lower) + } +} + +func TestExecuteControlResource_RefusesAmbiguousNameWithCandidates(t *testing.T) { + provider := &stubUnifiedResourceProvider{resources: []unifiedresources.Resource{ + controlTestProxmoxVM("win-05", 105, "pve-a", true), + controlTestProxmoxVM("win-05", 205, "pve-b", true), + }} + plans := &recordedPlan{} + executor := NewPulseToolExecutor(ExecutorConfig{ + UnifiedResourceProvider: provider, + ControlLevel: ControlLevelControlled, + TypedActionPlanner: plans.planner(nil), + }) + executor.SetResolvedContext(newControlTestResolvedContext()) + + result, err := executor.executeControl(context.Background(), map[string]interface{}{ + "type": "resource", + "resource_id": "win-05", + "action": "reboot", + }) + if err != nil { + t.Fatalf("executeControl: unexpected error: %v", err) + } + if !result.IsError || len(plans.requests) != 0 { + t.Fatalf("an ambiguous name must not plan, got result=%+v plans=%+v", result, plans.requests) + } + response := decodeControlToolResponse(t, result) + if response.Error == nil || response.Error.Code != agentcapabilities.ErrCodeInvalidInput { + t.Fatalf("expected %s, got %+v", agentcapabilities.ErrCodeInvalidInput, response.Error) + } + candidates, _ := response.Error.Details["candidates"].([]any) + if len(candidates) != 2 { + t.Fatalf("expected both canonical candidates to be listed, got %+v", response.Error.Details) + } +} + +func TestExecuteControlResource_UnadvertisedCapabilityIsToolEvidence(t *testing.T) { + vm := controlTestProxmoxVM("win-06", 106, "pve", false) // stopped: advertises start only + provider := &stubUnifiedResourceProvider{resources: []unifiedresources.Resource{vm}} + resolved := newControlTestResolvedContext() + reg, _ := canonicalGuestRegistration("vm", vm) + resolved.AddResolvedResource(reg) + + plans := &recordedPlan{} + executor := NewPulseToolExecutor(ExecutorConfig{ + UnifiedResourceProvider: provider, + ControlLevel: ControlLevelControlled, + TypedActionPlanner: plans.planner(fmt.Errorf("plan: %w", actionplanner.ErrCapabilityNotFound)), + }) + executor.SetResolvedContext(resolved) + + result, err := executor.executeControl(context.Background(), map[string]interface{}{ + "type": "resource", + "resource_id": "win-06", + "action": "reboot", + }) + if err != nil { + t.Fatalf("executeControl: unexpected error: %v", err) + } + if !result.IsError { + t.Fatalf("expected a boundary result, got %+v", result) + } + response := decodeControlToolResponse(t, result) + if response.Error == nil || response.Error.Code != agentcapabilities.ErrCodeActionNotAllowed { + t.Fatalf("expected %s, got %+v", agentcapabilities.ErrCodeActionNotAllowed, response.Error) + } + if !strings.Contains(response.Error.Message, "start") || !strings.Contains(response.Error.Message, "reboot") { + t.Fatalf("boundary must name the requested and advertised capabilities, got %q", response.Error.Message) + } + advertised, _ := response.Error.Details["advertised_capabilities"].([]any) + if len(advertised) != 1 || advertised[0] != "start" { + t.Fatalf("expected advertised_capabilities [start], got %+v", response.Error.Details) + } +} + +func TestSessionTargetsAdvertisingAction_UsesCanonicalCapabilities(t *testing.T) { + running := controlTestProxmoxVM("win-07", 107, "pve", true) + stopped := controlTestProxmoxVM("win-08", 108, "pve", false) + provider := &stubUnifiedResourceProvider{resources: []unifiedresources.Resource{running, stopped}} + resolved := newControlTestResolvedContext() + for _, vm := range []unifiedresources.Resource{running, stopped} { + reg, ok := canonicalGuestRegistration("vm", vm) + if !ok { + t.Fatalf("expected registration for %s", vm.Name) + } + resolved.AddResolvedResource(reg) + } + executor := NewPulseToolExecutor(ExecutorConfig{ + UnifiedResourceProvider: provider, + ControlLevel: ControlLevelControlled, + }) + executor.SetResolvedContext(resolved) + + reboot := executor.SessionTargetsAdvertisingAction("reboot") + if len(reboot) != 1 || reboot[0].CanonicalID != running.ID || reboot[0].Capability != "reboot" || reboot[0].Kind != "vm" { + t.Fatalf("expected only the running VM to advertise reboot, got %+v", reboot) + } + // The legacy executor action list never carried "reboot"; the canonical + // capability list is the only source of truth. + if targets := executor.SessionTargetsAdvertisingAction("restart"); len(targets) != 1 || targets[0].Capability != "reboot" { + t.Fatalf("restart must resolve to the advertised reboot synonym, got %+v", targets) + } + start := executor.SessionTargetsAdvertisingAction("start") + if len(start) != 1 || start[0].CanonicalID != stopped.ID { + t.Fatalf("expected only the stopped VM to advertise start, got %+v", start) + } + if targets := executor.SessionTargetsAdvertisingAction("delete"); len(targets) != 0 { + t.Fatalf("expected no targets for an unadvertised action, got %+v", targets) + } +} + +func TestSessionTargetsAdvertisingAction_IgnoresContextsWithoutEnumeration(t *testing.T) { + vm := controlTestProxmoxVM("win-09", 109, "pve", true) + provider := &stubUnifiedResourceProvider{resources: []unifiedresources.Resource{vm}} + resolved := &mockResolvedContext{ + resources: make(map[string]ResolvedResourceInfo), + aliases: make(map[string]ResolvedResourceInfo), + } + reg, _ := canonicalGuestRegistration("vm", vm) + resolved.AddResolvedResource(reg) + executor := NewPulseToolExecutor(ExecutorConfig{UnifiedResourceProvider: provider, ControlLevel: ControlLevelControlled}) + executor.SetResolvedContext(resolved) + if targets := executor.SessionTargetsAdvertisingAction("reboot"); len(targets) != 0 { + t.Fatalf("a context that cannot enumerate must yield no gate evidence, got %+v", targets) + } +} diff --git a/internal/ai/tools/tools_control.go b/internal/ai/tools/tools_control.go index a7d5d341c..b0ff3d91d 100644 --- a/internal/ai/tools/tools_control.go +++ b/internal/ai/tools/tools_control.go @@ -9,6 +9,7 @@ import ( "time" "github.com/google/uuid" + "github.com/rcourtman/pulse-go-rewrite/internal/actionplanner" "github.com/rcourtman/pulse-go-rewrite/internal/agentcapabilities" "github.com/rcourtman/pulse-go-rewrite/internal/agentexec" "github.com/rcourtman/pulse-go-rewrite/internal/ai/approval" @@ -81,24 +82,31 @@ func (e *PulseToolExecutor) executeControlResource(ctx context.Context, args map return NewErrorResult(fmt.Errorf("action is required")), nil } - validation := e.validateResolvedResource(resourceRef, action, true) - if validation.IsBlocked() { - return NewToolResponseResult(validation.StrictError.ToToolResponse()), nil + // Bind the reference to a canonical resource. The legacy per-executor + // allowed-action list is deliberately not consulted here: it predates + // canonical capabilities (Proxmox guests advertise "reboot", the legacy + // list only knew "restart"), and whether the action is available is the + // action lifecycle's decision at plan time, from the resource's own + // advertised capabilities. + target, blocked := e.resolveControlTarget(resourceRef, action) + if blocked != nil { + return *blocked, nil } - if validation.Resource == nil { - if validation.ErrorMsg != "" { - return NewErrorResult(errors.New(validation.ErrorMsg)), nil + // Current capability evidence is the only permitted source of "not + // available". When the canonical record is in hand and does not advertise + // the action (or its lifecycle synonym), answer from that evidence now so + // read-only platforms are refused with the resource's real capability + // list even before the planner runs. The planner applies the same rule. + if target.canonical != nil { + if _, ok := advertisedActionName(*target.canonical, action); !ok { + return controlPlanFailureResult(target, action, actionplanner.ErrCapabilityNotFound), nil } - return NewErrorResult(fmt.Errorf("resource '%s' has not been discovered in this session. Resource discovery is required first", resourceRef)), nil - } - if validation.ErrorMsg != "" { - return NewErrorResult(errors.New(validation.ErrorMsg)), nil } if e.typedActionPlanner == nil { return NewErrorResult(fmt.Errorf("canonical action planning is unavailable")), nil } - resourceID := unifiedresources.CanonicalResourceID(validation.Resource.GetResourceID()) + resourceID := target.canonicalID() if resourceID == "" { return NewErrorResult(fmt.Errorf("resource %q has no canonical resource id", resourceRef)), nil } @@ -110,18 +118,26 @@ func (e *PulseToolExecutor) executeControlResource(ctx context.Context, args map RequestedBy: "pulse_assistant", }) if err != nil { - return NewErrorResult(err), nil + return controlPlanFailureResult(target, action, err), nil + } + capability := action + if target.canonical != nil { + if advertised, ok := advertisedActionName(*target.canonical, action); ok { + capability = advertised + } } return NewJSONResult(map[string]interface{}{ "planned": true, "action_id": plan.ActionID, "resource_id": resourceID, - "capability": action, + "resource_name": target.displayName(), + "requested_action": action, + "capability": capability, "requires_approval": plan.RequiresApproval, "approval_policy": plan.ApprovalPolicy, "plan_hash": plan.PlanHash, "expires_at": plan.ExpiresAt, - "message": "Typed action planned. Approval and execution remain on the canonical action lifecycle.", + "message": "Typed action planned. Pulse owns approval, execution, and verification from here; do not ask the user to run the action manually.", }), nil }