diff --git a/agents/anyplot/plugins/tool_safety.py b/agents/anyplot/plugins/tool_safety.py index 86ede927a6..c365b73e01 100644 --- a/agents/anyplot/plugins/tool_safety.py +++ b/agents/anyplot/plugins/tool_safety.py @@ -26,7 +26,7 @@ from pydantic import BaseModel, ConfigDict, Field, ValidationError from ..policy import DATA_PREAMBLE, fence -from ..schemas import MAX_COLUMNS, Binding, PipelineArgs +from ..schemas import MAX_COLUMNS, Binding, PipelineArgs, PlotResult from .ledger import argument_hash, attribution, ledger_for @@ -79,9 +79,9 @@ class _BindingArgs(BaseModel): "get_spec_brief": frozenset({"status", "code", "brief"}), "get_current_code": frozenset({"status", "code", "version"}), "set_bindings": frozenset({"status", "reason", "code", "complete", "missing_roles", "errors"}), - PIPELINE_TOOL: frozenset( - {"status", "reason", "attempts", "artifacts", "changes", "residual_defects", "code", "result", "error"} - ), + # Every PlotResult field, so a field added to the schema (as `version` was) can never turn a + # shipped plot into `invalid_result` for the root, plus the tool-error shape. + PIPELINE_TOOL: frozenset(PlotResult.model_fields) | frozenset({"code", "result", "error"}), } diff --git a/changelog.d/agents-plot-result-version.md b/changelog.d/agents-plot-result-version.md new file mode 100644 index 0000000000..0a98226b08 --- /dev/null +++ b/changelog.d/agents-plot-result-version.md @@ -0,0 +1,9 @@ +### Fixed + +- **The root agent no longer reports a shipped plot as failed.** The chat UI + added `version` to the plot result, but the tool-safety allowlist for the + `plot_pipeline` result did not know the field, so every shipped plot reached + the root as `invalid_result` and its closing reply told the user that the + run had failed while the plot was already on screen. The allowlist is now + derived from the `PlotResult` schema, and a test holds the two together. + (#12123) diff --git a/tests/unit/agents/runtime/test_plugins.py b/tests/unit/agents/runtime/test_plugins.py index 26002a3ea9..d4cae16c3f 100644 --- a/tests/unit/agents/runtime/test_plugins.py +++ b/tests/unit/agents/runtime/test_plugins.py @@ -13,9 +13,9 @@ from agents.anyplot.plugins.budget import BudgetPlugin from agents.anyplot.plugins.ledger import CURRENT_LEDGER, RequestLedger, ledger_for from agents.anyplot.plugins.scope_guard import WITHHELD, ScopeGuardPlugin, judge_input -from agents.anyplot.plugins.tool_safety import ToolSafetyPlugin, has_url_or_path, result_limit, result_size +from agents.anyplot.plugins.tool_safety import RESULT_KEYS, ToolSafetyPlugin, has_url_or_path, result_limit, result_size from agents.anyplot.policy import DATA_PREAMBLE, fence -from agents.anyplot.schemas import ColumnProfile, DatasetProfile +from agents.anyplot.schemas import ColumnProfile, DatasetProfile, PlotResult from agents.anyplot.services import Services from agents.anyplot.settings import get_settings @@ -308,6 +308,20 @@ async def test_pipeline_notes_are_fenced_for_the_root(self, ledger: RequestLedge tool=FakeTool("plot_pipeline"), tool_args={}, tool_context=FakeContext(), result={"status": "failed"} ) == {"status": "failed"} + async def test_a_shipped_plot_result_reaches_the_root_whole(self, ledger: RequestLedger) -> None: + """Every PlotResult field is allowed: `version` (added with the chat UI) used to make the root read + `invalid_result` for every shipped plot and tell the user the run had failed.""" + shipped = PlotResult( + status="ok", attempts=1, artifacts=["plot-light.png"], changes=["Plotted df"], version=1 + ).model_dump(mode="json") + + result = await ToolSafetyPlugin().after_tool_callback( + tool=FakeTool("plot_pipeline"), tool_args={}, tool_context=FakeContext(), result=shipped + ) + + assert result is not None and result["status"] == "ok" and result["version"] == 1 + assert set(PlotResult.model_fields) <= RESULT_KEYS["plot_pipeline"] + async def test_errors_become_fixed_codes(self, ledger: RequestLedger) -> None: result = await ToolSafetyPlugin().on_tool_error_callback( tool=FakeTool("get_spec_brief"),