Feat/mcp structured tool results by Asaf-prog · Pull Request #135 · extra-org/extra

Asaf-prog · 2026-09-01T21:11:12Z

Summary

Preserve structured MCP tool results throughout Extra's execution pipeline instead of collapsing every successful tool result into plain text.

This builds on #133, which fixed MCP text extraction, and adds a provider-agnostic normalized tool-result model that keeps:

model-facing text;
machine-readable structured data;
bounded artifact metadata.

The structured result now survives hooks, persistence, idempotent replay, and HITL resume without leaking provider-specific LangChain objects into the core runtime.

What changed

Added an immutable normalized tool-result abstraction owned by Extra.
Preserve MCP structuredContent instead of dropping it.
Preserve local structured dict / list tool results where applicable.
Use deterministic canonical JSON for structured-only model-facing fallback.
Keep only text in the model conversation; structured/artifact data is not automatically exposed through ToolMessage.
Added versioned serialization with backwards compatibility for legacy text-only execution records.
Preserve structured results through idempotent replay and HITL resume.
Added atomic claim/wait semantics so concurrent duplicate calls do not execute the same tool side effect more than once.
Keep existing text-based transform_tool_result hooks compatible while preserving structured data.
Added bounded artifact normalization and safer handling of malformed/non-serializable provider values.
Avoid logging raw structured tool results or provider-supplied values.

Runtime flow

Provider / LangChain result
↓
Tool-result normalization
↓
NormalizedToolResult
├── text
├── structured
└── bounded artifact metadata
↓
Hooks
↓
Execution ledger
↓
Replay / HITL resume

text only
↓
Model conversation

Backwards compatibility

Existing local string tools behave as before.
MCP text-only results keep the behavior introduced by fix(engine): extract clean text from MCP tool results instead of stri… #133.
Legacy text-only execution records can still be restored.
Existing text transformation hooks remain supported.

Tests

Added/updated coverage for:

MCP text-only and multi-block results;
MCP structuredContent;
structured-only results;
deterministic JSON fallback;
local structured and string tool results;
malformed/non-serializable provider values;
artifact size and nesting limits;
sequential and concurrent idempotent replay;
terminal execution-state protection;
legacy ledger compatibility;
structured result preservation through hooks;
HITL suspend/resume;
prevention of structured data leaking into logs/callback metadata.

Validation

Ruff format/lint passed.
Mypy passed.
Pytest: 965 passed.
git diff --check passed.

Out of scope

Full multimodal MCP support.
Binary artifact persistence.
Widget rendering for structured/artifact results.
Universal output-schema validation.
Provider-specific model serialization strategies.

Add an immutable, versioned tool-result contract and make execution claims safe for concurrent replay. Preserve legacy text records while preventing terminal ledger overwrites.

Normalize MCP and local tool outputs once at the provider boundary, expose structured values to trusted hooks, and keep model messages and logs text-only. Cover structured-only, malformed, replay, concurrency, and HITL behavior.

Document normalization ownership, model visibility, artifact bounds, hook compatibility, and concurrency-safe persisted replay in ADR 0004 and runtime guides.

AmitAvital1

Requesting changes for correctness and contract issues found in the structured-result execution path; details are inline.

Make terminal execution writes cancellation-safe, model execution states with a StrEnum, bound structured JSON payloads, and require the MCP adapter version that provides the structured-content artifact contract. Add regressions for cancellation with duplicate waiters and oversized MCP structured results.

Record the structured JSON budgets, execution status enum, cancellation-safe ledger expectations, and langchain-mcp-adapters 0.2 minimum.

Asaf-prog · 2026-09-02T18:19:45Z

Thanks for the detailed review — I addressed all four findings:

Made terminal execution writes cancellation-safe. Failed results are persisted before secondary usage/hook processing, and cancellation waits for the shielded ledger write to finish. Added a regression proving concurrent duplicate callers do not hang or re-execute the provider.
Raised the langchain-mcp-adapters minimum version to >=0.2, matching the structured-content artifact contract used by the normalizer.
Added bounded JSON validation for structured results: depth, total values, string/key bytes, integer size, and final encoded size are limited. Oversized MCP structured results now produce controlled tool errors.
Replaced repeated execution-state strings with the wire-compatible ToolExecutionStatus StrEnum.
Documentation and ADR 0004 were updated accordingly. The full repository gate passes: 969 tests, Ruff, formatting, and mypy

AmitAvital1

Re-reviewed 91f90c3: the dependency floor, bounded JSON policy, and execution-status enum are fixed and those threads are resolved.
Cancellation safety still double-finalizes a successful execution under cancellation; see the inline reproduction, so I cannot approve this head yet.

Keep the terminal success write outside pre-publication recovery so cancellation does not attempt a second failed finalization. Add a regression covering the waiting duplicate and exactly-once provider execution.

Asaf-prog · 2026-09-04T13:04:05Z

Thanks for the re-review and clear reproduction. I’ve fixed the remaining cancellation race.

The success path now distinguishes between:

failures during pre-publication result processing, which finalize the execution as FAILED; and
cancellation during the terminal success write, which preserves the committed SUCCEEDED result and propagates CancelledError without attempting a second write.

I also added the symmetric regression test. It verifies that cancellation during a blocked successful write:

does not raise the terminal-overwrite ValueError;
preserves the successful result;
unblocks the duplicate caller;
returns the same result to that caller; and
executes the provider exactly once.

Validation is clean: Ruff, mypy, and the full suite pass with 970 tests passed.

AmitAvital1

Re-reviewed 828b6de: the cancellation race now preserves the terminal success result, propagates CancelledError, and keeps provider execution exactly once.
The regression test and independent reproduction pass; all previous findings are resolved and CI is green.

Asaf-prog added 3 commits September 2, 2026 00:05

feat(runtime): persist normalized tool execution results

e1d0427

Add an immutable, versioned tool-result contract and make execution claims safe for concurrent replay. Preserve legacy text records while preventing terminal ledger overwrites.

feat(engine): preserve structured tool results

dfedaac

Normalize MCP and local tool outputs once at the provider boundary, expose structured values to trusted hooks, and keep model messages and logs text-only. Cover structured-only, malformed, replay, concurrency, and HITL behavior.

docs(runtime): define structured tool result contract

6206ec1

Document normalization ownership, model visibility, artifact bounds, hook compatibility, and concurrency-safe persisted replay in ADR 0004 and runtime guides.

Asaf-prog requested a review from AmitAvital1 September 1, 2026 21:11

Asaf-prog self-assigned this Sep 1, 2026

AmitAvital1 requested changes Sep 2, 2026

View reviewed changes

Comment thread src/agent_engine/engine/langgraph/tools/tool_invoker.py Outdated

Comment thread src/agent_engine/engine/langgraph/tools/tool_result_normalizer.py

Comment thread src/agent_engine/runtime/tool_results.py

Comment thread src/agent_engine/approvals/models.py Outdated

Asaf-prog added 2 commits September 2, 2026 21:15

docs(runtime): document review-driven result safeguards

91f90c3

Record the structured JSON budgets, execution status enum, cancellation-safe ledger expectations, and langchain-mcp-adapters 0.2 minimum.

AmitAvital1 reviewed Sep 3, 2026

View reviewed changes

Comment thread src/agent_engine/engine/langgraph/tools/tool_invoker.py Outdated

AmitAvital1 requested changes Sep 3, 2026

View reviewed changes

fix(runtime): preserve successful result on cancellation

828b6de

Keep the terminal success write outside pre-publication recovery so cancellation does not attempt a second failed finalization. Add a regression covering the waiting duplicate and exactly-once provider execution.

AmitAvital1 approved these changes Sep 4, 2026

View reviewed changes

Asaf-prog merged commit b9acf1e into main Sep 4, 2026
3 checks passed

This was referenced Sep 7, 2026

feat(engine): support MCP structuredContent and artifacts in tool results (#134) #136

Closed

Support MCP structuredContent and Artifacts in Tool Results #134

Closed

Navigation Menu

Uh oh!

Feat/mcp structured tool results - #135

Feat/mcp structured tool results#135
Asaf-prog merged 6 commits into
mainfrom
feat/mcp-structured-tool-results

Asaf-prog commented Sep 1, 2026

Uh oh!

AmitAvital1 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Asaf-prog commented Sep 2, 2026

Uh oh!

Uh oh!

AmitAvital1 left a comment

Uh oh!

Asaf-prog commented Sep 4, 2026

Uh oh!

AmitAvital1 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Asaf-prog commented Sep 1, 2026

Summary

What changed

Runtime flow

Backwards compatibility

Tests

Validation

Out of scope

Uh oh!

AmitAvital1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Asaf-prog commented Sep 2, 2026

Uh oh!

Uh oh!

AmitAvital1 left a comment

Choose a reason for hiding this comment

Uh oh!

Asaf-prog commented Sep 4, 2026

Uh oh!

AmitAvital1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants