From 8f50b8179290c09921ad616dbe3c328983ac4a45 Mon Sep 17 00:00:00 2001 From: BaaRai <130491490+BaaRai@users.noreply.github.com> Date: Mon, 9 Nov 2026 22:17:56 +0100 Subject: [PATCH 1/3] feat: record which actor decided an intercept approval Add intercept_pending.decided_by (human | rule | model | system; empty while pending or for rows decided before the column existed). Approval semantics are unchanged: only the actor is recorded. - rule-matched and model-judged rows written by CreateDecidedIntercept record rule or model; - ResolveIntercept takes the actor explicitly and rejects any other value before touching the database; - Decide (human allow/deny) records human; timeouts and work-cancelled denials record system. Work-cancelled is a system refusal, not a human one. - decided_by is exposed on the pending and approval records and filterable with decided_by=human|rule|model|system (anything else is a 400). - Legacy rows show decision_source as their actor when decided_by is empty. The filter uses the stored column only, so decided_by=model cannot match older human decisions or timeouts. - Archive restore fills decided_by for archives that predate the column, and marks a restored pending row as system. --- db/intercept.go | 35 ++++- db/intercept_decided_by_test.go | 150 +++++++++++++++++++ db/intercept_detail.go | 16 +- db/intercept_detail_test.go | 18 ++- db/schema.sql | 4 + db/task_archives_restore.go | 5 + intercept/intercept.go | 8 +- server/intercept.go | 7 +- server/intercept_decided_by_test.go | 219 ++++++++++++++++++++++++++++ 9 files changed, 437 insertions(+), 25 deletions(-) create mode 100644 db/intercept_decided_by_test.go create mode 100644 server/intercept_decided_by_test.go diff --git a/db/intercept.go b/db/intercept.go index bfeade7..efe7354 100644 --- a/db/intercept.go +++ b/db/intercept.go @@ -27,6 +27,8 @@ type InterceptRule struct { } // InterceptPending is one row of intercept_pending. +// DecidedBy is the actor that settled the request: "" while pending, otherwise +// human | rule | model | system (timeouts and cancellations). type InterceptPending struct { ID int64 `json:"id"` RuleID *int64 `json:"rule_id"` @@ -37,6 +39,7 @@ type InterceptPending struct { ToolInput json.RawMessage `json:"tool_input"` Status string `json:"status"` DecisionSource string `json:"decision_source"` + DecidedBy string `json:"decided_by"` Reason string `json:"reason"` // rule message or model decision reason (prefixed with ModelReasonPrefix) DecidedAt *time.Time `json:"decided_at"` CreatedAt time.Time `json:"created_at"` @@ -163,19 +166,25 @@ func (d *DB) CreateDecidedIntercept(ruleID, convID int64, taskID, agentName, too if ruleID != 0 { ruleIDPtr = &ruleID } + // Rows logged here were settled without a human: by the matching rule, or by + // the model judge when no rule owns the decision. + decidedBy := "model" + if ruleID != 0 { + decidedBy = "rule" + } var id int64 err := d.QueryRow(` -INSERT INTO intercept_pending(rule_id, conversation_id, task_id, agent_name, tool_name, tool_input, status, reason, decided_at, decision_source, audit) -VALUES ($1, $2, $3, $4, $5, $6, $7, $8, NOW(), $9, $10) RETURNING id`, - ruleIDPtr, convIDPtr, taskIDPtr, agentName, toolName, raw, status, reason, interceptSource(ruleID, reason), firstAudit(audits)).Scan(&id) +INSERT INTO intercept_pending(rule_id, conversation_id, task_id, agent_name, tool_name, tool_input, status, reason, decided_at, decision_source, audit, decided_by) +VALUES ($1, $2, $3, $4, $5, $6, $7, $8, NOW(), $9, $10, $11) RETURNING id`, + ruleIDPtr, convIDPtr, taskIDPtr, agentName, toolName, raw, status, reason, interceptSource(ruleID, reason), firstAudit(audits), decidedBy).Scan(&id) return id, err } -const interceptPendingCols = `id, rule_id, conversation_id, task_id, agent_name, tool_name, tool_input, status, reason, decided_at, created_at, decision_source` +const interceptPendingCols = `id, rule_id, conversation_id, task_id, agent_name, tool_name, tool_input, status, reason, decided_at, created_at, decision_source, decided_by` func scanInterceptPending(s interface{ Scan(...any) error }, p *InterceptPending) error { return s.Scan(&p.ID, &p.RuleID, &p.ConversationID, &p.TaskID, &p.AgentName, - &p.ToolName, &p.ToolInput, &p.Status, &p.Reason, &p.DecidedAt, &p.CreatedAt, &p.DecisionSource) + &p.ToolName, &p.ToolInput, &p.Status, &p.Reason, &p.DecidedAt, &p.CreatedAt, &p.DecisionSource, &p.DecidedBy) } // ListPendingIntercepts returns all unresolved approval requests, newest first. @@ -221,7 +230,7 @@ func scanInterceptApprovalRow(rows interface{ Scan(...any) error }, r *Intercept return rows.Scan( &r.ID, &r.RuleID, &r.ConversationID, &r.TaskID, &r.AgentName, &r.ToolName, &r.ToolInput, &r.Status, &r.Reason, &r.DecidedAt, &r.CreatedAt, - &r.DecisionSource, &r.ConvTitle, &r.ConvAgentKey, &r.RuleName, + &r.DecisionSource, &r.DecidedBy, &r.ConvTitle, &r.ConvAgentKey, &r.RuleName, ) } @@ -246,8 +255,14 @@ const approvalDecisionSource = `COALESCE(NULLIF(ip.decision_source,''), CASE WHEN ip.rule_id IS NOT NULL THEN 'rule' WHEN lower(ip.reason) LIKE '` + ModelReasonPrefix + `%' OR ip.reason LIKE '` + legacyModelReasonPrefix + `%' THEN 'model' ELSE 'unknown' END)` +// Rows decided before decided_by existed keep it empty; show their decision +// source instead of rewriting history. A pending row has no deciding actor yet, +// so its empty value is returned as is. +const approvalDecidedBy = `CASE WHEN ip.decided_by<>'' OR ip.status='pending' THEN ip.decided_by ELSE ` + approvalDecisionSource + ` END` + const approvalRowColumns = `ip.id, ip.rule_id, ip.conversation_id, ip.task_id, ip.agent_name, ip.tool_name, ip.tool_input, ip.status, ip.reason, ip.decided_at, ip.created_at, ` + approvalDecisionSource + `, + ` + approvalDecidedBy + `, COALESCE(c.title,'') AS conv_title, COALESCE(c.agent_key,'') AS conv_agent_key, COALESCE(ir.name,'') AS rule_name` @@ -300,11 +315,12 @@ func (d *DB) ListAllIntercepts(limit int) ([]InterceptApprovalRow, error) { return out, rows.Err() } -// InterceptApprovalFilter combines exact status and decision-source filters. -// Empty fields include all values. +// InterceptApprovalFilter combines exact status, decision-source and +// deciding-actor filters. Empty fields include all values. type InterceptApprovalFilter struct { Status string DecisionSource string + DecidedBy string } // ListAllInterceptsPage returns one 1-based page and the total matching count. @@ -358,6 +374,9 @@ func (d *DB) listInterceptsPage(taskID string, page, size int, filter InterceptA add("ip.task_id", taskID) add("ip.status", filter.Status) add(approvalDecisionSource, filter.DecisionSource) + // Stored actor only: the legacy display fallback would let decided_by=model + // match human decisions and timeouts recorded before the column existed. + add("ip.decided_by", filter.DecidedBy) where := "" if len(conditions) > 0 { where = " WHERE " + strings.Join(conditions, " AND ") diff --git a/db/intercept_decided_by_test.go b/db/intercept_decided_by_test.go new file mode 100644 index 0000000..664142e --- /dev/null +++ b/db/intercept_decided_by_test.go @@ -0,0 +1,150 @@ +package db + +import ( + "slices" + "testing" +) + +// The actor whitelist is enforced in Go (there is no SQL CHECK). It must reject +// before touching the database, so this runs without Postgres. +func TestResolveInterceptRejectsUnknownActor(t *testing.T) { + d := &DB{} + for _, actor := range []string{"", "unknown", "Human", "admin"} { + if ok, err := d.ResolveIntercept(1, "allowed", "allow", "fixture", actor); err == nil || ok { + t.Fatalf("decided_by %q accepted: ok=%t err=%v", actor, ok, err) + } + } +} + +func TestInterceptDecidedBy(t *testing.T) { + d, err := Open(testDSN(t)) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = d.Close() }) + scope := "approval-decided-by" + t.Cleanup(func() { _, _ = d.Exec(`DELETE FROM intercept_pending WHERE task_id=$1`, scope) }) + rule, err := d.CreateInterceptRule("decided-by fixture", "tool_name", "string", "Bash", "deny", "fixture", 1, false, false, 0, "deny") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _, _ = d.Exec(`DELETE FROM intercept_rules WHERE id=$1`, rule.ID) }) + + input := []byte(`{"command":"fixture"}`) + decided := func(ruleID int64, status, reason string) int64 { + t.Helper() + id, err := d.CreateDecidedIntercept(ruleID, 0, scope, "test", "Bash", input, status, reason) + if err != nil { + t.Fatal(err) + } + return id + } + asked := func() int64 { + t.Helper() + id, err := d.CreateInterceptPending(0, 0, scope, "test", "Bash", input, ModelReasonPrefix+" confirm") + if err != nil { + t.Fatal(err) + } + return id + } + resolved := func(status, action, reason, actor string) int64 { + t.Helper() + id := asked() + if ok, err := d.ResolveIntercept(id, status, action, reason, actor); err != nil || !ok { + t.Fatalf("resolve %s/%s: ok=%t err=%v", status, actor, ok, err) + } + return id + } + + ruleID := decided(rule.ID, "denied", "fixture") + modelID := decided(0, "allowed", ModelReasonPrefix+" fixture") + pendingID := asked() + humanAllowID := resolved("allowed", "allow", "user approved execution", "human") + humanDenyID := resolved("denied", "deny", "user rejected execution", "human") + timeoutID := resolved("timeout", "deny", "approval timed out, applying timeout policy", "system") + // A cancellation is a system refusal even though its status is 'denied'. + cancelledID := resolved("denied", "deny", "work cancelled", "system") + + want := map[int64]string{ + ruleID: "rule", modelID: "model", pendingID: "", + humanAllowID: "human", humanDenyID: "human", + timeoutID: "system", cancelledID: "system", + } + for id, actor := range want { + stored, err := d.GetInterceptPending(id) + if err != nil || stored == nil || stored.DecidedBy != actor { + t.Fatalf("stored decided_by for %d: %+v err=%v, want %q", id, stored, err, actor) + } + detail, err := d.GetInterceptDetail(id) + if err != nil || detail == nil || detail.DecidedBy != actor { + t.Fatalf("detail decided_by for %d: %+v err=%v, want %q", id, detail, err, actor) + } + } + rows, err := d.ListTaskIntercepts(scope) + if err != nil || len(rows) != len(want) { + t.Fatalf("list: %d rows err=%v", len(rows), err) + } + for _, r := range rows { + if r.DecidedBy != want[r.ID] { + t.Fatalf("listed decided_by for %d = %q, want %q", r.ID, r.DecidedBy, want[r.ID]) + } + } + + // A rejected actor must leave the request pending and unattributed. + if ok, err := d.ResolveIntercept(pendingID, "allowed", "allow", "fixture", "admin"); err == nil || ok { + t.Fatalf("invalid actor resolved the request: ok=%t err=%v", ok, err) + } + if p, err := d.GetInterceptPending(pendingID); err != nil || p.Status != "pending" || p.DecidedBy != "" { + t.Fatalf("pending row changed: %+v err=%v", p, err) + } + + filtered := func(filter InterceptApprovalFilter) []int64 { + t.Helper() + rows, total, err := d.ListTaskInterceptsPage(scope, 1, 100, filter) + if err != nil || total != len(rows) { + t.Fatalf("filter %+v: total=%d rows=%d err=%v", filter, total, len(rows), err) + } + ids := []int64{} + for _, r := range rows { + ids = append(ids, r.ID) + } + slices.Sort(ids) + return ids + } + for _, tc := range []struct { + filter InterceptApprovalFilter + want []int64 + }{ + // Neither the human decisions nor the timeout of a model-raised ask. + {InterceptApprovalFilter{DecidedBy: "model"}, []int64{modelID}}, + {InterceptApprovalFilter{DecidedBy: "rule"}, []int64{ruleID}}, + {InterceptApprovalFilter{DecidedBy: "human"}, []int64{humanAllowID, humanDenyID}}, + {InterceptApprovalFilter{DecidedBy: "system"}, []int64{timeoutID, cancelledID}}, + {InterceptApprovalFilter{Status: "denied", DecidedBy: "human"}, []int64{humanDenyID}}, + {InterceptApprovalFilter{Status: "denied", DecidedBy: "system"}, []int64{cancelledID}}, + {InterceptApprovalFilter{DecidedBy: "human' OR 1=1 --"}, []int64{}}, + } { + if got := filtered(tc.filter); !slices.Equal(got, tc.want) { + t.Fatalf("filter %+v: got %v want %v", tc.filter, got, tc.want) + } + } + + // Rows decided before the column existed: displayed through decision_source, + // never rewritten, and not matched by the stored-actor filter. + if _, err := d.Exec(`UPDATE intercept_pending SET decided_by='' WHERE id IN ($1,$2,$3)`, ruleID, modelID, timeoutID); err != nil { + t.Fatal(err) + } + for id, source := range map[int64]string{ruleID: "rule", modelID: "model", timeoutID: "model"} { + detail, err := d.GetInterceptDetail(id) + if err != nil || detail.DecidedBy != source || detail.DecisionSource != source { + t.Fatalf("legacy fallback for %d: %+v err=%v, want %q", id, detail, err, source) + } + stored, err := d.GetInterceptPending(id) + if err != nil || stored.DecidedBy != "" { + t.Fatalf("legacy row %d rewritten: %+v err=%v", id, stored, err) + } + } + if got := filtered(InterceptApprovalFilter{DecidedBy: "model"}); len(got) != 0 { + t.Fatalf("legacy rows matched decided_by=model: %v", got) + } +} diff --git a/db/intercept_detail.go b/db/intercept_detail.go index 9373f4a..135270b 100644 --- a/db/intercept_detail.go +++ b/db/intercept_detail.go @@ -3,6 +3,7 @@ package db import ( "database/sql" "encoding/json" + "fmt" "time" ) @@ -55,7 +56,7 @@ func (d *DB) GetInterceptDetail(id int64) (*InterceptDetail, error) { err := d.QueryRow(approvalRowSelectWithAudit+` WHERE ip.id=$1`, id).Scan( &out.ID, &out.RuleID, &out.ConversationID, &out.TaskID, &out.AgentName, &out.ToolName, &out.ToolInput, &out.Status, &out.Reason, &out.DecidedAt, &out.CreatedAt, - &out.DecisionSource, &out.ConvTitle, &out.ConvAgentKey, &out.RuleName, &raw, + &out.DecisionSource, &out.DecidedBy, &out.ConvTitle, &out.ConvAgentKey, &out.RuleName, &raw, ) if err == sql.ErrNoRows { return nil, nil @@ -73,7 +74,14 @@ func (d *DB) GetInterceptDetail(id int64) (*InterceptDetail, error) { // ResolveIntercept atomically settles a pending request. A timeout cannot // overwrite a human decision and repeat decisions cannot rewrite history. -func (d *DB) ResolveIntercept(id int64, status, action, reason string) (bool, error) { +// decidedBy records the settling actor (human | rule | model | system); it is +// supplied by the caller because status alone cannot tell them apart. +func (d *DB) ResolveIntercept(id int64, status, action, reason, decidedBy string) (bool, error) { + switch decidedBy { + case "human", "rule", "model", "system": + default: + return false, fmt.Errorf("decided_by must be human, rule, model or system, got %q", decidedBy) + } execution := "not_executed" if action == "allow" { execution = "awaiting_result" @@ -84,11 +92,11 @@ func (d *DB) ResolveIntercept(id int64, status, action, reason string) (bool, er if err != nil { return false, err } - r, err := d.Exec(`UPDATE intercept_pending SET status=$2, decided_at=NOW(), + r, err := d.Exec(`UPDATE intercept_pending SET status=$2, decided_at=NOW(), decided_by=$5, audit=CASE WHEN audit IS NULL THEN NULL ELSE audit || $3::jsonb || CASE WHEN $4='allow' AND audit->>'correlation' IS DISTINCT FROM 'exact' THEN '{"execution_status":"unknown"}'::jsonb ELSE '{}'::jsonb END END - WHERE id=$1 AND status='pending'`, id, status, patch, action) + WHERE id=$1 AND status='pending'`, id, status, patch, action, decidedBy) if err != nil { return false, err } diff --git a/db/intercept_detail_test.go b/db/intercept_detail_test.go index 5769cb0..03ca3e3 100644 --- a/db/intercept_detail_test.go +++ b/db/intercept_detail_test.go @@ -49,7 +49,7 @@ func TestInterceptDetails(t *testing.T) { var wg sync.WaitGroup for range 8 { wg.Go(func() { - ok, err := d.ResolveIntercept(id, "allowed", "allow", "allowed manually") + ok, err := d.ResolveIntercept(id, "allowed", "allow", "allowed manually", "human") if err != nil { t.Error(err) } @@ -62,7 +62,7 @@ func TestInterceptDetails(t *testing.T) { if wins.Load() != 1 { t.Fatalf("%d decisions won", wins.Load()) } - ok, err := d.ResolveIntercept(id, "timeout", "deny", "late timeout") + ok, err := d.ResolveIntercept(id, "timeout", "deny", "late timeout", "system") if err != nil || ok { t.Fatal("timeout overwrote decision") } @@ -90,7 +90,7 @@ func TestInterceptDetails(t *testing.T) { }) t.Run("denied output and timeout allow", func(t *testing.T) { id := create(t, &InterceptAudit{RunID: "run", ToolUseID: "call", ExecutionStatus: "not_started"}) - if _, err := d.ResolveIntercept(id, "denied", "deny", "denied manually"); err != nil { + if _, err := d.ResolveIntercept(id, "denied", "deny", "denied manually", "human"); err != nil { t.Fatal(err) } if err := d.CompleteIntercept(id, "run", "call", "failed", "Blocked by hook", false); err != nil { @@ -101,7 +101,7 @@ func TestInterceptDetails(t *testing.T) { t.Fatal("denial presented as executed") } id = create(t, &InterceptAudit{RunID: "run2", ToolUseID: "call2", Correlation: "exact", InitialAction: "ask"}) - if _, err := d.ResolveIntercept(id, "timeout", "allow", "timeout allow"); err != nil { + if _, err := d.ResolveIntercept(id, "timeout", "allow", "timeout allow", "system"); err != nil { t.Fatal(err) } if err := d.CompleteIntercept(id, "run2", "call2", "succeeded", "ok", false); err != nil { @@ -126,6 +126,7 @@ func TestInterceptDetails(t *testing.T) { if legacy { delete(row, "audit") delete(row, "decision_source") + delete(row, "decided_by") } archived, _ := json.Marshal([]map[string]any{row}) tx, err := d.Begin() @@ -139,13 +140,14 @@ func TestInterceptDetails(t *testing.T) { if err := restoreInterceptRows(tx, archived); err != nil { t.Fatalf("legacy=%t: %v", legacy, err) } - var status, source string + var status, source, decidedBy string var auditJSON []byte - if err := tx.QueryRow(`SELECT status, decision_source, audit FROM intercept_pending WHERE id=$1`, id).Scan(&status, &source, &auditJSON); err != nil { + if err := tx.QueryRow(`SELECT status, decision_source, decided_by, audit FROM intercept_pending WHERE id=$1`, id).Scan(&status, &source, &decidedBy, &auditJSON); err != nil { t.Fatal(err) } - if status != "timeout" || source != "model" { - t.Fatalf("restored %s/%s", status, source) + // The restore itself times the request out: a system decision. + if status != "timeout" || source != "model" || decidedBy != "system" { + t.Fatalf("restored %s/%s/%s", status, source, decidedBy) } if legacy && len(auditJSON) > 0 { t.Fatal("fabricated legacy audit") diff --git a/db/schema.sql b/db/schema.sql index 5f8a9ba..7037f1c 100644 --- a/db/schema.sql +++ b/db/schema.sql @@ -1108,6 +1108,10 @@ UPDATE intercept_pending SET decision_source=CASE WHEN rule_id IS NOT NULL THEN UPDATE intercept_pending SET decision_source='model' WHERE decision_source='unknown' AND rule_id IS NULL AND (reason LIKE '[model]%' OR reason LIKE '[模型]%'); +-- Actor that settled the request: '' (pending, or decided before this column +-- existed) | human | rule | model | system. No backfill: readers fall back to +-- decision_source for legacy rows instead of rewriting history. +ALTER TABLE intercept_pending ADD COLUMN IF NOT EXISTS decided_by TEXT NOT NULL DEFAULT ''; -- ===================================================================== -- L. Finding persistence diff --git a/db/task_archives_restore.go b/db/task_archives_restore.go index 44c4a72..c2e85bf 100644 --- a/db/task_archives_restore.go +++ b/db/task_archives_restore.go @@ -682,8 +682,13 @@ func restoreInterceptRows(tx *sql.Tx, raw json.RawMessage) error { } row["decision_source"] = source } + // Archives predating decided_by lack this NOT NULL column too. + if row["decided_by"] == nil { + row["decided_by"] = "" + } if status, _ := row["status"].(string); status == "pending" { row["status"] = "timeout" + row["decided_by"] = "system" row["reason"] = "approval timed out while the task was archived" row["decided_at"] = time.Now().UTC() if audit, ok := row["audit"].(map[string]any); ok { diff --git a/intercept/intercept.go b/intercept/intercept.go index d503322..4f0cee6 100644 --- a/intercept/intercept.go +++ b/intercept/intercept.go @@ -637,7 +637,7 @@ func (i *Interceptor) HandleAsk(ctx context.Context, convID int64, dec Decision, case allowed := <-ch: return allowed case <-ctx.Done(): - _, _ = i.db.ResolveIntercept(pendingID, "denied", "deny", "work cancelled") + _, _ = i.db.ResolveIntercept(pendingID, "denied", "deny", "work cancelled", "system") _ = i.db.CompleteIntercept(pendingID, audit.RunID, audit.ToolUseID, "not_executed", "work cancelled before execution", false) return false } @@ -658,7 +658,7 @@ func (i *Interceptor) HandleAsk(ctx context.Context, convID int64, dec Decision, if allowed { action = "allow" } - resolved, err := i.db.ResolveIntercept(pendingID, "timeout", action, "approval timed out, applying timeout policy") + resolved, err := i.db.ResolveIntercept(pendingID, "timeout", action, "approval timed out, applying timeout policy", "system") if err != nil { return false } @@ -668,7 +668,7 @@ func (i *Interceptor) HandleAsk(ctx context.Context, convID int64, dec Decision, } return allowed case <-ctx.Done(): - _, _ = i.db.ResolveIntercept(pendingID, "denied", "deny", "work cancelled") + _, _ = i.db.ResolveIntercept(pendingID, "denied", "deny", "work cancelled", "system") _ = i.db.CompleteIntercept(pendingID, audit.RunID, audit.ToolUseID, "not_executed", "work cancelled before execution", false) return false } @@ -686,7 +686,7 @@ func (i *Interceptor) Decide(pendingID int64, allowed bool) error { if allowed { action, reason = "allow", "user approved execution" } - resolved, err := i.db.ResolveIntercept(pendingID, status, action, reason) + resolved, err := i.db.ResolveIntercept(pendingID, status, action, reason, "human") if err != nil { return err } diff --git a/server/intercept.go b/server/intercept.go index 3f1e00e..adc9f42 100644 --- a/server/intercept.go +++ b/server/intercept.go @@ -321,7 +321,7 @@ func (s *Server) interceptHistory(w http.ResponseWriter, r *http.Request) { } func interceptFilterParams(q url.Values) (db.InterceptApprovalFilter, error) { - filter := db.InterceptApprovalFilter{Status: q.Get("status"), DecisionSource: q.Get("decision_source")} + filter := db.InterceptApprovalFilter{Status: q.Get("status"), DecisionSource: q.Get("decision_source"), DecidedBy: q.Get("decided_by")} switch filter.Status { case "", "pending", "allowed", "denied", "timeout": default: @@ -332,6 +332,11 @@ func interceptFilterParams(q url.Values) (db.InterceptApprovalFilter, error) { default: return filter, fmt.Errorf("decision_source must be model, rule or unknown") } + switch filter.DecidedBy { + case "", "human", "rule", "model", "system": + default: + return filter, fmt.Errorf("decided_by must be human, rule, model or system") + } return filter, nil } diff --git a/server/intercept_decided_by_test.go b/server/intercept_decided_by_test.go new file mode 100644 index 0000000..6a74443 --- /dev/null +++ b/server/intercept_decided_by_test.go @@ -0,0 +1,219 @@ +package server + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/Autumn-27/artex/db" + "github.com/Autumn-27/artex/intercept" +) + +// Pure parameter validation: runs without Postgres. +func TestInterceptFilterParamsDecidedBy(t *testing.T) { + for _, actor := range []string{"", "human", "rule", "model", "system"} { + filter, err := interceptFilterParams(url.Values{"decided_by": {actor}}) + if err != nil || filter.DecidedBy != actor { + t.Fatalf("decided_by=%q: %+v err=%v", actor, filter, err) + } + } + for _, actor := range []string{"invalid", "MODEL", "unknown", "human' OR 1=1 --"} { + if _, err := interceptFilterParams(url.Values{"decided_by": {actor}}); err == nil { + t.Fatalf("decided_by=%q accepted", actor) + } + } +} + +// Drives every real writer (rule/model log, HTTP decide, timeout, cancellation) +// and reads the actor back through the HTTP lists. +func TestInterceptDecidedByHTTP(t *testing.T) { + dsn, _, err := db.DSN() + if err != nil { + t.Skip("no test database configured") + } + d, err := db.Open(dsn) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = d.Close() }) + m := &Manager{pg: d, interceptor: intercept.New(d)} + s := &Server{m: m, jwtKey: []byte("approval-decided-by-test-key")} + h := s.Handler() + token, err := signJWT(s.jwtKey) + if err != nil { + t.Fatal(err) + } + scope := "approval-http-decided-by" + t.Cleanup(func() { _, _ = d.Exec(`DELETE FROM intercept_pending WHERE task_id=$1`, scope) }) + rule, err := d.CreateInterceptRule("decided-by http fixture", "tool_name", "string", "Bash", "deny", "fixture", 1, false, false, 0, "deny") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _, _ = d.Exec(`DELETE FROM intercept_rules WHERE id=$1`, rule.ID) }) + do := func(method, path, body string) *httptest.ResponseRecorder { + req := httptest.NewRequest(method, path, strings.NewReader(body)) + req.Header.Set("Authorization", "Bearer "+token) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + return rec + } + + // Every step below adds exactly one row to the scope; added returns it. + want := map[int64]string{} + added := func(actor, status string) int64 { + t.Helper() + rows, err := d.ListTaskIntercepts(scope) + if err != nil { + t.Fatal(err) + } + for _, r := range rows { + if _, known := want[r.ID]; known { + continue + } + if r.Status != status { + t.Fatalf("new row %d has status %q, want %q", r.ID, r.Status, status) + } + want[r.ID] = actor + return r.ID + } + t.Fatalf("no new row for %s/%s", actor, status) + return 0 + } + ctx := intercept.WithTaskContext(context.Background(), scope, "test", nil) + input := []byte(`{"command":"fixture"}`) + modelAsk := intercept.Decision{Action: "ask", Message: db.ModelReasonPrefix + " confirm"} + + m.interceptor.Log(ctx, 0, intercept.Decision{Action: "deny", RuleID: rule.ID, Message: "fixture"}, "Bash", input, "denied") + ruleID := added("rule", "denied") + m.interceptor.Log(ctx, 0, intercept.Decision{Action: "allow", Message: db.ModelReasonPrefix + " fixture"}, "Bash", input, "allowed") + modelID := added("model", "allowed") + + for _, decision := range []string{"allowed", "denied"} { + id, err := d.CreateInterceptPending(0, 0, scope, "test", "Bash", input, modelAsk.Message) + if err != nil { + t.Fatal(err) + } + if r := do(http.MethodPost, fmt.Sprintf("/api/intercept/pending/%d/decide", id), `{"decision":"`+decision+`"}`); r.Code != 200 { + t.Fatalf("decide %s: %d %s", decision, r.Code, r.Body.String()) + } + if got := added("human", decision); got != id { + t.Fatalf("decided row %d, want %d", got, id) + } + } + + timed := modelAsk + timed.TimeoutEnabled, timed.TimeoutSeconds, timed.TimeoutAction = true, 1, "deny" + if m.interceptor.HandleAsk(ctx, 0, timed, "Bash", input) { + t.Fatal("timeout policy deny allowed the call") + } + added("system", "timeout") + + // Work cancelled: a system refusal recorded with status 'denied', on both + // the no-timeout and the timeout-armed wait. + cancelled, cancel := context.WithCancel(ctx) + cancel() + if m.interceptor.HandleAsk(cancelled, 0, modelAsk, "Bash", input) { + t.Fatal("cancelled ask allowed the call") + } + added("system", "denied") + timed.TimeoutSeconds = 60 + if m.interceptor.HandleAsk(cancelled, 0, timed, "Bash", input) { + t.Fatal("cancelled timed ask allowed the call") + } + added("system", "denied") + + if _, err := d.CreateInterceptPending(0, 0, scope, "test", "Bash", input, modelAsk.Message); err != nil { + t.Fatal(err) + } + added("", "pending") + + type item struct { + ID int64 `json:"id"` + Status string `json:"status"` + DecidedBy *string `json:"decided_by"` + } + list := func(path string) (items []item, total int) { + t.Helper() + rec := do(http.MethodGet, path, "") + var body struct { + Items []item `json:"items"` + Total int `json:"total"` + } + if rec.Code != 200 || json.Unmarshal(rec.Body.Bytes(), &body) != nil { + t.Fatalf("%s: %d %s", path, rec.Code, rec.Body.String()) + } + for _, it := range body.Items { + if it.DecidedBy == nil { + t.Fatalf("%s: item %d has no decided_by field", path, it.ID) + } + } + return body.Items, body.Total + } + task := "/api/intercept/task/" + scope + items, total := list(task) + if total != len(want) || len(items) != len(want) { + t.Fatalf("task list: total=%d items=%d want=%d", total, len(items), len(want)) + } + for _, it := range items { + if *it.DecidedBy != want[it.ID] { + t.Fatalf("item %d (%s) decided_by=%q, want %q", it.ID, it.Status, *it.DecidedBy, want[it.ID]) + } + } + + for _, tc := range []struct { + query string + total int + }{ + {"?decided_by=model", 1}, + {"?decided_by=rule", 1}, + {"?decided_by=human", 2}, + {"?decided_by=system", 3}, + {"?status=denied&decided_by=human", 1}, + {"?status=denied&decided_by=system", 2}, + {"?status=timeout&decided_by=model", 0}, + } { + items, total := list(task + tc.query) + if total != tc.total || len(items) != tc.total { + t.Fatalf("%s: total=%d items=%d want=%d", tc.query, total, len(items), tc.total) + } + for _, it := range items { + if !strings.Contains(tc.query, "decided_by="+*it.DecidedBy) { + t.Fatalf("%s returned item %d decided by %q", tc.query, it.ID, *it.DecidedBy) + } + } + } + if items, _ := list(task + "?decided_by=model"); len(items) != 1 || items[0].ID != modelID { + t.Fatalf("decided_by=model returned %+v, want only %d", items, modelID) + } + // The global history holds other tests' rows too: none of this test's human, + // rule or system decisions may surface under decided_by=model. + items, _ = list("/api/intercept/history?size=100&decided_by=model") + for _, it := range items { + if actor, mine := want[it.ID]; *it.DecidedBy != "model" || mine && actor != "model" { + t.Fatalf("history decided_by=model returned item %d (%s) decided by %q", it.ID, it.Status, *it.DecidedBy) + } + } + + for _, base := range []string{"/api/intercept/history", task} { + for _, query := range []string{"?decided_by=invalid", "?decided_by=MODEL", "?decided_by=unknown", "?page=1&decided_by=human%27+OR+1%3D1--"} { + if rec := do(http.MethodGet, base+query, ""); rec.Code != 400 { + t.Fatalf("invalid filter %s%s: %d %s", base, query, rec.Code, rec.Body.String()) + } + } + } + + // A row decided before the column existed falls back to decision_source. + if _, err := d.Exec(`UPDATE intercept_pending SET decided_by='' WHERE id=$1`, ruleID); err != nil { + t.Fatal(err) + } + rec := do(http.MethodGet, fmt.Sprintf("/api/intercept/history/%d", ruleID), "") + var legacy item + if rec.Code != 200 || json.Unmarshal(rec.Body.Bytes(), &legacy) != nil || legacy.DecidedBy == nil || *legacy.DecidedBy != "rule" { + t.Fatalf("legacy detail: %d %s", rec.Code, rec.Body.String()) + } +} From 2218480ebae4d5537e80b32868cf49a364dc390b Mon Sep 17 00:00:00 2001 From: BaaRai <130491490+BaaRai@users.noreply.github.com> Date: Mon, 9 Nov 2026 22:35:09 +0100 Subject: [PATCH 2/3] feat: show and filter the decided_by actor in the approval records The approval list and detail return decided_by exactly as stored, and the decided_by filter matches that same value. Rows decided before the column existed show "Legacy" and match no decided_by filter; their decision_source stays in its own column and nothing is rewritten. UI: a "Decided by" column (Human, Rule, Model, System) and a matching filter beside the decision-source filter, including "AI-approved only" for model. Types and the API query carry decided_by the same way decision_source does. --- db/intercept.go | 15 +++--- db/intercept_decided_by_test.go | 35 ++++++++++---- server/intercept_decided_by_test.go | 18 +++++-- web/src/components/approval-records.tsx | 62 +++++++++++++++++++++++-- web/src/lib/api.ts | 1 + web/src/lib/mock/data.ts | 1 + web/src/lib/mock/handler.ts | 7 ++- web/src/lib/types.ts | 4 ++ 8 files changed, 117 insertions(+), 26 deletions(-) diff --git a/db/intercept.go b/db/intercept.go index efe7354..22991ab 100644 --- a/db/intercept.go +++ b/db/intercept.go @@ -255,14 +255,12 @@ const approvalDecisionSource = `COALESCE(NULLIF(ip.decision_source,''), CASE WHEN ip.rule_id IS NOT NULL THEN 'rule' WHEN lower(ip.reason) LIKE '` + ModelReasonPrefix + `%' OR ip.reason LIKE '` + legacyModelReasonPrefix + `%' THEN 'model' ELSE 'unknown' END)` -// Rows decided before decided_by existed keep it empty; show their decision -// source instead of rewriting history. A pending row has no deciding actor yet, -// so its empty value is returned as is. -const approvalDecidedBy = `CASE WHEN ip.decided_by<>'' OR ip.status='pending' THEN ip.decided_by ELSE ` + approvalDecisionSource + ` END` - +// decided_by is returned exactly as stored, never merged with decision_source: +// rows decided before the column existed keep it empty, so the displayed value +// is always the one the decided_by filter matches. const approvalRowColumns = `ip.id, ip.rule_id, ip.conversation_id, ip.task_id, ip.agent_name, ip.tool_name, ip.tool_input, ip.status, ip.reason, ip.decided_at, ip.created_at, ` + approvalDecisionSource + `, - ` + approvalDecidedBy + `, + ip.decided_by, COALESCE(c.title,'') AS conv_title, COALESCE(c.agent_key,'') AS conv_agent_key, COALESCE(ir.name,'') AS rule_name` @@ -374,8 +372,9 @@ func (d *DB) listInterceptsPage(taskID string, page, size int, filter InterceptA add("ip.task_id", taskID) add("ip.status", filter.Status) add(approvalDecisionSource, filter.DecisionSource) - // Stored actor only: the legacy display fallback would let decided_by=model - // match human decisions and timeouts recorded before the column existed. + // Stored actor only, the same value the rows display: deriving it from + // decision_source would let decided_by=model match human decisions and + // timeouts recorded before the column existed. add("ip.decided_by", filter.DecidedBy) where := "" if len(conditions) > 0 { diff --git a/db/intercept_decided_by_test.go b/db/intercept_decided_by_test.go index 664142e..1153207 100644 --- a/db/intercept_decided_by_test.go +++ b/db/intercept_decided_by_test.go @@ -129,22 +129,41 @@ func TestInterceptDecidedBy(t *testing.T) { } } - // Rows decided before the column existed: displayed through decision_source, - // never rewritten, and not matched by the stored-actor filter. + // Rows decided before the column existed: decided_by stays empty everywhere it + // is read, decision_source keeps its own column, nothing is rewritten, and no + // decided_by filter matches them. if _, err := d.Exec(`UPDATE intercept_pending SET decided_by='' WHERE id IN ($1,$2,$3)`, ruleID, modelID, timeoutID); err != nil { t.Fatal(err) } - for id, source := range map[int64]string{ruleID: "rule", modelID: "model", timeoutID: "model"} { + legacy := map[int64]string{ruleID: "rule", modelID: "model", timeoutID: "model"} + for id, source := range legacy { detail, err := d.GetInterceptDetail(id) - if err != nil || detail.DecidedBy != source || detail.DecisionSource != source { - t.Fatalf("legacy fallback for %d: %+v err=%v, want %q", id, detail, err, source) + if err != nil || detail == nil || detail.DecidedBy != "" || detail.DecisionSource != source { + t.Fatalf("legacy detail for %d: %+v err=%v, want decided_by \"\" and decision_source %q", id, detail, err, source) } stored, err := d.GetInterceptPending(id) - if err != nil || stored.DecidedBy != "" { + if err != nil || stored == nil || stored.DecidedBy != "" { t.Fatalf("legacy row %d rewritten: %+v err=%v", id, stored, err) } } - if got := filtered(InterceptApprovalFilter{DecidedBy: "model"}); len(got) != 0 { - t.Fatalf("legacy rows matched decided_by=model: %v", got) + rows, err = d.ListTaskIntercepts(scope) + if err != nil { + t.Fatal(err) + } + for _, r := range rows { + if source, ok := legacy[r.ID]; ok && (r.DecidedBy != "" || r.DecisionSource != source) { + t.Fatalf("legacy list row %d: decided_by=%q decision_source=%q, want \"\" and %q", r.ID, r.DecidedBy, r.DecisionSource, source) + } + } + for _, actor := range []string{"human", "rule", "model", "system"} { + for _, id := range filtered(InterceptApprovalFilter{DecidedBy: actor}) { + if _, ok := legacy[id]; ok { + t.Fatalf("legacy row %d matched decided_by=%s", id, actor) + } + } + } + // decision_source still finds them through its own filter. + if got := filtered(InterceptApprovalFilter{DecisionSource: "rule"}); !slices.Contains(got, ruleID) { + t.Fatalf("legacy rule row missing from decision_source=rule: %v", got) } } diff --git a/server/intercept_decided_by_test.go b/server/intercept_decided_by_test.go index 6a74443..114298f 100644 --- a/server/intercept_decided_by_test.go +++ b/server/intercept_decided_by_test.go @@ -207,13 +207,25 @@ func TestInterceptDecidedByHTTP(t *testing.T) { } } - // A row decided before the column existed falls back to decision_source. + // A row decided before the column existed keeps an empty decided_by; its + // decision_source stays in its own field and no decided_by filter returns it. if _, err := d.Exec(`UPDATE intercept_pending SET decided_by='' WHERE id=$1`, ruleID); err != nil { t.Fatal(err) } rec := do(http.MethodGet, fmt.Sprintf("/api/intercept/history/%d", ruleID), "") - var legacy item - if rec.Code != 200 || json.Unmarshal(rec.Body.Bytes(), &legacy) != nil || legacy.DecidedBy == nil || *legacy.DecidedBy != "rule" { + var legacy struct { + DecidedBy *string `json:"decided_by"` + DecisionSource string `json:"decision_source"` + } + if rec.Code != 200 || json.Unmarshal(rec.Body.Bytes(), &legacy) != nil || + legacy.DecidedBy == nil || *legacy.DecidedBy != "" || legacy.DecisionSource != "rule" { t.Fatalf("legacy detail: %d %s", rec.Code, rec.Body.String()) } + if items, total := list(task + "?decided_by=rule"); total != 0 || len(items) != 0 { + t.Fatalf("legacy row matched decided_by=rule: %+v", items) + } + items, _ = list(task + "?decision_source=rule") + if len(items) != 1 || items[0].ID != ruleID || *items[0].DecidedBy != "" { + t.Fatalf("legacy row under decision_source=rule: %+v", items) + } } diff --git a/web/src/components/approval-records.tsx b/web/src/components/approval-records.tsx index 12fafad..9a4b3da 100644 --- a/web/src/components/approval-records.tsx +++ b/web/src/components/approval-records.tsx @@ -108,6 +108,23 @@ function StatusBadge({ status }: { status: string }) { return {labels[status] ?? status}; } +const decidedByLabels: Record = { human: "Human", rule: "Rule", model: "Model", system: "System" }; + +// decided_by is shown exactly as stored, so a row always sits under the +// "Decided by" filter option matching its badge. A decided row without an actor +// predates the column: it reads "Legacy" and matches no option. +function DecidedByBadge({ row }: { row: InterceptApprovalRow }) { + if (row.status === "pending") return —; + if (!row.decided_by) { + return ( + + Legacy + + ); + } + return {decidedByLabels[row.decided_by] ?? row.decided_by}; +} + function MatchCell({ row, showReason = true }: { row: InterceptApprovalRow; showReason?: boolean }) { const reason = row.reason?.replace(/^\[model\]\s*/, ""); return ( @@ -383,6 +400,10 @@ export function ApprovalDetail({
{fmtTime(row.created_at)}
Decided at
{fmtTime(current.decided_at)}
+
Decided by
+
+ +
{audit?.rule_name ? ( <>
Rule snapshot
@@ -527,10 +548,10 @@ function subscribeColumns(onChange: () => void) { } function visibleColumnCount() { const matches = approvalBreakpoints.map((query) => window.matchMedia(query).matches); - return 3 + Number(matches[0]) + Number(matches[1]) + 2 * Number(matches[2]) + 2 * Number(matches[3]); + return 3 + Number(matches[0]) + Number(matches[1]) + 3 * Number(matches[2]) + 2 * Number(matches[3]); } function serverColumnCount() { - return 9; + return 10; } function ApprovalTable({ @@ -570,6 +591,7 @@ function ApprovalTable({ Matched rule Arguments Status + Decided by Requested at Decided at @@ -623,6 +645,9 @@ function ApprovalTable({ + + + {fmtTime(row.created_at)} @@ -663,7 +688,7 @@ export function ApprovalRecords({ taskId }: { taskId?: string }) { const request = React.useRef(0); const decisionLock = React.useRef(false); const filterID = React.useId(); - const filtered = Boolean(filter.status || filter.decision_source); + const filtered = Boolean(filter.status || filter.decision_source || filter.decided_by); // A task can stay mounted while the user switches between task details. // Reset the cursor so the new scope always starts at its newest records. @@ -749,7 +774,9 @@ export function ApprovalRecords({ taskId }: { taskId?: string }) { // Optimistically drop the decided item from the independent pending queue // for instant feedback; the re-fetch below reconciles with server truth. setRows((prev) => - prev.map((row) => (row.id === id ? { ...row, status: decision, decided_at: new Date().toISOString() } : row)), + prev.map((row) => + row.id === id ? { ...row, status: decision, decided_by: "human", decided_at: new Date().toISOString() } : row, + ), ); setPendingRows((prev) => prev.filter((row) => row.id !== id)); setRevision((v) => v + 1); @@ -840,6 +867,31 @@ export function ApprovalRecords({ taskId }: { taskId?: string }) { + + Decided by + + {filtered ? (