docs(policy,spec): address PR #124 review — migration, tests, godoc

- Fix NewEvaluator/Engine.Evaluator godoc for nil-policy safety enforcement
- Add CHANGELOG [Unreleased] with breaking-change migration guide
- Document tool-level trusted vs requiredFor; plan prefix vs runtime exact match
- Mark MCP SafetyFromMCPMeta/MergeToolSafety as not wired yet
- Validate non-empty spec.safety blocks; export spec.BoolPtr
- Add run_safety CLI fixture: exit 5 from safety only (no Policy)
- Revert testGraphWithTools to production fail-closed defaults
- Add prefix vs exact approval tests; CONTRIBUTING CHANGELOG note

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: leo-aa88

CHANGELOG.md

@@ -0,0 +1,33 @@
+# Changelog
+
+All notable changes to this project are documented in this file.
+
+The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
+
+## [Unreleased]
+
+### Added
+
+- **`spec.safety` on Tool resources** (issue #103): optional `trusted`, `sideEffects`, and `requiresApproval` fields. [NormalizeProjectGraph] materializes fail-closed defaults on load.
+- **Policy safety fallback**: when no `approvals.requiredFor` entry matches the exact `uses` string, [policy.Derive] consults resolved safety metadata. Unattended mutating tools require `--approve` (exit code **5**, `approval_required`).
+- **Plan risk hints** for tools that will require approval at run, including decision source (`explicit_policy_rule`, `safety_metadata`, `fail_closed_default`).
+
+### Changed
+
+- **Breaking — tool calls without explicit policy are no longer unrestricted.** Previously, `CheckToolCall` with a nil [spec.PolicySpec] allowed all tools. Now fail-closed safety always applies from the project graph (even when the workflow omits `spec.policy` or the Policy resource is missing).
+- Tools with **no** `spec.safety` block behave as **untrusted with side effects** after normalization → require `--approve` unless an explicit `approvals.requiredFor` rule matches.
+
+### Migration
+
+1. For **read-only** native or mock tools (echo, fetch, identity), add:
+   ```yaml
+   spec:
+     safety:
+       sideEffects: false
+   ```
+2. For tools where you accept **tool-wide** unattended use but still gate specific operations, set `trusted: true` and list write operations under `Policy.spec.approvals.requiredFor` (exact `uses` strings).
+3. Do **not** set `trusted: true` unless you intend every operation on that tool to run without safety-derived approval; per-action gating remains `requiredFor` only (exact match at runtime).
+
+### Not yet wired
+
+- MCP discovery does **not** yet apply [spec.SafetyFromMCPMeta] / [spec.MergeToolSafety]; author-set `spec.safety` in YAML is the source of truth until MCP merge lands (tracked separately from #103).

CONTRIBUTING.md

@@ -22,6 +22,8 @@ Run **`make`** or **`make help`** for a full list of targets.
 
 ## Before you open a pull request
 
+User-visible behavior changes should include an entry under **[Unreleased]** in [`CHANGELOG.md`](CHANGELOG.md) (especially breaking changes and migrations).
+
 1. **Format** — `make fmt` or ensure `gofmt -l .` prints nothing (same check as CI).
 2. **Static analysis** — `make vet` (or `go vet ./...`).
 3. **Tests** — `make test` (`go test ./... -race`).

internal/cli/run_test.go

@@ -29,6 +29,16 @@ func runPolicyRoot(t *testing.T) string {
 	return abs
 }
 
+func runSafetyRoot(t *testing.T) string {
+	t.Helper()
+	p := filepath.Join("testdata", "run_safety")
+	abs, err := filepath.Abs(p)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return abs
+}
+
 func TestRun_demo_integration_succeeds(t *testing.T) {
 	db := filepath.Join(t.TempDir(), "run-cli.db")
 	root := runProjRoot(t)
@@ -54,6 +64,53 @@ func TestRun_demo_integration_succeeds(t *testing.T) {
 	}
 }
 
+func TestRun_safetyOnlyDenial_exit5(t *testing.T) {
+	db := filepath.Join(t.TempDir(), "run-safety.db")
+	root := runSafetyRoot(t)
+
+	ResetGlobalsForTest()
+	cmd := NewRootCmd()
+	cmd.SetOut(io.Discard)
+	cmd.SetErr(io.Discard)
+	cmd.SetArgs([]string{
+		"run", "workflow/echo",
+		"--project", root,
+		"--state", db,
+		"--input", "topic=x",
+	})
+	err := cmd.Execute()
+	if err == nil {
+		t.Fatal("expected safety-derived policy denial")
+	}
+	if ExitCodeOf(err) != ExitPolicyDenied {
+		t.Fatalf("exit=%d want %d err=%v", ExitCodeOf(err), ExitPolicyDenied, err)
+	}
+}
+
+func TestRun_safetyOnly_withApprove_succeeds(t *testing.T) {
+	db := filepath.Join(t.TempDir(), "run-safety-ok.db")
+	root := runSafetyRoot(t)
+
+	ResetGlobalsForTest()
+	var out bytes.Buffer
+	cmd := NewRootCmd()
+	cmd.SetOut(&out)
+	cmd.SetErr(&out)
+	cmd.SetArgs([]string{
+		"run", "workflow/echo",
+		"--project", root,
+		"--state", db,
+		"--input", "topic=x",
+		"--approve", "tool.helper.echo",
+	})
+	if err := cmd.Execute(); err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(out.String(), "Status: succeeded") {
+		t.Fatalf("output:\n%s", out.String())
+	}
+}
+
 func TestRun_policyDenial_exit5(t *testing.T) {
 	db := filepath.Join(t.TempDir(), "run-pol.db")
 	root := runPolicyRoot(t)

internal/cli/testdata/run_safety/project.yaml

@@ -0,0 +1,12 @@
+apiVersion: agentic.dev/v0
+kind: Project
+metadata:
+  name: safetyproj
+spec:
+  imports:
+    - ./tools.yaml
+    - ./workflow.yaml
+  providers:
+    models:
+      mock:
+        type: mock

internal/cli/testdata/run_safety/schemas/in.json

@@ -0,0 +1,9 @@
+{
+  "$schema": "https://json-schema.org/draft/2020-12/schema",
+  "type": "object",
+  "required": ["topic"],
+  "properties": {
+    "topic": { "type": "string" }
+  },
+  "additionalProperties": true
+}

internal/cli/testdata/run_safety/tools.yaml

@@ -0,0 +1,6 @@
+apiVersion: agentic.dev/v0
+kind: Tool
+metadata:
+  name: helper
+spec:
+  type: native

internal/cli/testdata/run_safety/workflow.yaml

@@ -0,0 +1,15 @@
+apiVersion: agentic.dev/v0
+kind: Workflow
+metadata:
+  name: echo
+spec:
+  input:
+    schema: ./schemas/in.json
+  steps:
+    - id: s1
+      uses: tool.helper.echo
+      with:
+        topic: "${input.topic}"
+  output:
+    value:
+      out: "${steps.s1.output.echo}"

internal/engine/execution_test.go

@@ -16,8 +16,6 @@ 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)
@@ -51,7 +49,7 @@ func TestRun_sequentialToolAndAgent_mockModel(t *testing.T) {
 				Metadata:   spec.Metadata{Name: "helper"},
 				Spec: spec.ToolSpec{
 					Type:   "native",
-					Safety: &spec.ToolSafety{SideEffects: boolPtr(false)},
+					Safety: &spec.ToolSafety{SideEffects: spec.BoolPtr(false)},
 				},
 			},
 		},

internal/plan/risk.go

@@ -260,6 +260,7 @@ func addToolSafetyRisk(add func(string), toolName string, cur policy.ToolDecisio
 	if prev != nil && prev.Decision == policy.DecisionRequireApproval {
 		return
 	}
+	// Plan uses prefix match on tool.<name>. for explicit requiredFor (conservative); runtime matches exact uses.
 	add(fmt.Sprintf(
 		"Tool/%s will require approval at run (decision=%s, source=%s).",
 		toolName, cur.Decision, cur.Source,

internal/policy/derive.go

@@ -50,7 +50,7 @@ func EffectiveToolDecision(graph *spec.ProjectGraph, pol *spec.PolicySpec, toolN
 	toolName = strings.TrimSpace(toolName)
 	safety := resolvedSafetyForTool(graph, toolName)
 	if pol != nil && pol.Approvals != nil {
-		prefix := "tool." + toolName + "."
+		prefix := toolUsesPrefix(toolName)
 		for _, r := range pol.Approvals.RequiredFor {
 			r = strings.TrimSpace(r)
 			if r == prefix || strings.HasPrefix(r, prefix) {
@@ -109,15 +109,21 @@ func checkSafetyDerived(graph *spec.ProjectGraph, call ToolCallContext) error {
 			},
 		)
 	default:
+		// Derive never returns DecisionDeny; reserved for future explicit denylists.
 		return denied(
 			ReasonDenied,
-			"policy: tool denied by safety metadata",
+			fmt.Sprintf("policy: unexpected tool decision %q", td.Decision),
 			call.Uses,
 			map[string]any{"tool": toolName},
 		)
 	}
 }
 
+// toolUsesPrefix is the plan-risk prefix for tool.<name>. (conservative; runtime uses exact uses).
+func toolUsesPrefix(toolName string) string {
+	return "tool." + strings.TrimSpace(toolName) + "."
+}
+
 func actionApproved(uses string, approved []string) bool {
 	u := strings.TrimSpace(uses)
 	for _, a := range approved {