diff --git a/aperag/domains/agent_runtime/tools/consent.py b/aperag/domains/agent_runtime/tools/consent.py index b49116a49..21db93645 100644 --- a/aperag/domains/agent_runtime/tools/consent.py +++ b/aperag/domains/agent_runtime/tools/consent.py @@ -31,8 +31,8 @@ (``state="expired"``). 4. On approval, the runtime fetches raw args via :meth:`ConsentService.consume_raw_args` and dispatches the tool. - On denial / expiry, the runtime emits - ``tool-output-available {errorText}`` to surface the rejection. + On denial / expiry, the runtime emits a ``tool-output-error`` + wire part (per AI SDK v5 strict spec) to surface the rejection. Design points: diff --git a/aperag/domains/agent_runtime/tools/elicitation.py b/aperag/domains/agent_runtime/tools/elicitation.py index cd692e1b1..5e37c1d82 100644 --- a/aperag/domains/agent_runtime/tools/elicitation.py +++ b/aperag/domains/agent_runtime/tools/elicitation.py @@ -41,9 +41,9 @@ State transitions: ``pending -> submitted | cancelled``. There is no "expired" state for elicitation in D9 §5.1 -- the runtime decides whether to time out the awaiting tool execution and surface a -``tool-output-available {errorText}`` instead. We mirror that here -by exposing :meth:`ElicitationService.cancel` for the runtime to -call on abort / timeout. +``tool-output-error`` wire part (per AI SDK v5 strict spec) instead. +We mirror that here by exposing :meth:`ElicitationService.cancel` for +the runtime to call on abort / timeout. """ from __future__ import annotations diff --git a/aperag/domains/agent_runtime/wire/parts.py b/aperag/domains/agent_runtime/wire/parts.py index 11959f324..3681bd4eb 100644 --- a/aperag/domains/agent_runtime/wire/parts.py +++ b/aperag/domains/agent_runtime/wire/parts.py @@ -28,7 +28,8 @@ / ``abort`` / ``error`` * text: ``text-start`` / ``text-delta`` / ``text-end`` * tool lifecycle: ``tool-input-start`` / ``tool-input-delta`` / - ``tool-input-available`` / ``tool-output-available`` + ``tool-input-available`` / ``tool-output-available`` / + ``tool-output-error`` * sources: ``source-url`` / ``source-document`` * ApeRAG custom: ``data-citation`` / ``data-activity`` * placeholders for #75 chenyexuan: ``data-tool-consent`` / @@ -178,15 +179,47 @@ class ToolInputAvailablePart(BaseModel): class ToolOutputAvailablePart(BaseModel): - """Tool finished. ``error_text`` set ⇒ tool failure (output may - still carry a partial error payload depending on the tool).""" + """Tool finished successfully — carries the tool's ``output`` payload. + + Per AI SDK v5 strict spec, success and failure are split onto two + distinct wire events: ``tool-output-available`` (this class) for the + success path and :class:`ToolOutputErrorPart` for the failure path. + A failed tool call must NOT be emitted as ``tool-output-available`` + with a populated ``errorText``; the FE reducer collapses both events + into the consolidated at-rest :class:`ToolPart` state machine + (``state ∈ output-available | output-error``). + + ``extra="forbid"`` actively rejects the legacy + ``ToolOutputAvailablePart(errorText=...)`` shape so that residual + callers from before the D8.0c+ #89 split surface as a clean + ``ValidationError`` instead of silently masquerading as success. + """ type: Literal["tool-output-available"] = "tool-output-available" tool_call_id: str = Field(alias="toolCallId") output: Any = None - error_text: Optional[str] = Field(default=None, alias="errorText") - model_config = ConfigDict(populate_by_name=True) + model_config = ConfigDict(populate_by_name=True, extra="forbid") + + +class ToolOutputErrorPart(BaseModel): + """Tool finished with a failure — carries the ``error_text``. + + Per AI SDK v5 strict spec the error path is a separate wire event + type; ``error_text`` is required (the FE always has something to + render) and there is no ``output`` field — partial failure outputs + are flattened into the textual error_text by the runtime emitter. + + ``extra="forbid"`` mirrors the success-path discipline so an + accidental ``output=...`` kwarg here surfaces as a + ``ValidationError`` rather than silently flowing through. + """ + + type: Literal["tool-output-error"] = "tool-output-error" + tool_call_id: str = Field(alias="toolCallId") + error_text: str = Field(alias="errorText") + + model_config = ConfigDict(populate_by_name=True, extra="forbid") # --------------------------------------------------------------------------- @@ -359,6 +392,7 @@ class DataElicitationPart(BaseModel): ToolInputDeltaPart, ToolInputAvailablePart, ToolOutputAvailablePart, + ToolOutputErrorPart, SourceUrlPart, SourceDocumentPart, DataCitationPart, @@ -428,6 +462,7 @@ def parse_part(payload: dict[str, Any]) -> StreamPart: "ToolInputDeltaPart", "ToolInputStartPart", "ToolOutputAvailablePart", + "ToolOutputErrorPart", "dump_part", "parse_part", ] diff --git a/aperag/domains/agent_runtime/wire/translator.py b/aperag/domains/agent_runtime/wire/translator.py index eb6d8a7c6..7dab2eef8 100644 --- a/aperag/domains/agent_runtime/wire/translator.py +++ b/aperag/domains/agent_runtime/wire/translator.py @@ -77,6 +77,7 @@ ToolInputAvailablePart, ToolInputStartPart, ToolOutputAvailablePart, + ToolOutputErrorPart, ) SafeToolNameResolver = Callable[[str], tuple[str, dict[str, Any]]] @@ -300,23 +301,29 @@ def _translate_tool_started(envelope: AgentTimelineEventEnvelope, state: Transla def _translate_tool_finished(envelope: AgentTimelineEventEnvelope) -> list[StreamPart]: - output = _extract_tool_output(envelope) - error_text: Optional[str] = None if _is_failure_status(envelope.status): - # Best-effort error text — fall back to the envelope label so - # the FE always has something to render even when the tool - # didn't surface an explicit error message. + # Per AI SDK v5 strict spec the failure path emits a separate + # ``tool-output-error`` event, not ``tool-output-available`` with + # an embedded ``errorText``. Best-effort error text — fall back + # to the envelope label so the FE always has something to + # render even when the tool didn't surface an explicit error + # message. data = envelope.data or {} error_text = ( str(data.get("error")) if data.get("error") else f"tool {envelope.label or 'unknown'} failed with status={envelope.status}" ) + return [ + ToolOutputErrorPart( + tool_call_id=envelope.event_id, + error_text=error_text, + ) + ] return [ ToolOutputAvailablePart( tool_call_id=envelope.event_id, - output=output, - error_text=error_text, + output=_extract_tool_output(envelope), ) ] diff --git a/docs/modularization/agent-message-protocol-design.md b/docs/modularization/agent-message-protocol-design.md index ab3641949..94583b0ef 100644 --- a/docs/modularization/agent-message-protocol-design.md +++ b/docs/modularization/agent-message-protocol-design.md @@ -37,7 +37,7 @@ Stream parts (AI SDK v5): - Lifecycle: `start` / `start-step` / `finish-step` / `finish` / `abort` / `error` - Text: `text-start {id}` → `text-delta {id, delta}` → `text-end {id}` - Reasoning: `reasoning-start/-delta/-end` -- Tools: `tool-input-start {toolCallId, toolName}` → `tool-input-delta` → `tool-input-available` → `tool-output-available` +- Tools: `tool-input-start {toolCallId, toolName}` → `tool-input-delta` → `tool-input-available` → `tool-output-available` *(success)* | `tool-output-error {errorText}` *(failure, AI SDK v5 strict spec — D8.0c+ #89)* - Sources: `source-url {sourceId, url, title?}` / `source-document {sourceId, mediaType, title}` - Custom (ApeRAG 扩展): - `data-citation {cited_text, location: char_location | page_location | content_block_location | url_citation}`（Anthropic-shape） diff --git a/docs/modularization/agent-runtime-mcp-design.md b/docs/modularization/agent-runtime-mcp-design.md index c443214cc..48ccf497a 100644 --- a/docs/modularization/agent-runtime-mcp-design.md +++ b/docs/modularization/agent-runtime-mcp-design.md @@ -89,7 +89,7 @@ Registry 必须存 `(mcpServer, mcpToolName, safeName)` 三元组以保证 safeN D8.3 tool lifecycle implementation 必须 enforce： - **Visibility filter**: 不 visible 的 tool 不进入 agent 的 system prompt tool list - **Consent gate hook**: agent 决定调用 consent-required tool 时，不直接发 `tool-input-start`，而是先发 `data-tool-consent` part，等待用户 decision；approved 后发 `tool-input-start` continue -- **Invocation block**: agent 调用 blocked tool 时直接返回 `tool-output-available` with `errorText: "Tool invocation denied: requires user consent"` +- **Invocation block**: agent 调用 blocked tool 时直接发 `tool-output-error` (AI SDK v5 strict spec, post D8.0c+ #89) with `errorText: "Tool invocation denied: requires user consent"` ## 3. Tool Consent UI in UIMessage Parts @@ -137,7 +137,7 @@ Frontend 渲染 consent UI 用 `toolName + argsPreview + risk` 给用户决策 5. backend updates data-tool-consent part state → "approved" or "denied" 6. backend → agent runtime: - approved: continue with tool-input-start lifecycle (raw args used internally) - - denied: emit tool-output-available with errorText "denied by user" + - denied: emit tool-output-error with errorText "denied by user" (AI SDK v5 strict spec, post D8.0c+ #89) ``` ### 3.3 Timeout policy diff --git a/tests/unit_test/test_agent_runtime_wire_parts.py b/tests/unit_test/test_agent_runtime_wire_parts.py index 03bb9fc3d..131833c8d 100644 --- a/tests/unit_test/test_agent_runtime_wire_parts.py +++ b/tests/unit_test/test_agent_runtime_wire_parts.py @@ -69,6 +69,7 @@ ToolInputAvailablePart, ToolInputStartPart, ToolOutputAvailablePart, + ToolOutputErrorPart, ) @@ -205,7 +206,8 @@ def test_translate_tool_started_finished(): assert isinstance(finished[0], ToolOutputAvailablePart) assert finished[0].tool_call_id == "event-tool" assert finished[0].output == {"items": [{"id": "doc-1"}]} - assert finished[0].error_text is None + # Strict AI SDK v5: success path has no error_text field at all. + assert not hasattr(finished[0], "error_text") def test_translate_tool_failure(): @@ -225,7 +227,9 @@ def test_translate_tool_failure(): assert len(parts) == 1 out = parts[0] - assert isinstance(out, ToolOutputAvailablePart) + # Strict AI SDK v5: failures emit ``tool-output-error``, NOT + # ``tool-output-available`` with errorText (D8.0c+ #89 fix-forward). + assert isinstance(out, ToolOutputErrorPart) assert out.tool_call_id == "event-fail" assert out.error_text == "rate limited" @@ -407,7 +411,8 @@ def test_part_json_round_trip(): metadata={"mcpServer": "aperag", "mcpToolName": "search_collection"}, ), ToolInputAvailablePart(tool_call_id="t-1", tool_name="search_collection", input={"q": "rag"}), - ToolOutputAvailablePart(tool_call_id="t-1", output={"items": []}, error_text=None), + ToolOutputAvailablePart(tool_call_id="t-1", output={"items": []}), + ToolOutputErrorPart(tool_call_id="t-2", error_text="rate limited"), SourceUrlPart(source_id="src-1", url="https://example.com/rag", title="RAG Primer"), DataCitationPart( data={ @@ -447,6 +452,47 @@ def test_part_wire_uses_camel_case_keys(): assert "error_text" not in error +def test_tool_output_strict_split_per_ai_sdk_v5(): + """Strict AI SDK v5: tool failure emits ``tool-output-error``, + success emits ``tool-output-available``. Neither part type carries + the other's payload (D8.0c+ #89 fix-forward).""" + + success_dict = dump_part(ToolOutputAvailablePart(tool_call_id="t-ok", output={"items": []})) + assert success_dict["type"] == "tool-output-available" + # Success path MUST NOT carry an errorText key on the wire. + assert "errorText" not in success_dict + assert "error_text" not in success_dict + + error_dict = dump_part(ToolOutputErrorPart(tool_call_id="t-fail", error_text="rate limited")) + assert error_dict["type"] == "tool-output-error" + assert error_dict["errorText"] == "rate limited" + assert error_dict["toolCallId"] == "t-fail" + # Failure path MUST NOT carry an output key on the wire. + assert "output" not in error_dict + + # Both shapes are recognized by the discriminated union TypeAdapter. + assert isinstance(parse_part(success_dict), ToolOutputAvailablePart) + assert isinstance(parse_part(error_dict), ToolOutputErrorPart) + + +def test_tool_output_available_rejects_error_text_kwarg(): + """ToolOutputAvailablePart no longer accepts ``error_text`` — + constructing it with one must surface as a Pydantic error rather + than silently coexist (regression guard for #89 strict split).""" + + import pydantic + + with pytest.raises(pydantic.ValidationError): + ToolOutputAvailablePart.model_validate( + { + "type": "tool-output-available", + "toolCallId": "t-1", + "output": {"items": []}, + "errorText": "should not be accepted", + } + ) + + # --------------------------------------------------------------------------- # SSE route contract tests # --------------------------------------------------------------------------- diff --git a/web/src/features/agent-runtime/reducer.ts b/web/src/features/agent-runtime/reducer.ts index 712d50e9a..2f88694c4 100644 --- a/web/src/features/agent-runtime/reducer.ts +++ b/web/src/features/agent-runtime/reducer.ts @@ -139,25 +139,20 @@ export function applyPart( }), lastSequence: maxSeq(state.lastSequence, eventId), }; - case 'tool-output-available': { - // BE today (#73) emits failures here too with `errorText` set; - // task #89 splits failures onto `tool-output-error`. We accept - // both shapes so the FE rolls forward without coupling to BE - // timing. - const failed = - typeof part.errorText === 'string' && part.errorText.length > 0; + case 'tool-output-available': + // Strict AI SDK v5 success shape (post task #89 fix-forward): + // success path carries only `output`. Failures arrive as a + // separate `tool-output-error` event handled below. return { ...state, parts: upsertTool(state.parts, part.toolCallId, { output: part.output, - errorText: failed ? part.errorText! : undefined, - state: failed ? 'output-error' : 'output-available', + state: 'output-available', }), lastSequence: maxSeq(state.lastSequence, eventId), }; - } case 'tool-output-error': - // Strict AI SDK v5 failure shape (task #89 fix-forward target). + // Strict AI SDK v5 failure shape (task #89 fix-forward landed). return { ...state, parts: upsertTool(state.parts, part.toolCallId, { diff --git a/web/src/features/agent-runtime/types.ts b/web/src/features/agent-runtime/types.ts index 01024fd71..432dcf029 100644 --- a/web/src/features/agent-runtime/types.ts +++ b/web/src/features/agent-runtime/types.ts @@ -137,12 +137,6 @@ export type StreamPart = type: 'tool-output-available'; toolCallId: string; output: unknown; - // BE `parts.py` (#73) currently sets `errorText` here on failure; - // AI SDK v5 strict spec migrates failure to `tool-output-error` - // (see task #89 D8.0c+ hygiene fix-forward). The reducer accepts - // both shapes so the FE rolls forward without coupling to the - // BE's split timing. - errorText?: string | null; } | { type: 'tool-output-error'; toolCallId: string; errorText: string } | { type: 'source-url'; sourceId: string; url: string; title?: string | null }