feat: Migrate direct Actor tools to RunResponse with waitSec#853
Merged
MQ37 merged 10 commits intoMay 18, 2026
Conversation
…e with itemsSchema support
…ove unused appsActorExecutor import
github-actions
Bot
added
t-ai
…ts for consistency
…it until terminal
…it until terminal
Collaborator
Author
|
E2E tested via
Session: https://claude.ai/code/session_0152Dv5wbydZEnhvHbPeTAbz Generated by Claude Code |
MQ37
approved these changes
May 18, 2026
Contributor
MQ37
left a comment
MQ37
left a comment
There was a problem hiding this comment.
Good job! I was playing with it and it works really well, it makes the MCP server experience much better.
there is only one refactor change, but let's make a separate issue where we address all the issue with this PR chain, so we can prevent any further merge conflicts.
While playing with it I discovered an issue when trying to call Actor MCP server, that was supposedly there from February essentially preventing any real client from using the Actor MCP servers through the call-actor tool 🤯 🤦 Fix stacked on top of this PR #859
Comment on lines
+402
to
+427
| return { | ||
| ...getActorRunOutputSchema, | ||
| properties: { | ||
| ...getActorRunOutputSchema.properties, | ||
| storages: { | ||
| ...getActorRunOutputSchema.properties.storages, | ||
| properties: { | ||
| ...getActorRunOutputSchema.properties.storages.properties, | ||
| datasets: { | ||
| ...datasets, | ||
| properties: { | ||
| ...datasets.properties, | ||
| default: { | ||
| ...defaultDataset, | ||
| properties: { | ||
| ...defaultDataset.properties, | ||
| itemsSchema, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
refactor: oh my god! 🤯 we need to refactor this - what about clone and setting the fields directly?
Collaborator
Author
There was a problem hiding this comment.
yeah, this is terrible
4 tasks
…ugh (#859) ## Context `call-actor` with MCP-server pass-through syntax (`actor: "apify/actors-mcp-server:search-apify-docs"`) fails on master with `MCP error -32600: Tool call-actor has an output schema but did not return structured content`. Repro with `mcpc --json @apify tools-call call-actor actor:='"apify/actors-mcp-server:search-apify-docs"' input:='{"query":"weather"}'`. Regression introduced in #415 (`f9512c4`, 2026-01-29) when `outputSchema` was added to `call-actor`; the MCP-pass-through code in `handleMcpToolCall` has been returning `{ content }` only since #274 (2025-09-18). Both ingredients sat dormant until the SDK on `^1.25.2` (already enforcing since 1.11.4) saw them combined. ## Solution In `handleMcpToolCall`, synthesize a sentinel `RunResponse` matching `getActorRunOutputSchema.required` (`runId: 'mcp-passthrough'`, `actorId: baseActorName`, `status`, `storages: {}`, `summary`, `nextStep`) and forward `result.isError` from the remote tool. The remote tool's payload still flows through `content`. Stacks on #853. ## Worth your attention - **Sentinel runId** — `'mcp-passthrough'` is a deliberate non-Apify literal so logs / dashboards never mistake it for a real run id. Open to `mcp-passthrough:${mcpToolName}` for greppability per tool if preferred. - **Integration coverage** — the existing happy-path test for this code path didn't catch the regression because it never calls `client.listTools()`, so the SDK never builds the validator cache. The new test at `tests/integration/suite.ts:813` calls `listTools()` first to mirror real clients (mcpc, Claude Desktop), which is the only way to surface this class of bug at the integration layer. - **Not the architectural fix** — `call-actor` doing two unrelated things (Apify run vs. MCP tool pass-through) under one tool name is the root cause; this PR keeps that shape and just makes the response spec-compliant. Follow-up tracked in #860 (see comment). ## Follow-up - Split `call-actor` MCP pass-through into a dedicated tool, or move to code-mode as the default. Issue: #860
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up from #825 Direct Actor tools now share the same contract as
call-actorandget-actor-run. Finally one uniform experience, this is what I was looking forward to when writing #825 :)Much smaller PR than #825 this time, just the migration and merging two executors into one. It actually removes some code, which is good.
About the
itemsSchemagotcha called out in #852 — LLMs may read `itemsSchema.properties` as "these fields are in `structuredContent`", which they are not. Rows live in the dataset and you fetch them with `get-dataset-items`. The `itemsSchema.description` says this explicitly, so it should be fine.I tested it, it works but I did not tested thoroughly, there might be some rough edges.
Summary
Direct Actor tools now return the canonical `RunResponse` shape — same as `call-actor` and `get-actor-run`: `runId`, `status`, `storages`, `summary`, `nextStep`. No more inlined items; the LLM follows `nextStep` to `get-dataset-items`.
Same `waitSecs` contract as `call-actor` — same MCP-only opt-in: default 30 s, max 45 s. Task mode overrides `waitSecs` and waits until terminal. If the Actor's own input schema happens to declare `waitSecs`, ours wins so there's no collision.
Per-tool `itemsSchema` in the dataset metadata — for direct Actor tools the target Actor is known at `tools/list` time, so the historical-inferred row schema lands both in the tool's `outputSchema` and in the runtime response under `storages.datasets.default.itemsSchema`. LLMs can plan a `fields` projection before the first call.
One mode-agnostic executor — separate default-mode and apps-mode executors are gone, replaced by a single one. Direct Actor tools never render widgets (`widget: false`), so the apps-mode executor was not doing anything different anymore.
Closes #852. Phase 2 of #587.