Merge pull request #47 from LAA-Software-Engineering/issue/13-plan-risk

Issue/13 plan risk

Author: leo-aa88

Makefile

@@ -1,7 +1,10 @@
 .PHONY: test build
 
 test:
-	go test ./...
+	go test ./... -race
+
+test-coverage:
+	go test ./... -race -coverprofile=coverage.out
 
 build:
 	mkdir -p bin

internal/plan/doc.go

@@ -1,5 +1,6 @@
 // Package plan computes desired vs current state diffs and risk summaries.
 //
 // Deployment comparison uses canonical JSON from encoding/json and spec_hash = SHA-256(hex)
-// of those bytes (design doc §14.1, issue #12).
+// of those bytes (design doc §14.1, issue #12). [RiskSummary] is filled from Policy, Agent, and
+// Tool diffs (issue #13); tool mutating risk uses [ActionSuggestsWriteSideEffects].
 package plan

internal/plan/output.go

@@ -36,5 +36,11 @@ func FormatPlan(p *Plan) string {
 			fmt.Fprintf(&b, "- delete %s\n", op.Target.String())
 		}
 	}
+	if len(p.Risk.Messages) > 0 {
+		b.WriteString("\nRisk delta:\n")
+		for _, m := range p.Risk.Messages {
+			fmt.Fprintf(&b, "- %s\n", m)
+		}
+	}
 	return strings.TrimSuffix(b.String(), "\n")
 }

internal/plan/plan.go

@@ -35,8 +35,10 @@ type FieldChange struct {
 	New  string
 }
 
-// RiskSummary is reserved for richer risk deltas (§12.2); MVP planner leaves it empty.
-type RiskSummary struct{}
+// RiskSummary carries MVP plan risk signals (design doc §12.2, §10.2).
+type RiskSummary struct {
+	Messages []string
+}
 
 // Planner reads deployment state to compare desired vs applied resources (design doc §5.2).
 type Planner struct {

internal/plan/planner.go

@@ -84,7 +84,8 @@ func (p *Planner) ComputePlan(ctx context.Context, env string, g *spec.ProjectGr
 
 	sortOperations(ops)
 
-	return &Plan{Operations: ops, Risk: RiskSummary{}}, nil
+	risk := summarizeRisks(appliedByID, desiredByID, ops)
+	return &Plan{Operations: ops, Risk: risk}, nil
 }
 
 func desiredRows(g *spec.ProjectGraph) ([]desiredRow, error) {

internal/plan/risk.go

@@ -0,0 +1,264 @@
+package plan
+
+import (
+	"encoding/json"
+	"fmt"
+	"sort"
+	"strings"
+
+	"github.com/LAA-Software-Engineering/agentic-control-plane/internal/spec"
+	"github.com/LAA-Software-Engineering/agentic-control-plane/internal/state"
+)
+
+// ActionSuggestsWriteSideEffects is the MVP heuristic for whether a tool permission "allow"
+// action may grant mutating side effects. It is used when diffing Tool specs and when
+// planning brand-new tools (no prior state). True when s (ASCII case-folding) contains any of:
+//   - "write"  (e.g. issues.write, pull_requests.write)
+//   - "delete"
+//   - "merge"
+//   - ".send"  (e.g. slack.message.send)
+//   - ".post"
+func ActionSuggestsWriteSideEffects(action string) bool {
+	s := strings.ToLower(strings.TrimSpace(action))
+	if s == "" {
+		return false
+	}
+	return strings.Contains(s, "write") ||
+		strings.Contains(s, "delete") ||
+		strings.Contains(s, "merge") ||
+		strings.Contains(s, ".send") ||
+		strings.Contains(s, ".post")
+}
+
+type policySpecRisk struct {
+	Execution *struct {
+		MaxTotalCostUsd float64 `json:"maxTotalCostUsd"`
+	} `json:"execution"`
+	Approvals *struct {
+		RequiredFor []string `json:"requiredFor"`
+	} `json:"approvals"`
+}
+
+type agentSpecRisk struct {
+	Model string `json:"model"`
+}
+
+type toolSpecRisk struct {
+	Permissions *struct {
+		Allow []string `json:"allow"`
+	} `json:"permissions"`
+}
+
+type jsonEnvelope struct {
+	Spec json.RawMessage `json:"spec"`
+}
+
+func summarizeRisks(
+	appliedByID map[string]state.AppliedResource,
+	desiredByID map[string]desiredRow,
+	ops []Operation,
+) RiskSummary {
+	seen := map[string]struct{}{}
+	var msgs []string
+	add := func(s string) {
+		s = strings.TrimSpace(s)
+		if s == "" {
+			return
+		}
+		if _, ok := seen[s]; ok {
+			return
+		}
+		seen[s] = struct{}{}
+		msgs = append(msgs, s)
+	}
+
+	for _, op := range ops {
+		key := resourceMapKey(op.Target.Kind, op.Target.Name)
+		des := desiredByID[key]
+		prev, hadPrev := appliedByID[key]
+
+		var oldJSON string
+		if hadPrev {
+			oldJSON = prev.NormalizedSpecJSON
+		}
+
+		switch op.Target.Kind {
+		case spec.KindPolicy:
+			summarizePolicyRisk(add, op, oldJSON, des.json, hadPrev)
+		case spec.KindAgent:
+			summarizeAgentRisk(add, op, oldJSON, des.json, hadPrev)
+		case spec.KindTool:
+			summarizeToolRisk(add, op, oldJSON, des.json, hadPrev)
+		}
+	}
+
+	sort.Strings(msgs)
+	return RiskSummary{Messages: msgs}
+}
+
+func summarizePolicyRisk(add func(string), op Operation, oldJSON, newJSON string, hadPrev bool) {
+	newPol, ok := parsePolicySpec(newJSON)
+	if !ok {
+		return
+	}
+	newCost := policyMaxCost(newPol)
+	newApprovals := policyApprovals(newPol)
+
+	if op.Action == ActionCreate || !hadPrev {
+		if newCost > 0 {
+			add(fmt.Sprintf("New policy defines a cost ceiling (Policy/%s).", op.Target.Name))
+		}
+		if len(newApprovals) > 0 {
+			add(fmt.Sprintf("New policy defines approval requirements (Policy/%s).", op.Target.Name))
+		}
+		return
+	}
+
+	oldPol, ok := parsePolicySpec(oldJSON)
+	if !ok {
+		return
+	}
+	oldCost := policyMaxCost(oldPol)
+	oldApprovals := policyApprovals(oldPol)
+
+	if newCost > oldCost+1e-9 {
+		add(fmt.Sprintf("Cost ceiling increased (Policy/%s).", op.Target.Name))
+	}
+	for _, a := range oldApprovals {
+		if !containsString(newApprovals, a) {
+			add(fmt.Sprintf("Approval requirements removed for actions (Policy/%s).", op.Target.Name))
+			break
+		}
+	}
+}
+
+func summarizeAgentRisk(add func(string), op Operation, oldJSON, newJSON string, hadPrev bool) {
+	newAg, ok := parseAgentSpec(newJSON)
+	if !ok {
+		return
+	}
+	newModel := strings.TrimSpace(newAg.Model)
+
+	if op.Action == ActionCreate || !hadPrev {
+		if newModel != "" {
+			add(fmt.Sprintf("New agent binds a model (Agent/%s).", op.Target.Name))
+		}
+		return
+	}
+
+	oldAg, ok := parseAgentSpec(oldJSON)
+	if !ok {
+		return
+	}
+	oldModel := strings.TrimSpace(oldAg.Model)
+	if newModel != oldModel && (newModel != "" || oldModel != "") {
+		add(fmt.Sprintf("Agent model changed (Agent/%s).", op.Target.Name))
+	}
+}
+
+func summarizeToolRisk(add func(string), op Operation, oldJSON, newJSON string, hadPrev bool) {
+	newTool, ok := parseToolSpec(newJSON)
+	if !ok {
+		return
+	}
+	newAllows := toolAllows(newTool)
+
+	if op.Action == ActionCreate || !hadPrev {
+		for _, a := range newAllows {
+			if ActionSuggestsWriteSideEffects(a) {
+				add(fmt.Sprintf("New tool may grant write-like permissions (Tool/%s); see ActionSuggestsWriteSideEffects.", op.Target.Name))
+				break
+			}
+		}
+		return
+	}
+
+	oldTool, ok := parseToolSpec(oldJSON)
+	if !ok {
+		return
+	}
+	oldAllows := toolAllows(oldTool)
+	oldSet := make(map[string]struct{}, len(oldAllows))
+	for _, a := range oldAllows {
+		oldSet[strings.TrimSpace(a)] = struct{}{}
+	}
+	for _, a := range newAllows {
+		a = strings.TrimSpace(a)
+		if a == "" {
+			continue
+		}
+		if _, ok := oldSet[a]; ok {
+			continue
+		}
+		if ActionSuggestsWriteSideEffects(a) {
+			add(fmt.Sprintf("New write-like tool permissions added (Tool/%s); see ActionSuggestsWriteSideEffects.", op.Target.Name))
+			break
+		}
+	}
+}
+
+func parsePolicySpec(resourceJSON string) (*policySpecRisk, bool) {
+	var env jsonEnvelope
+	if err := json.Unmarshal([]byte(resourceJSON), &env); err != nil {
+		return nil, false
+	}
+	var p policySpecRisk
+	if err := json.Unmarshal(env.Spec, &p); err != nil {
+		return nil, false
+	}
+	return &p, true
+}
+
+func parseAgentSpec(resourceJSON string) (*agentSpecRisk, bool) {
+	var env jsonEnvelope
+	if err := json.Unmarshal([]byte(resourceJSON), &env); err != nil {
+		return nil, false
+	}
+	var a agentSpecRisk
+	if err := json.Unmarshal(env.Spec, &a); err != nil {
+		return nil, false
+	}
+	return &a, true
+}
+
+func parseToolSpec(resourceJSON string) (*toolSpecRisk, bool) {
+	var env jsonEnvelope
+	if err := json.Unmarshal([]byte(resourceJSON), &env); err != nil {
+		return nil, false
+	}
+	var t toolSpecRisk
+	if err := json.Unmarshal(env.Spec, &t); err != nil {
+		return nil, false
+	}
+	return &t, true
+}
+
+func policyMaxCost(p *policySpecRisk) float64 {
+	if p == nil || p.Execution == nil {
+		return 0
+	}
+	return p.Execution.MaxTotalCostUsd
+}
+
+func policyApprovals(p *policySpecRisk) []string {
+	if p == nil || p.Approvals == nil {
+		return nil
+	}
+	return p.Approvals.RequiredFor
+}
+
+func toolAllows(t *toolSpecRisk) []string {
+	if t == nil || t.Permissions == nil {
+		return nil
+	}
+	return t.Permissions.Allow
+}
+
+func containsString(slice []string, want string) bool {
+	for _, s := range slice {
+		if s == want {
+			return true
+		}
+	}
+	return false
+}