feat(agents): emit a machine-readable approval decision on gated PXI tools (#15029)

* feat(agents): stamp a machine-readable approval decision on gated tools

Approval-gated PXI tools each hand-rolled their accept/reject output, so the
payload vocabulary had drifted: most emit `status: "accepted" | "rejected"`,
while `save_prompt` demotes the verdict to `approvalStatus` and sets
`status: "saved"`, and `load_dataset` / `patch_experiment` use `"loaded"` /
`"applied"`. Nothing in span metadata marked a call as approval-gated at all —
rejections complete with status OK like any other tool call.

That left trace consumers no way to identify an approval decision except by
matching a hand-maintained list of tool names, which had already gone stale
(every dataset-write tool is gated and was missing from it).

Add a reserved, nested `approval: { decision, source }` marker, stamped by a
shared `approvalOutcome()` helper in every accept/reject path. It is:

- nested, so tools that spread their own action result into the output
  (`save_prompt`, `write_prompt_tools`) cannot clobber it;
- additive, leaving existing `status`/`acceptedBy` keys untouched, so the
  tool-card UI parses exactly what it parses today;
- meaningful by its absence — cancellations and still-pending proposals stay
  unmarked, so consumers can skip them rather than guess.

The server promotes the marker onto the emitted TOOL span as the
`pxi.approval.decision` / `pxi.approval.source` attributes, in the single place
every client-tool result becomes output attributes. This makes approval
decisions filterable server-side via `get_spans(attributes=...)` instead of
requiring consumers to fetch every TOOL span and scan `output.value`, and
centralizes the defensive payload parsing once rather than per consumer.

A source-level drift guard asserts every `pending*.ts` module that emits tool
output stamps both branches, so a newly gated tool cannot silently go unmarked.

* fix(agents): promote the approval marker where client tool spans are made

The first pass set the span attributes in `ToolSpanMixin.set_output`, which
approval-gated tools never reach: they are external tools, so they never execute
server-side (`execute_tool_call` raises before the hooks run, and resumption
assigns the browser's result directly). Their TOOL spans are synthesized from
the request body in `_synthesize_client_tool_spans`, which is the only seam
where the marker can reach the span — verified against real spans, whose
`session.id`/`tool.id` attributes only that path emits.

Move the extractor to `phoenix.server.agents.approval`, call it where the
output attributes are actually built, and revert the mixin. Add a router test
asserting the attributes land on a synthesized gated span and stay absent on a
non-gated one — the test that would have caught this.

Also from review:

- Guard the extractor's frozenset membership tests with isinstance checks. The
  marker is browser-supplied JSON, so an unhashable `decision` raised TypeError
  out of span emission, contradicting the "never fails a tool call" contract.
- Stamp `source: "auto"` as a literal in the evaluator auto-accept branch rather
  than echoing `result.acceptedBy`, which a future unwrapped submit host could
  set to "user".
- Find approval payloads by content rather than by `pending*.ts` filename. The
  old guard missed `agent/tools/approval.ts` — a payload site this very change
  had to patch — and now pins the full emitter list.
- Document the pre-existing `submit_*_evaluator_draft` gap: those decisions
  happen in a dialog that never writes tool output, so they stay invisible.

* docs(agents): link the submit_* approval gap to its tracking issue

The gap is documented where a reader of the marker would hit it; point it at
issue #15033 so the follow-up is findable from the code rather than only from
the tracker.

* refactor(agents): simplify approval marker contract

* refactor(agents): colocate approval marker extraction

* feat(evals): add suggestion_accepted online eval for approval decisions (#15030)

* feat(evals): add suggestion_accepted online eval targeting approval-gated TOOL spans

* test(evals): cover suggestion_accepted semantics and mixed root/TOOL runner targeting

* test(evals): add sanitized real-shape suggestion approval trace fixtures

* docs(evals): document suggestion_accepted target discovery and approval semantics

* fix(evals): enforce span selector parent matching

* refactor(evals): discover suggestion outcomes by approval attributes

`suggestion_accepted` identified approval decisions by matching `tool.name`
against a hand-maintained `APPROVAL_GATED_TOOLS` list, then parsing
`output.value` through a defensive multi-shape decoder because each tool spells
its outcome differently (`accepted`, `saved`, `loaded`, `applied`, `removed`,
and `save_prompt`'s `approvalStatus`).

That list was already stale before shipping: every dataset-write tool is
approval-gated via `stageDatasetWrite` and none were listed, so those decisions
went unmeasured. Keeping it correct meant maintaining a cross-language contract
with the frontend by hand, with nothing enforcing it.

PXI tools now stamp a uniform approval marker that the server promotes onto the
span, so:

- Delete the allowlist and the name-based applicability gate. Discovery selects
  on `pxi.approval.source = "user"` — one server-side query that yields exactly
  the annotated set, since rejections are always a user action and automatic
  accepts are never annotated.
- Delete the `output.value` decoder. Classification reads
  `pxi.approval.decision` only, so a tool's own status vocabulary is irrelevant
  and a look-alike payload on a non-gated tool can no longer be misread.
- Extend `SpanSelector` with hashable attribute filters, letting an evaluator
  target spans by what they record rather than by which tool produced them.
  A selector still requires a name or attribute filter so discovery stays
  bounded.

A newly approval-gated tool is now measured the day it ships. Fixtures keep each
tool's original status vocabulary alongside the marker, which is what makes them
worth keeping: they prove classification no longer depends on it.

* fix(evals): harden approval-attribute discovery after review

- Import the attribute names from `phoenix.server.agents.approval` rather than
  re-declaring the literals. The eval reads what the server writes, and a
  drifted name would not raise: discovery would return nothing, forever, and
  look like a quiet window. A test pins the two modules together.
- Isolate per-selector discovery failures. Attribute filtering requires a newer
  Phoenix server than name filtering, so an old server or a transient error on
  one selector previously aborted the whole scheduled job, taking
  `tool_count_per_turn` and `user_friction` with it. The candidate-limit guard
  stays fatal via its own `CandidateLimitError`: unlike a failed query, a
  truncated candidate set makes the run's results quietly incomplete.
- Reject non-string attribute values in `SpanSelector`, which would otherwise
  serialize into the query and then never match locally.
- Apply the attribute filter in the runner's fake `get_spans`, so a test would
  notice if server-side filtering silently stopped working.
- Docs: soften the drift-guard claim (it recognizes payloads by the known
  accept/reject vocabulary, so a hand-rolled tool inventing new wording is not
  covered — tools on `bindPendingApproval` are covered by construction), and
  state that pre-marker spans are invisible to discovery and cannot be
  backfilled.

* docs(evals): link the unmeasured submit_* tools to their tracking issue

Note in both eval docs that accept/reject rates exclude the two submit tools
until #15033 lands, so the gap is visible to anyone reading the numbers.

* refactor(evals): simplify suggestion outcome eval

* style(evals): format rebased online evals

* fix(evals): type suggestion fixture payload

* fix(agents): use client tool metadata wire keys

Author: ehutt

app/src/agent/shared/pendingApproval/__tests__/approvalOutcome.test.ts

@@ -0,0 +1,81 @@
+import { readdirSync, readFileSync } from "node:fs";
+import { join } from "node:path";
+
+import { approvalOutcome } from "../approvalOutcome";
+
+describe("approvalOutcome", () => {
+  it("nests the decision and source under a reserved approval key", () => {
+    expect(approvalOutcome({ decision: "accepted", source: "user" })).toEqual({
+      approval: { decision: "accepted", source: "user" },
+    });
+    expect(approvalOutcome({ decision: "rejected", source: "user" })).toEqual({
+      approval: { decision: "rejected", source: "user" },
+    });
+  });
+
+  it("survives being spread after a payload that carries its own approval key", () => {
+    const toolPayload = { status: "saved", approval: "not-the-marker" };
+    const output = {
+      ...toolPayload,
+      ...approvalOutcome({ decision: "accepted", source: "auto" }),
+    };
+    expect(output.approval).toEqual({ decision: "accepted", source: "auto" });
+  });
+});
+
+describe("approval marker coverage", () => {
+  const agentDir = join(__dirname, "..", "..", "..");
+
+  function sourceFiles(dir: string): string[] {
+    return readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
+      const path = join(dir, entry.name);
+      if (entry.isDirectory()) {
+        return entry.name === "__tests__" ? [] : sourceFiles(path);
+      }
+      return entry.name.endsWith(".ts") ? [path] : [];
+    });
+  }
+
+  const ACCEPT_EVIDENCE = /acceptedBy:|approvalStatus:|status: "accepted"/;
+  const REJECT_EVIDENCE = /status: "rejected"/;
+  const EMITS_TOOL_OUTPUT = /addToolOutput\(|AgentClientActionResult/;
+
+  const emitters = sourceFiles(agentDir)
+    .map((path) => ({ path, source: readFileSync(path, "utf-8") }))
+    .filter(
+      ({ source }) =>
+        EMITS_TOOL_OUTPUT.test(source) &&
+        (ACCEPT_EVIDENCE.test(source) || REJECT_EVIDENCE.test(source))
+    );
+
+  it("finds every module that emits an approval payload", () => {
+    expect(emitters.map((e) => e.path.slice(agentDir.length + 1)).sort())
+      .toMatchInlineSnapshot(`
+        [
+          "shared/pendingApproval/bindPendingApproval.ts",
+          "tools/approval.ts",
+          "tools/batchSpanAnnotate/pendingBatchSpanAnnotate.ts",
+          "tools/codeEvaluatorDraft/pendingCodeEvaluatorEdit.ts",
+          "tools/llmEvaluatorDraft/pendingLlmEvaluatorEdit.ts",
+          "tools/patchExperiment/pendingPatchExperiment.ts",
+          "tools/playgroundLoadDataset/pendingLoadDataset.ts",
+          "tools/playgroundPrompt/pendingPromptEdit.ts",
+          "tools/playgroundPrompt/pendingPromptInstanceRemoval.ts",
+          "tools/playgroundPromptTools/pendingPromptToolWrite.ts",
+          "tools/playgroundSavePrompt/pendingSavePrompt.ts",
+        ]
+      `);
+  });
+
+  it.each(emitters.map((e) => [e.path, e.source] as const))(
+    "%s stamps every approval branch it emits",
+    (_path, source) => {
+      if (ACCEPT_EVIDENCE.test(source)) {
+        expect(source).toContain('decision: "accepted"');
+      }
+      if (REJECT_EVIDENCE.test(source)) {
+        expect(source).toContain('decision: "rejected"');
+      }
+    }
+  );
+});

app/src/agent/shared/pendingApproval/__tests__/pendingApproval.test.ts

@@ -42,15 +42,23 @@ describe("bindPendingApproval", () => {
         state: "output-available",
         tool: "some_write_tool",
         toolCallId: "call-1",
-        output: { status: "accepted", acceptedBy: "user", message: "done" },
+        output: {
+          status: "accepted",
+          acceptedBy: "user",
+          message: "done",
+          approval: { decision: "accepted", source: "user" },
+        },
       },
     ]);
   });
 
   it("records the approval source for an auto (bypass) accept", async () => {
     const { pending, outputs } = setup({ ok: true, output: "done" });
     await pending.accept?.({ approvalSource: "auto" });
-    expect(outputs[0].output).toMatchObject({ acceptedBy: "auto" });
+    expect(outputs[0].output).toMatchObject({
+      acceptedBy: "auto",
+      approval: { decision: "accepted", source: "auto" },
+    });
   });
 
   it("emits an error output without applying again when apply fails", async () => {
@@ -79,7 +87,11 @@ describe("bindPendingApproval", () => {
         state: "output-available",
         tool: "some_write_tool",
         toolCallId: "call-1",
-        output: { status: "rejected", message: "nothing written" },
+        output: {
+          status: "rejected",
+          message: "nothing written",
+          approval: { decision: "rejected", source: "user" },
+        },
       },
     ]);
   });

app/src/agent/shared/pendingApproval/approvalOutcome.ts

@@ -0,0 +1,21 @@
+import type { ApprovalSource } from "./types";
+
+export type ApprovalDecision = "accepted" | "rejected";
+
+export type ApprovalOutcome = {
+  approval: {
+    decision: ApprovalDecision;
+    source: ApprovalSource;
+  };
+};
+
+/** Build the approval marker promoted to `pxi.approval.*` span attributes. */
+export function approvalOutcome({
+  decision,
+  source,
+}: {
+  decision: ApprovalDecision;
+  source: ApprovalSource;
+}): ApprovalOutcome {
+  return { approval: { decision, source } };
+}

app/src/agent/shared/pendingApproval/bindPendingApproval.ts

@@ -1,3 +1,4 @@
+import { approvalOutcome } from "./approvalOutcome";
 import type { BindPendingApprovalOptions, PendingApproval } from "./types";
 
 /**
@@ -40,6 +41,7 @@ export function bindPendingApproval<TPreview>({
           status: "accepted",
           acceptedBy: approvalSource,
           message: result.output,
+          ...approvalOutcome({ decision: "accepted", source: approvalSource }),
         },
       });
     },
@@ -49,7 +51,11 @@ export function bindPendingApproval<TPreview>({
         state: "output-available",
         tool: toolName,
         toolCallId,
-        output: { status: "rejected", message: rejectedMessage },
+        output: {
+          status: "rejected",
+          message: rejectedMessage,
+          ...approvalOutcome({ decision: "rejected", source: "user" }),
+        },
       });
     },
   };

app/src/agent/shared/pendingApproval/index.ts

@@ -1,4 +1,6 @@
 export { bindPendingApproval } from "./bindPendingApproval";
+export { approvalOutcome } from "./approvalOutcome";
+export type { ApprovalDecision, ApprovalOutcome } from "./approvalOutcome";
 export type {
   ApprovalApplyResult,
   ApprovalSource,

app/src/agent/shared/pendingApproval/types.ts

@@ -1,9 +1,7 @@
 import type { Chat } from "@ai-sdk/react";
 import type { UIMessage } from "ai";
 
-import type { ApprovalSource } from "@phoenix/agent/tools/approval";
-
-export type { ApprovalSource };
+export type ApprovalSource = "user" | "auto";
 
 export type ApprovalToolOutputSender = Chat<UIMessage>["addToolOutput"];
 

app/src/agent/tools/approval.ts

@@ -1,6 +1,11 @@
+import {
+  approvalOutcome,
+  type ApprovalOutcome,
+  type ApprovalSource,
+} from "@phoenix/agent/shared/pendingApproval";
 import type { AgentClientActionResult } from "@phoenix/store/agentStore";
 
-export type ApprovalSource = "user" | "auto";
+export type { ApprovalSource };
 
 export type EvaluatorSubmitResult =
   | {
@@ -41,12 +46,12 @@ type EvaluatorSubmitHost = {
 };
 
 export type EvaluatorSubmitToolOutput =
-  | {
+  | ({
       status: "saved";
       persisted: true;
       acceptedBy: ApprovalSource;
       evaluator: { id: string; name: string };
-    }
+    } & ApprovalOutcome)
   | {
       status: "awaiting_user";
       persisted: false;
@@ -98,6 +103,7 @@ export function createEvaluatorSubmitClientAction<
       persisted: true,
       acceptedBy: result.acceptedBy,
       evaluator: result.evaluator,
+      ...approvalOutcome({ decision: "accepted", source: "auto" }),
     };
     return { ok: true, output: JSON.stringify(output) };
   };

app/src/agent/tools/batchSpanAnnotate/pendingBatchSpanAnnotate.ts

@@ -1,3 +1,5 @@
+import { approvalOutcome } from "@phoenix/agent/shared/pendingApproval";
+
 import { BATCH_SPAN_ANNOTATE_TOOL_NAME } from "./constants";
 import type {
   AnnotateSpanInput,
@@ -61,6 +63,7 @@ export function bindPendingBatchSpanAnnotateActions({
             approvalSource === "auto"
               ? `${count} span ${noun} auto-approved.`
               : `${count} span ${noun} applied.`,
+          ...approvalOutcome({ decision: "accepted", source: approvalSource }),
         },
       });
     },
@@ -75,6 +78,7 @@ export function bindPendingBatchSpanAnnotateActions({
           count,
           annotations: annotations.map(toAnnotationOutput),
           message: `User rejected the proposed span ${noun}.`,
+          ...approvalOutcome({ decision: "rejected", source: "user" }),
         },
       });
     },

app/src/agent/tools/codeEvaluatorDraft/__tests__/submitCodeEvaluatorDraft.test.ts

@@ -157,6 +157,7 @@ describe("submit_code_evaluator_draft agent tool", () => {
       persisted: true,
       acceptedBy: "auto",
       evaluator: { id: "ev-1", name: "hallucination" },
+      approval: { decision: "accepted", source: "auto" },
     });
   });
 

app/src/agent/tools/codeEvaluatorDraft/pendingCodeEvaluatorEdit.ts

@@ -1,3 +1,5 @@
+import { approvalOutcome } from "@phoenix/agent/shared/pendingApproval";
+
 import {
   EDIT_CODE_EVALUATOR_DRAFT_NAVIGATION_CANCEL_ERROR,
   EDIT_CODE_EVALUATOR_DRAFT_TOOL_NAME,
@@ -38,6 +40,7 @@ export function bindPendingCodeEvaluatorEditActions({
             approvalSource === "auto"
               ? "Code-evaluator draft edit auto-approved."
               : "Code-evaluator draft edit applied.",
+          ...approvalOutcome({ decision: "accepted", source: approvalSource }),
         },
       });
     },
@@ -50,6 +53,7 @@ export function bindPendingCodeEvaluatorEditActions({
         output: {
           status: "rejected",
           message: "User rejected the proposed code-evaluator draft edit.",
+          ...approvalOutcome({ decision: "rejected", source: "user" }),
         },
       });
     },