feat(plan): flag tools requiring safety-derived approval in plan risk

leo-aa88 · cursoragent · leo-aa88 · commit a004272208a5 · 2026-06-01T04:29:06.000-03:00
Surface effective decision and source in plan risk summaries. Update
fixtures with explicit safety on demo and test tools so read-only native
helpers and trusted GitHub demos keep expected run behavior.

Co-authored-by: Cursor &lt;cursoragent@cursor.com&gt;
diff --git a/examples/example1/tools/helper.yaml b/examples/example1/tools/helper.yaml
@@ -4,3 +4,5 @@ metadata:
   name: helper
 spec:
   type: native
+  safety:
+    sideEffects: false
diff --git a/examples/pr-review-demo/tools/github.yaml b/examples/pr-review-demo/tools/github.yaml
@@ -4,3 +4,5 @@ metadata:
   name: github
 spec:
   type: native
+  safety:
+    trusted: true
diff --git a/examples/pr-review-github-actions/tools/github.yaml b/examples/pr-review-github-actions/tools/github.yaml
@@ -4,3 +4,5 @@ metadata:
   name: github
 spec:
   type: native
+  safety:
+    trusted: true
diff --git a/examples/pr-review-github/tools/github.yaml b/examples/pr-review-github/tools/github.yaml
@@ -4,3 +4,5 @@ metadata:
   name: github
 spec:
   type: native
+  safety:
+    trusted: true
diff --git a/internal/cli/initembed/tools/helper.yaml b/internal/cli/initembed/tools/helper.yaml
@@ -4,3 +4,5 @@ metadata:
   name: helper
 spec:
   type: native
+  safety:
+    sideEffects: false
diff --git a/internal/cli/testdata/fmt_messy/tool.yaml b/internal/cli/testdata/fmt_messy/tool.yaml
@@ -4,3 +4,5 @@ metadata:
   name: helper
 spec:
   type: native
+  safety:
+    sideEffects: false
diff --git a/internal/cli/testdata/plan_project/tool.yaml b/internal/cli/testdata/plan_project/tool.yaml
@@ -4,3 +4,5 @@ metadata:
   name: helper
 spec:
   type: native
+  safety:
+    sideEffects: false
diff --git a/internal/cli/testdata/run_policy/tools.yaml b/internal/cli/testdata/run_policy/tools.yaml
@@ -4,3 +4,5 @@ metadata:
   name: helper
 spec:
   type: native
+  safety:
+    sideEffects: false
diff --git a/internal/cli/testdata/validate_ok/tool.yaml b/internal/cli/testdata/validate_ok/tool.yaml
@@ -4,3 +4,5 @@ metadata:
   name: helper
 spec:
   type: native
+  safety:
+    sideEffects: false
diff --git a/internal/cli/testdata/wf_tests/tool.yaml b/internal/cli/testdata/wf_tests/tool.yaml
@@ -4,3 +4,5 @@ metadata:
   name: helper
 spec:
   type: native
+  safety:
+    sideEffects: false
diff --git a/internal/engine/execution_test.go b/internal/engine/execution_test.go
@@ -16,6 +16,8 @@ import (
 	"github.com/LAA-Software-Engineering/agentic-control-plane/internal/trace"
 )
 
+func boolPtr(b bool) *bool { v := b; return &v }
+
 func testProjectRoot(t *testing.T) string {
 	t.Helper()
 	_, file, _, ok := runtime.Caller(0)
@@ -47,7 +49,10 @@ func TestRun_sequentialToolAndAgent_mockModel(t *testing.T) {
 				APIVersion: spec.APIVersionV0,
 				Kind:       spec.KindTool,
 				Metadata:   spec.Metadata{Name: "helper"},
-				Spec:       spec.ToolSpec{Type: "native"},
+				Spec: spec.ToolSpec{
+					Type:   "native",
+					Safety: &spec.ToolSafety{SideEffects: boolPtr(false)},
+				},
 			},
 		},
 		Agents: map[string]*spec.AgentResource{
diff --git a/internal/plan/planner.go b/internal/plan/planner.go
@@ -106,7 +106,7 @@ func (p *Planner) ComputePlan(ctx context.Context, env string, g *spec.ProjectGr
 
 	sortOperations(ops)
 
-	risk := summarizeRisks(appliedByID, desiredByID, ops)
+	risk := summarizeRisks(g, appliedByID, desiredByID, ops)
 	fp, err := DeploymentStateFingerprint(ctx, p.Deploy, env, projectName)
 	if err != nil {
 		return nil, err
diff --git a/internal/plan/risk.go b/internal/plan/risk.go
@@ -6,6 +6,7 @@ import (
 	"sort"
 	"strings"
 
+	"github.com/LAA-Software-Engineering/agentic-control-plane/internal/policy"
 	"github.com/LAA-Software-Engineering/agentic-control-plane/internal/spec"
 	"github.com/LAA-Software-Engineering/agentic-control-plane/internal/state"
 )
@@ -47,13 +48,19 @@ type toolSpecRisk struct {
 	Permissions *struct {
 		Allow []string `json:"allow"`
 	} `json:"permissions"`
+	Safety *struct {
+		Trusted          *bool `json:"trusted"`
+		SideEffects      *bool `json:"sideEffects"`
+		RequiresApproval *bool `json:"requiresApproval"`
+	} `json:"safety"`
 }
 
 type jsonEnvelope struct {
 	Spec json.RawMessage `json:"spec"`
 }
 
 func summarizeRisks(
+	g *spec.ProjectGraph,
 	appliedByID map[string]state.AppliedResource,
 	desiredByID map[string]desiredRow,
 	ops []Operation,
@@ -88,7 +95,7 @@ func summarizeRisks(
 		case spec.KindAgent:
 			summarizeAgentRisk(add, op, oldJSON, des.json, hadPrev)
 		case spec.KindTool:
-			summarizeToolRisk(add, op, oldJSON, des.json, hadPrev)
+			summarizeToolRisk(add, g, op, oldJSON, des.json, hadPrev)
 		}
 	}
 
@@ -156,12 +163,13 @@ func summarizeAgentRisk(add func(string), op Operation, oldJSON, newJSON string,
 	}
 }
 
-func summarizeToolRisk(add func(string), op Operation, oldJSON, newJSON string, hadPrev bool) {
+func summarizeToolRisk(add func(string), g *spec.ProjectGraph, op Operation, oldJSON, newJSON string, hadPrev bool) {
 	newTool, ok := parseToolSpec(newJSON)
 	if !ok {
 		return
 	}
 	newAllows := toolAllows(newTool)
+	newDecision := toolPlanDecisionFromGraph(g, op.Target.Name)
 
 	if op.Action == ActionCreate || !hadPrev {
 		for _, a := range newAllows {
@@ -170,6 +178,7 @@ func summarizeToolRisk(add func(string), op Operation, oldJSON, newJSON string,
 				break
 			}
 		}
+		addToolSafetyRisk(add, op.Target.Name, newDecision, nil)
 		return
 	}
 
@@ -195,6 +204,66 @@ func summarizeToolRisk(add func(string), op Operation, oldJSON, newJSON string,
 			break
 		}
 	}
+	oldDecision := toolDecisionFromParsed(g, op.Target.Name, oldTool)
+	addToolSafetyRisk(add, op.Target.Name, newDecision, &oldDecision)
+}
+
+func toolPlanDecisionFromGraph(g *spec.ProjectGraph, toolName string) policy.ToolDecision {
+	if g != nil {
+		for _, pr := range g.Policies {
+			if pr == nil {
+				continue
+			}
+			td := policy.EffectiveToolDecision(g, &pr.Spec, toolName)
+			if td.Decision == policy.DecisionRequireApproval {
+				return td
+			}
+		}
+	}
+	return policy.EffectiveToolDecision(g, nil, toolName)
+}
+
+func toolDecisionFromParsed(g *spec.ProjectGraph, toolName string, parsed *toolSpecRisk) policy.ToolDecision {
+	if g != nil {
+		for _, pr := range g.Policies {
+			if pr == nil {
+				continue
+			}
+			td := policy.EffectiveToolDecision(g, &pr.Spec, toolName)
+			if td.Decision == policy.DecisionRequireApproval {
+				return td
+			}
+		}
+	}
+	var safety *spec.ToolSafety
+	src := policy.SourceFailClosedDefault
+	if parsed != nil && parsed.Safety != nil {
+		safety = &spec.ToolSafety{
+			Trusted:          parsed.Safety.Trusted,
+			SideEffects:      parsed.Safety.SideEffects,
+			RequiresApproval: parsed.Safety.RequiresApproval,
+		}
+		src = policy.SourceSafetyMetadata
+	}
+	resolved := spec.ResolveToolSafety(safety)
+	return policy.ToolDecision{
+		Decision: policy.Derive(resolved),
+		Source:   src,
+		Safety:   resolved,
+	}
+}
+
+func addToolSafetyRisk(add func(string), toolName string, cur policy.ToolDecision, prev *policy.ToolDecision) {
+	if cur.Decision != policy.DecisionRequireApproval {
+		return
+	}
+	if prev != nil && prev.Decision == policy.DecisionRequireApproval {
+		return
+	}
+	add(fmt.Sprintf(
+		"Tool/%s will require approval at run (decision=%s, source=%s).",
+		toolName, cur.Decision, cur.Source,
+	))
 }
 
 func parsePolicySpec(resourceJSON string) (*policySpecRisk, bool) {
diff --git a/internal/plan/risk_safety_test.go b/internal/plan/risk_safety_test.go
@@ -0,0 +1,62 @@
+package plan
+
+import (
+	"context"
+	"strings"
+	"testing"
+
+	"github.com/LAA-Software-Engineering/agentic-control-plane/internal/spec"
+)
+
+func TestRiskSummary_newTool_flagsSafetyApproval(t *testing.T) {
+	g := minimalGraph()
+	g.Tools["risky"] = &spec.ToolResource{
+		APIVersion: spec.APIVersionV0,
+		Kind:       spec.KindTool,
+		Metadata:   spec.Metadata{Name: "risky"},
+		Spec:       spec.ToolSpec{Type: "native"},
+	}
+	spec.NormalizeProjectGraph(g)
+
+	p := NewPlanner(&fakeDeploy{list: nil})
+	pl, err := p.ComputePlan(context.Background(), "dev", g)
+	if err != nil {
+		t.Fatal(err)
+	}
+	var found bool
+	for _, m := range pl.Risk.Messages {
+		if strings.Contains(m, "Tool/risky") && strings.Contains(m, "require approval") {
+			found = true
+			break
+		}
+	}
+	if !found {
+		t.Fatalf("expected safety approval risk, got %#v", pl.Risk.Messages)
+	}
+}
+
+func TestRiskSummary_trustedTool_noSafetyApprovalRisk(t *testing.T) {
+	g := minimalGraph()
+	trusted := true
+	g.Tools["safe"] = &spec.ToolResource{
+		APIVersion: spec.APIVersionV0,
+		Kind:       spec.KindTool,
+		Metadata:   spec.Metadata{Name: "safe"},
+		Spec: spec.ToolSpec{
+			Type:   "native",
+			Safety: &spec.ToolSafety{Trusted: &trusted},
+		},
+	}
+	spec.NormalizeProjectGraph(g)
+
+	p := NewPlanner(&fakeDeploy{list: nil})
+	pl, err := p.ComputePlan(context.Background(), "dev", g)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, m := range pl.Risk.Messages {
+		if strings.Contains(m, "Tool/safe") && strings.Contains(m, "require approval") {
+			t.Fatalf("unexpected approval risk: %s", m)
+		}
+	}
+}
diff --git a/internal/project/testdata/refs_unknown_tool/tools/github.yaml b/internal/project/testdata/refs_unknown_tool/tools/github.yaml
@@ -4,6 +4,8 @@ metadata:
   name: github
 spec:
   type: mcp
+  safety:
+    trusted: true
   mcp:
     transport: stdio
     command: npx
diff --git a/internal/runtime/local/testdata/retention/tools.yaml b/internal/runtime/local/testdata/retention/tools.yaml
@@ -4,3 +4,5 @@ metadata:
   name: helper
 spec:
   type: native
+  safety:
+    sideEffects: false
diff --git a/internal/runtime/local/testdata/runproj/tools.yaml b/internal/runtime/local/testdata/runproj/tools.yaml
@@ -4,3 +4,5 @@ metadata:
   name: helper
 spec:
   type: native
+  safety:
+    sideEffects: false
