Skip to content

Conversation

@MQ37
Copy link
Contributor

@MQ37 MQ37 commented Apr 14, 2025

This PR adds functionality for calling Actorized MCP servers from the Apify store, esentially acting as a MCP proxy.

Our MCP server endpoint (MCPAPI) - that calls the Actorized MCP servers (AMCP); Always creates short lived connections to the AMCPs - initial one for tool list and each for each tool call acting as a proxy.

We check if the Actor is an MCP server if it has the webServerMcpPath from actorDefinition. This value is also used as MCP server path -> {standbyURL}{MCPPath}.

TODOs:

Deployed and tested @ https://mcp-securitybyobscurity.apify.com/ - both nornal Actors (https://securitybyobscurity.apify.com/jakub.kopecky/python-example) and Actorized MCp server (https://securitybyobscurity.apify.com/jakub.kopecky/apify-mcp-server)

MQ37 and others added 5 commits April 1, 2025 16:26
@MQ37 MQ37 added the beta Create beta prereleases label Apr 14, 2025
@MQ37 MQ37 marked this pull request as ready for review April 14, 2025 19:36
@MQ37 MQ37 added beta Create beta prereleases and removed beta Create beta prereleases labels Apr 14, 2025
@MQ37 MQ37 force-pushed the feat/actorized-mcp branch from 83236d5 to 3543a5e Compare April 14, 2025 20:04
@MQ37 MQ37 marked this pull request as draft April 15, 2025 11:58
@MQ37 MQ37 requested a review from Copilot April 15, 2025 13:20
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

@MQ37 MQ37 marked this pull request as ready for review April 15, 2025 13:23
@MQ37 MQ37 requested a review from jirispilka April 15, 2025 14:27
Copy link
Collaborator

@jirispilka jirispilka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After all, I like the separation of actor, mcp, tools ...

It makes it nicer 👍🏻 .... but ... when we are both working on the same repo it would be better not to move things that much.

The code will need more love, but let's get out a working version first.

So the only issue that is not clear is getActorDefinition function. Perhaps there are other things that I missed.

We will need to fix lint issues from the other PR, which contains lint fixes changes: #72

Comment on lines +177 to +183
export type Input = {
actors: string[] | string;
enableActorAutoLoading?: boolean;
maxActorMemoryBytes?: number;
debugActor?: string;
debugActorInput?: unknown;
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, we should have this separate Actor vs mcp-server

Copy link
Contributor Author

@MQ37 MQ37 Apr 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved that since functions used in mcp-server are using this type. I agree that in future we should separate this

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, this should be divided into Actor vs mcp-server input

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also agree, we should refactor this

Comment on lines 53 to 76
export async function getActorDefinition(actorID: string, apifyToken: string): Promise<ActorDefinition> {
const apifyClient = new ApifyClient({ token: apifyToken
})
const actor = apifyClient.actor(actorID);
const info = await actor.get();
if (!info) {
throw new Error(`Actor ${actorID} not found`);
}
const latestBuildID = info.taggedBuilds?.['latest']?.buildId;
if (!latestBuildID) {
throw new Error(`Actor ${actorID} does not have a latest build`);
}
const build = apifyClient.build(latestBuildID);
const buildInfo = await build.get();
if (!buildInfo) {
throw new Error(`Build ${latestBuildID} not found`);
}
const actorDefinition = buildInfo.actorDefinition;
if (!actorDefinition) {
throw new Error(`Build ${latestBuildID} does not have an actor definition`);
}

return actorDefinition;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we need to add a new function?
We have an existing function for that

/**
 * Get actor input schema by actor name.
 * First, fetch the actor details to get the default build tag and buildId.
 * Then, fetch the build details and return actorName, description, and input schema.
 * @param {string} actorIdOrName - Actor ID or Actor full name.
 * @param {number} limit - Truncate the README to this limit.
 * @returns {Promise<ActorDefinitionWithDesc | null>} - The actor definition with description or null if not found.
 */
export async function getActorDefinition(actorIdOrName: string, limit: number = ACTOR_README_MAX_LENGTH): Promise<ActorDefinitionPruned | null> {
    const client = new ApifyClient({ token: process.env.APIFY_TOKEN });
    const actorClient = client.actor(actorIdOrName);

    try {
        // Fetch actor details
        const actor = await actorClient.get();
        if (!actor) {
            log.error(`Failed to fetch input schema for Actor: ${actorIdOrName}. Actor not found.`);
            return null;
        }

        // fnesveda: The default build is not necessarily tagged, you can specify any build number as default build.
        // There will be a new API endpoint to fetch a default build.
        // For now, we'll use the tagged build, it will work for 90% of Actors. Later, we can update this.
        const tag = actor.defaultRunOptions?.build || '';
        const buildId = actor.taggedBuilds?.[tag]?.buildId || '';

        if (!buildId) {
            log.error(`Failed to fetch input schema for Actor: ${actorIdOrName}. Build ID not found.`);
            return null;
        }
        // Fetch build details and return the input schema
        const buildDetails = await client.build(buildId).get();
        if (buildDetails?.actorDefinition) {
            const actorDefinitions = buildDetails?.actorDefinition as ActorDefinitionWithDesc;
            actorDefinitions.id = actor.id;
            actorDefinitions.readme = truncateActorReadme(actorDefinitions.readme || '', limit);
            actorDefinitions.description = actor.description || '';
            actorDefinitions.actorFullName = `${actor.username}/${actor.name}`;
            actorDefinitions.defaultRunOptions = actor.defaultRunOptions;
            return pruneActorDefinition(actorDefinitions);
        }
        return null;
    } catch (error) {
        const errorMessage = `Failed to fetch input schema for Actor: ${actorIdOrName} with error ${error}.`;
        log.error(errorMessage);
        throw new Error(errorMessage);
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not want to modify this function but it returns only pruned actorDefinition for the Actorized MCP purpose we need to get the webServerMcpPath from top level of actorDefinition

@MQ37
Copy link
Contributor Author

MQ37 commented Apr 16, 2025

After all, I like the separation of actor, mcp, tools ...

It makes it nicer 👍🏻 .... but ... when we are both working on the same repo it would be better not to move things that much.

The code will need more love, but let's get out a working version first.

So the only issue that is not clear is getActorDefinition function. Perhaps there are other things that I missed.

We will need to fix lint issues from the other PR, which contains lint fixes changes: #72

Thank you for review and effort 👍 The code definitely needs more love and refactoring, let's do that as a first thing after we deploy first functional version, and let's also add more tests.

@MQ37
Copy link
Contributor Author

MQ37 commented Apr 16, 2025

after conflicts are resolved we need to merge #73

jirispilka and others added 2 commits April 16, 2025 13:39
* fix get default build in get actor definition

* fix tests

* fix double import, lint
@MQ37 MQ37 merged commit 28d5621 into feat/decouple Apr 16, 2025
1 check passed
@MQ37 MQ37 deleted the feat/actorized-mcp branch April 16, 2025 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants