feat: mutlti root workspace support#1333
Conversation
🦋 Changeset detectedLatest commit: 5b49c73 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8718f5bdd2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@faga295 Thanks a lot for putting this PR together! I was looking into the additionalDirectories feature and testing things out, and I noticed a couple of edge cases we might need to handle to fully align with the ACP specification https://agentclientprotocol.com/protocol/v1/session-setup#additional-workspace-roots. I found two specific requirements in the spec that might need a bit more enforcement in the current implementation:
I've actually experimented with a way to handle both of these! We could add a validateAdditionalDirectories helper and pass the resolved roots into AcpKaos . Something like this: Would you be open to incorporating something like this into your PR? I'd be happy to help out or submit a patch to your branch if that makes it easier. Thanks again for driving this feature! 🚀 |
|
@luckzylp Thanks a lot for your suggestions. You’re welcome to help fix this issue with us. I’ve granted you access to my fork repo, so feel free to push your changes there. |
|
@luckzylp For your second point, I’m not sure this is the right approach. Rather than passing |
|
@faga295 This is merely my understanding of the ACP specification. Updating the workspace scope in core is exactly what we are doing via However, we also must pass Because Therefore, injecting it into Currently, I have attempted to push the code directly to your repository (but failed). I have submitted a pull request to your repository and I sincerely welcome everyone to review and discuss. |
@luckzylp Since |
Related Issue
Resolve #(issue_number)
#1287
Problem
See linked issue
What changed
Implemented ACP multi-root workspace support for Zed and other ACP clients.
The adapter now advertises
sessionCapabilities.additionalDirectories, so clients know Kimi Code supports multi-root workspaces during initialize. It also maps ACP additionalDirectories to Kimi Code SDK/core additionalDirs forsession/new,session/load, andsession/resume.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.