-
Notifications
You must be signed in to change notification settings - Fork 79
ON HOLD feat: skyfire payments #253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
commit: |
MichalKalita
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, the generated summary is nice but IMO completely use-less, srry :).
I would appreciate your thoughts more than AI ones.
What about this approach?
I'm not really a fan of passing around AuthToken everywhere, when, most of the time, we just need actual token string. It's a bit clunky
Instead of the AuthToken object, we'll just use plain strings. The ApifyClient will handle figuring out if it's an Apify token or Skyfire token internally, so tools don't need to worry.
We can expose this function for classification, to be reusable (or we can duplicate it).
Why this is better:
• cleaner API - tools just pass the token string
• classification logic lives where it's actually used
• simpler code
We can also live with the authToken but it seems to be a bit overkill.
Yes, I'm being pedantic because you said it was vibe-coded 😜
Thank you for your hawk-eye review 😄 Makes sense, the generated summary is a bit verbose and sometimes not to the point. Will introduce my own 👍 Regarding the passing token around I understand that it would be simpler but we need to somehow "mark" the token provider on the HTTP server level before it gets to the tools since in future we might not be able to recognize the key just by the string value but for example only from header it comes from ( |
|
Well I find this logic a bit flawed .... first, we have this as POC, but at the same time you're making it more complicated to be future-proof. ok, fair enough, let's go out with that. But then please do not name it |
|
closing in favor of v2 PRs |
related to https://github.com/apify/apify-mcp-server-internal/pull/149
This PR prepares the core MCP repo for custom payment agentic key provider.
Backstory
Till now we just received Apify token through Auth header Bearer token or
?tokenquery param. This now changes as we want to implement the Skyfire for agentic payments. So we will be supporting both Apify tokens and also Skyfire payment tokens (PAY) as input.Main idea
The main ideas is the Apify token logic remains as it is and we will also accept the PAY token through the
skyfire-pay-idheader or the Auth Bearer auth header (we can recognize Apify token (starts either withapify_apiorintegration_apifor OAuth). We will then pass that to the MCP tools as we did with the Apify token and then forward it to the Apify API server.Technical details
For this I introduced the
AuthTokentype so we can clearly mark from which provider (currently only Apify or Skyfire) the token is since we may support multiple providers in future and they may not be easily recognizable by their string value but for example only via the header they come from - this needs to be somehow passed from the HTTP server in this wrapper.Additional changes
getRealActorID()andgetActorDefinition().