-
Notifications
You must be signed in to change notification settings - Fork 6k
feat(plugin): use SDK v2 #8380
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
base: dev
Are you sure you want to change the base?
feat(plugin): use SDK v2 #8380
Conversation
|
Hey! Your PR title Please update it to start with one of:
Where See CONTRIBUTING.md for details. |
|
The following comment was made by an LLM, it may be inaccurate: Based on my search, I found a related PR: Related/Potential Duplicate:
Why it's related: This PR is explicitly mentioned in your description. PR #7639 addresses the same issue (#7641) and takes a similar approach to migrate the plugin system to SDK v2. However, your PR (#8380) takes a different, less breaking approach by exposing both the v1 Recommendation: Consider whether both PRs are needed or if one approach should supersede the other. If #7639 is still open, you may want to coordinate with its author or potentially mark one as a duplicate depending on which approach the maintainers prefer. |
| input: { sessionID: string; agent: string; model: Model; provider: ProviderContext; message: UserMessage }, | ||
| output: { temperature: number; topP: number; topK: number; options: Record<string, any> }, | ||
| ) => Promise<void> | ||
| "permission.ask"?: (input: Permission, output: { status: "ask" | "deny" | "allow" }) => Promise<void> |
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.
The Permission type was technically a lie. There is no analog in SDK v2, so rolled one that is correct (which is already a common pattern here).
What does this PR do?
Fixes #7641
Fixes #7147
While investigating this for my own plugin, I arrived at a similar direction as #7639. I'm wasn't sure why several of the changes in that PR were made, and it also introduces breaking changes for plugin developers. So I decided to roll a new PR out.
This PR takes a different approach with a minimal set of updates to enable the plugin system to use the
v2client. It also exposes thev2client asclientNextviaPluginInput.Important
Providing two clients avoids the backwards-compatibility issue. Keeping the original
clientintact allowsclientNextto adopt SDK v2 without forcing developers to migrate immediately. Plugin authors can move at their own pace, and we can defer any breaking changes until the next major release.How did you verify your code works?