From 6f0b610e390d248345b5729f0d6a1f3e1add4745 Mon Sep 17 00:00:00 2001 From: nfebe Date: Thu, 23 Jul 2026 20:40:35 +0100 Subject: [PATCH] feat(ai): Enforce agent governance policy, step budgets, and dry runs An agent's frontmatter can now declare governance the runtime enforces deterministically, before any tool runs: auto-approved tools execute without pausing, require-approval tools always pause even when read only, and denied tools are never advertised to the model at all. The default is unchanged: a state-changing tool pauses for per-call approval. The policy is re-read from the file each turn, so the file stays the source of truth mid-run. Agents can also raise their per-turn tool budget within a hard ceiling, and a run can be started as a dry run: every state change is declined and reported instead of executed, with nothing pausing. --- docs/AGENTS.md | 21 +++-- internal/ai/agent.go | 62 +++++++++++-- internal/ai/agent_test.go | 44 +++++++++ internal/ai/session.go | 17 +++- internal/api/agent_handlers.go | 12 +++ internal/api/agent_handlers_test.go | 134 ++++++++++++++++++++++++++++ internal/api/ai_session_handlers.go | 55 ++++++++++-- 7 files changed, 322 insertions(+), 23 deletions(-) diff --git a/docs/AGENTS.md b/docs/AGENTS.md index e902814..e6118c6 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -64,16 +64,19 @@ Hard floors live in frontmatter policy, enforced by the runtime before any tool runs: ```yaml +max_steps: 20 # tool rounds per turn, capped at 50 policy: - auto_approve: [github__create_pull_request] # runs without asking - require_approval: [github__merge_pull_request] # always asks, regardless - deny: [control_deployment] # not even registered + auto_approve: [write_deployment_file] # runs without asking + require_approval: [get_deployment_logs] # always asks, even read-only + deny: [control_deployment] # not even advertised to the model ``` -The default is unchanged: any state-changing tool pauses for per-call operator -approval. Policy can only widen approval upward (`auto_approve`) for named -tools, or harden it (`require_approval`, `deny`). A `dry_run` run auto-declines -every mutation and reports what would have happened. +Precedence is deny, then require_approval, then auto_approve. The default is +unchanged: any state-changing tool pauses for per-call operator approval. +The policy is re-read from the file on every turn, so the file stays the +source of truth mid-run. A run started with `dry_run: true` declines every +state change instead of executing or pausing, reporting what would have +happened. ## External tools over MCP @@ -106,8 +109,8 @@ run away. ## Delivery slices -1. Frontmatter policy (`auto_approve` / `require_approval` / `deny`), - `max_steps`, `dry_run` runs. +1. ~~Frontmatter policy (`auto_approve` / `require_approval` / `deny`), + `max_steps`, `dry_run` runs.~~ Shipped. 2. Run lifecycle: `wait_for_event` / `finish`, `waiting` / `completed` statuses, the events endpoint. 3. MCP client and `mcp_servers`. diff --git a/internal/ai/agent.go b/internal/ai/agent.go index 8596d7d..8f2c647 100644 --- a/internal/ai/agent.go +++ b/internal/ai/agent.go @@ -16,13 +16,62 @@ import ( // shared tool set; the file is the whole definition, so it can be read, // edited, and versioned like any other deployment file. type Agent struct { - Name string `json:"name" yaml:"-"` - Description string `json:"description" yaml:"description"` - Scope string `json:"scope" yaml:"scope"` - Deployment string `json:"deployment,omitempty" yaml:"deployment"` - Instructions string `json:"-" yaml:"-"` + Name string `json:"name" yaml:"-"` + Description string `json:"description" yaml:"description"` + Scope string `json:"scope" yaml:"scope"` + Deployment string `json:"deployment,omitempty" yaml:"deployment"` + MaxSteps int `json:"max_steps,omitempty" yaml:"max_steps"` + Policy *AgentPolicy `json:"policy,omitempty" yaml:"policy"` + Instructions string `json:"-" yaml:"-"` } +// AgentPolicy is the agent's governance: deterministic rules the runtime +// enforces before any tool runs, regardless of what the model decides. +// Precedence is Deny, then RequireApproval, then AutoApprove. Without a rule, +// the default holds: state-changing tools pause for per-call approval. +type AgentPolicy struct { + // AutoApprove names state-changing tools that run without pausing. + AutoApprove []string `json:"auto_approve,omitempty" yaml:"auto_approve"` + // RequireApproval names tools that always pause, even read-only ones. + RequireApproval []string `json:"require_approval,omitempty" yaml:"require_approval"` + // Deny names tools the agent can never call; they are not even advertised. + Deny []string `json:"deny,omitempty" yaml:"deny"` +} + +func inList(list []string, name string) bool { + for _, n := range list { + if n == name { + return true + } + } + return false +} + +// Denies reports whether the policy forbids a tool outright. +func (p *AgentPolicy) Denies(name string) bool { + return p != nil && inList(p.Deny, name) +} + +// RequiresPause reports whether a call to the named tool must pause for +// operator approval. mutates is the tool's own classification; policy can +// widen it in either direction, but never past a Deny. +func (p *AgentPolicy) RequiresPause(name string, mutates bool) bool { + if p == nil { + return mutates + } + if inList(p.RequireApproval, name) { + return true + } + if mutates && inList(p.AutoApprove, name) { + return false + } + return mutates +} + +// maxAgentSteps is the hard ceiling on max_steps, so a definition cannot buy +// itself an unbounded loop. +const maxAgentSteps = 50 + var frontmatterPattern = regexp.MustCompile(`(?s)\A---\s*\n(.*?)\n---\s*\n?`) // agentNamePattern keeps agent names path-safe: they become filenames and URL @@ -51,6 +100,9 @@ func ParseAgent(name, content string) (*Agent, error) { if agent.Scope == SessionScopeDeployment && agent.Deployment == "" { return nil, fmt.Errorf("a deployment-scoped agent must name its deployment") } + if agent.MaxSteps < 0 || agent.MaxSteps > maxAgentSteps { + return nil, fmt.Errorf("max_steps must be between 1 and %d", maxAgentSteps) + } agent.Instructions = strings.TrimSpace(body) if agent.Instructions == "" { return nil, fmt.Errorf("the agent has no instructions") diff --git a/internal/ai/agent_test.go b/internal/ai/agent_test.go index 1d70679..7e57af2 100644 --- a/internal/ai/agent_test.go +++ b/internal/ai/agent_test.go @@ -6,6 +6,50 @@ import ( "testing" ) +func TestParseAgentPolicyAndBudget(t *testing.T) { + agent, err := ParseAgent("governed", `--- +max_steps: 20 +policy: + auto_approve: [write_deployment_file] + require_approval: [list_networks] + deny: [control_deployment] +--- +Do the work.`) + if err != nil { + t.Fatal(err) + } + if agent.MaxSteps != 20 { + t.Errorf("max_steps = %d", agent.MaxSteps) + } + p := agent.Policy + if p.Denies("list_networks") || !p.Denies("control_deployment") { + t.Error("deny list misread") + } + if p.RequiresPause("write_deployment_file", true) { + t.Error("an auto-approved write must not pause") + } + if !p.RequiresPause("list_networks", false) { + t.Error("require_approval must pause a read tool") + } + if !p.RequiresPause("run_quick_action", true) { + t.Error("an unlisted mutating tool keeps the default pause") + } + + if _, err := ParseAgent("greedy", "---\nmax_steps: 500\n---\nLoop."); err == nil { + t.Error("max_steps beyond the ceiling must be rejected") + } +} + +func TestNilPolicyKeepsDefaults(t *testing.T) { + var p *AgentPolicy + if p.Denies("anything") { + t.Error("nil policy must deny nothing") + } + if !p.RequiresPause("write_deployment_file", true) || p.RequiresPause("list_networks", false) { + t.Error("nil policy must keep the mutating-pauses default") + } +} + func TestParseAgentWithFrontmatter(t *testing.T) { rt, err := ParseAgent("tidy-logs", `--- description: Trim old logs diff --git a/internal/ai/session.go b/internal/ai/session.go index f13a482..0acb253 100644 --- a/internal/ai/session.go +++ b/internal/ai/session.go @@ -37,7 +37,12 @@ type Session struct { Deployment string `json:"deployment,omitempty"` // Agent names the agent definition this session is a run of, when it was // started from one rather than typed by an operator. - Agent string `json:"agent,omitempty"` + Agent string `json:"agent,omitempty"` + // MaxSteps overrides the per-turn tool-round budget when set. + MaxSteps int `json:"max_steps,omitempty"` + // DryRun marks a run whose state-changing tools are declined instead of + // executed, reporting what would have happened. + DryRun bool `json:"dry_run,omitempty"` AutoRun bool `json:"auto_run"` Status string `json:"status"` Model string `json:"model,omitempty"` @@ -94,8 +99,14 @@ func (s *Session) AddToolResult(call ToolCall, result string) { } // MaxToolSteps is the per-turn cap on consecutive tool rounds, so a -// misbehaving model cannot loop forever. -func (s *Session) MaxToolSteps() int { return maxSessionToolSteps } +// misbehaving model cannot loop forever. An agent may raise it, within the +// definition's validated ceiling. +func (s *Session) MaxToolSteps() int { + if s.MaxSteps > 0 { + return s.MaxSteps + } + return maxSessionToolSteps +} var sessionIDPattern = regexp.MustCompile(`^ais_[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$`) diff --git a/internal/api/agent_handlers.go b/internal/api/agent_handlers.go index 32b14fe..4c3057b 100644 --- a/internal/api/agent_handlers.go +++ b/internal/api/agent_handlers.go @@ -92,9 +92,21 @@ func (s *Server) runAgent(c *gin.Context) { return } + var req struct { + DryRun bool `json:"dry_run"` + } + if c.Request.ContentLength > 0 { + if err := c.ShouldBindJSON(&req); err != nil { + c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + return + } + } + prompt := ai.BuildSessionPrompt(agent.Scope, agent.Deployment, s.config.AI.DocsURL) sess := ai.NewSession(agent.Scope, agent.Deployment, true, sessionActorFrom(c), prompt) sess.Agent = agent.Name + sess.MaxSteps = agent.MaxSteps + sess.DryRun = req.DryRun display := fmt.Sprintf("Run the %q agent", agent.Name) sess.AddUserMessage(s.redactSessionInput(sess, agent.Instructions), display, false) diff --git a/internal/api/agent_handlers_test.go b/internal/api/agent_handlers_test.go index 2d97551..61ca159 100644 --- a/internal/api/agent_handlers_test.go +++ b/internal/api/agent_handlers_test.go @@ -190,6 +190,140 @@ func TestWriteAgentFileToolCreatesRunnableAgent(t *testing.T) { } } +func TestAutoApprovePolicyRunsWriteWithoutPause(t *testing.T) { + s, tmpDir, ts := setupPlanTestServer(t) + s.aiAgents = ai.NewAgentStore(tmpDir) + createTestDeployment(t, tmpDir, "myapp", &models.ServiceMetadata{Name: "myapp"}) + writeAgentFile(t, tmpDir, "fixer.md", + "---\nscope: deployment\ndeployment: myapp\npolicy:\n auto_approve: [write_deployment_file]\n---\nEnsure conf/app.conf sets key to value.") + + s.aiProvider = &scriptedProvider{responses: []*ai.Response{ + {ToolCalls: []ai.ToolCall{{ID: "c1", Name: "write_deployment_file", + Arguments: `{"path":"conf/app.conf","content":"key = value\n"}`}}, Model: "scripted"}, + {Content: "Done.", Model: "scripted"}, + }} + + resp, parsed := doJSON(t, http.MethodPost, ts.URL+"/api/ai/agents/fixer/run", nil) + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, body %v", resp.StatusCode, parsed) + } + if parsed["status"] != "ready" { + t.Fatalf("status = %v, want ready: an auto-approved write must not pause", parsed["status"]) + } + data, err := os.ReadFile(filepath.Join(tmpDir, "myapp", "conf", "app.conf")) + if err != nil || string(data) != "key = value\n" { + t.Errorf("auto-approved write not applied: %q err=%v", string(data), err) + } +} + +func TestRequireApprovalPolicyPausesReadTool(t *testing.T) { + s, tmpDir, ts := setupPlanTestServer(t) + s.aiAgents = ai.NewAgentStore(tmpDir) + writeAgentFile(t, tmpDir, "careful.md", + "---\npolicy:\n require_approval: [list_networks]\n---\nList the networks.") + + s.aiProvider = &scriptedProvider{responses: []*ai.Response{ + {ToolCalls: []ai.ToolCall{{ID: "c1", Name: "list_networks", Arguments: "{}"}}, Model: "scripted"}, + {Content: "Done.", Model: "scripted"}, + }} + + resp, parsed := doJSON(t, http.MethodPost, ts.URL+"/api/ai/agents/careful/run", nil) + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, body %v", resp.StatusCode, parsed) + } + if parsed["status"] != "awaiting_approval" { + t.Errorf("status = %v: require_approval must pause even a read-only tool", parsed["status"]) + } +} + +func TestDenyPolicyHidesTool(t *testing.T) { + s, tmpDir, ts := setupPlanTestServer(t) + s.aiAgents = ai.NewAgentStore(tmpDir) + createTestDeployment(t, tmpDir, "myapp", &models.ServiceMetadata{Name: "myapp"}) + writeAgentFile(t, tmpDir, "restrained.md", + "---\nscope: deployment\ndeployment: myapp\npolicy:\n deny: [control_deployment]\n---\nRestart the deployment.") + + stub := &scriptedProvider{responses: []*ai.Response{ + {ToolCalls: []ai.ToolCall{{ID: "c1", Name: "control_deployment", + Arguments: `{"action":"restart"}`}}, Model: "scripted"}, + {Content: "Could not.", Model: "scripted"}, + }} + s.aiProvider = stub + + resp, parsed := doJSON(t, http.MethodPost, ts.URL+"/api/ai/agents/restrained/run", nil) + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, body %v", resp.StatusCode, parsed) + } + // The denied tool is not registered: the call fails as unknown rather than + // pausing or executing, and the tool is not advertised to the model. + if parsed["status"] != "ready" { + t.Errorf("status = %v", parsed["status"]) + } + for _, tool := range stub.lastReq.Tools { + if tool.Name == "control_deployment" { + t.Error("a denied tool must not be advertised to the model") + } + } +} + +func TestDryRunDeclinesWritesWithoutPausing(t *testing.T) { + s, tmpDir, ts := setupPlanTestServer(t) + s.aiAgents = ai.NewAgentStore(tmpDir) + createTestDeployment(t, tmpDir, "myapp", &models.ServiceMetadata{Name: "myapp"}) + writeAgentFile(t, tmpDir, "fixer.md", + "---\nscope: deployment\ndeployment: myapp\n---\nEnsure conf/app.conf sets key to value.") + + s.aiProvider = &scriptedProvider{responses: []*ai.Response{ + {ToolCalls: []ai.ToolCall{{ID: "c1", Name: "write_deployment_file", + Arguments: `{"path":"conf/app.conf","content":"key = value\n"}`}}, Model: "scripted"}, + {Content: "Reported.", Model: "scripted"}, + }} + + resp, parsed := doJSON(t, http.MethodPost, ts.URL+"/api/ai/agents/fixer/run", + map[string]interface{}{"dry_run": true}) + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, body %v", resp.StatusCode, parsed) + } + if parsed["status"] != "ready" || parsed["dry_run"] != true { + t.Fatalf("status = %v dry_run = %v", parsed["status"], parsed["dry_run"]) + } + if _, err := os.Stat(filepath.Join(tmpDir, "myapp", "conf", "app.conf")); !os.IsNotExist(err) { + t.Fatal("a dry run must not write the file") + } + // The decline is reported to the model as the tool result. + found := false + for _, m := range parsed["messages"].([]interface{}) { + if steps, ok := m.(map[string]interface{})["tool_steps"].([]interface{}); ok { + for _, st := range steps { + if r, _ := st.(map[string]interface{})["result"].(string); strings.Contains(r, "dry run") { + found = true + } + } + } + } + if !found { + t.Error("expected a dry-run decline in the tool results") + } +} + +func TestAgentMaxStepsHonored(t *testing.T) { + s, tmpDir, ts := setupPlanTestServer(t) + s.aiAgents = ai.NewAgentStore(tmpDir) + writeAgentFile(t, tmpDir, "looper.md", "---\nmax_steps: 1\n---\nKeep listing networks.") + + loop := &ai.Response{ToolCalls: []ai.ToolCall{{ID: "c", Name: "list_networks", Arguments: "{}"}}, Model: "scripted"} + stub := &scriptedProvider{responses: []*ai.Response{loop, loop, loop}} + s.aiProvider = stub + + resp, parsed := doJSON(t, http.MethodPost, ts.URL+"/api/ai/agents/looper/run", nil) + if resp.StatusCode != http.StatusOK { + t.Fatalf("status = %d, body %v", resp.StatusCode, parsed) + } + if stub.calls != 1 { + t.Errorf("engine calls = %d, want exactly max_steps", stub.calls) + } +} + func TestRunAgentNotFound(t *testing.T) { s, tmpDir, ts := setupPlanTestServer(t) s.aiAgents = ai.NewAgentStore(tmpDir) diff --git a/internal/api/ai_session_handlers.go b/internal/api/ai_session_handlers.go index 7fec6f3..17d197b 100644 --- a/internal/api/ai_session_handlers.go +++ b/internal/api/ai_session_handlers.go @@ -82,26 +82,54 @@ func (s *Server) advanceSession(c *gin.Context, sess *ai.Session) error { return s.absorbAdvance(sess, conv, from, engine, err) } +// sessionPolicy resolves the governance for a session. A plain session has +// none; an agent run re-reads its definition each turn, so the file stays the +// source of truth. An agent deleted mid-run keeps the default gating. +func (s *Server) sessionPolicy(sess *ai.Session) *ai.AgentPolicy { + if sess.Agent == "" { + return nil + } + agent, err := s.aiAgents.Get(sess.Agent) + if err != nil { + return nil + } + return agent.Policy +} + // aiRunner assembles the runner for one session turn. A session that does not // auto-run pauses before any tools run, surfacing them for per-call approval. -// Even with auto-run on, a batch containing a state-changing tool pauses: -// reads run free, writes always ask. +// Even with auto-run on, a batch containing a state-changing tool pauses, +// unless the agent's governance auto-approves that tool; governance can also +// force a pause for any tool. A dry run declines every state change instead +// of executing or pausing. func (s *Server) aiRunner(c *gin.Context, sess *ai.Session, engine agents.Engine) agents.Runner { + policy := s.sessionPolicy(sess) runner := agents.Runner{ Engine: engine, Harness: agents.Harness{ Model: sess.Model, MaxSteps: sess.MaxToolSteps(), - Tools: s.sessionToolRegistry(c, sess.Deployment), + Tools: s.sessionToolRegistry(c, sess.Deployment, policy), StepLimitMessage: aiStepLimitMessage, }, } + if sess.DryRun { + runner.Authorize = func(_ context.Context, tool agents.Tool, _ agents.ToolCall) error { + return fmt.Errorf("dry run: %s was not executed; it would have run with the given arguments", tool.Name) + } + return runner + } runner.Approve = func(_ context.Context, calls []agents.ToolCall) (bool, error) { if !sess.AutoRun { return false, nil } for _, call := range calls { - if s.toolMutates(call.Name) { + // A denied tool is not registered; the call fails as unknown, so + // pausing for it would stall the run on a tool that cannot exist. + if policy.Denies(call.Name) { + continue + } + if policy.RequiresPause(call.Name, s.toolMutates(call.Name)) { return false, nil } } @@ -112,16 +140,21 @@ func (s *Server) aiRunner(c *gin.Context, sess *ai.Session, engine agents.Engine // sessionToolRegistry exposes the assistant's tools to the runner, each bound to // this request and the session's deployment so per-tool permission and -// protected-mode checks run exactly as they do for a direct tool call. -func (s *Server) sessionToolRegistry(c *gin.Context, deployment string) *agents.Registry { +// protected-mode checks run exactly as they do for a direct tool call. A tool +// the governance denies is not registered at all, so the model never sees it. +func (s *Server) sessionToolRegistry(c *gin.Context, deployment string, policy *ai.AgentPolicy) *agents.Registry { specs := s.aiToolSpecs() tools := make([]agents.Tool, 0, len(specs)) for _, spec := range specs { + if policy.Denies(spec.Name) { + continue + } spec := spec tools = append(tools, agents.Tool{ Name: spec.Name, Description: spec.Description, Parameters: spec.Parameters, + Mutates: s.toolMutates(spec.Name), Handler: func(_ context.Context, raw json.RawMessage) (string, error) { return s.runAITool(c, deployment, ai.ToolCall{Name: spec.Name, Arguments: string(raw)}), nil }, @@ -190,6 +223,7 @@ func (s *Server) sessionResponse(c *gin.Context, sess *ai.Session) { "scope": sess.Scope, "deployment": sess.Deployment, "agent": sess.Agent, + "dry_run": sess.DryRun, "auto_run": sess.AutoRun, "status": sess.Status, "model": sess.Model, @@ -369,6 +403,15 @@ func (s *Server) approveAISessionTools(c *gin.Context) { if decisions == nil { decisions = map[string]bool{} } + // Calls the batch carried along that never needed a pause themselves, reads + // or governance-auto-approved writes, run without an explicit decision. An + // explicit decline from the operator still wins. + policy := s.sessionPolicy(sess) + for _, call := range sess.Pending { + if _, decided := decisions[call.ID]; !decided && !policy.RequiresPause(call.Name, s.toolMutates(call.Name)) { + decisions[call.ID] = true + } + } engine := ai.NewCapturingEngine(s.aiProvider) runner := s.aiRunner(c, sess, engine) conv := &agents.Conversation{Messages: ai.MessagesToAgents(sess.Messages)}