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 ? (