feat: mutlti root workspace support by faga295 · Pull Request #1333 · MoonshotAI/kimi-code

faga295 · 2026-07-03T02:54:32Z

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 for session/new, session/load, and session/resume.

Checklist

I have read the CONTRIBUTING document.
I have linked a related issue, or explained the problem above.
I have added tests that prove my feature works.
Ran gen-changesets skill, or this PR needs no changeset.
Ran gen-docs skill, or this PR needs no doc update.

changeset-bot · 2026-07-03T02:54:35Z

🦋 Changeset detected

Latest commit: 59c4ea6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@moonshot-ai/kimi-code	Patch
@moonshot-ai/kimi-code-sdk	Patch
@moonshot-ai/acp-adapter	Patch

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

chatgpt-codex-connector

💡 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".

luckzylp · 2026-07-03T03:33:53Z

@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:

Absolute Path Validation & Fail-Closed
The spec mentions: "each additionalDirectories entry MUST be an absolute path". Right now, if a client sends a relative path or invalid data, it gets passed directly to the harness.
It might be safer to validate this at the adapter layer and return a JSON-RPC invalid_params error if it's not an absolute path.
Enforcing File System Boundaries
The spec also mentions: "The session's effective root set is [cwd, ...additionalDirectories] . This root set SHOULD serve as a boundary for tool operations". It looks like
maybeBuildAcpKaos currently doesn't receive the additionalDirectories to inject them into AcpKaos , which means the sandbox might not know about these new boundaries.

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:

   // 1. Validation helper
    export function validateAdditionalDirectories(dirs: unknown): string[] | undefined {
      if (dirs === undefined || dirs === null) return undefined;
      if (!Array.isArray(dirs)) throw RequestError.invalidParams({ additionalDirectories: dirs }, 'must be an array');
  
      for (let i = 0; i < dirs.length; i++) {
        const entry = dirs[i];
        if (typeof entry !== 'string' || entry === '') {
          throw RequestError.invalidParams({ additionalDirectories: dirs }, `entry at [${i}] must be a non-empty string`);
         }
         if (!path.isAbsolute(entry)) {
           throw RequestError.invalidParams({ additionalDirectories: dirs }, `entry at [${i}] must be an absolute path`);
         }
       }
       return dirs;
     }
  
     // 2. Injecting into AcpKaos (inside maybeBuildAcpKaos)
     const roots = [cwd, ...(additionalDirs ?? [])];
     const canonical = await resolveCanonicalRoots(roots);
     return new AcpKaos(this.conn, sessionId, innerKaos, canonical); // Passing canonical roots here

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! 🚀

faga295 · 2026-07-03T06:49:39Z

@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.

faga295 · 2026-07-03T10:36:02Z

@luckzylp For your second point, I’m not sure this is the right approach. Rather than passing additionalDirectories into kaos, I think we should update the workspace scope in core when an active session is matched.

- Add path-boundary module with resolveCanonicalPath, resolveCanonicalRoots and assertPathInRoots primitives - Inject effectiveRoots into AcpKaos to validate every file operation against canonical workspace roots - Add validateAdditionalDirectories to reject malformed additionalDirectories at session-init time - Wire canonical roots through newSession/loadSession/resumeSession lifecycle

luckzylp · 2026-07-03T11:27:30Z

@faga295 This is merely my understanding of the ACP specification.

Updating the workspace scope in core is exactly what we are doing via harness.resumeSession({additionalDirs }) , which translates to active.setAdditionalDirs() in core-impl.ts .

However, we also must pass additionalDirectories into AcpKaos . AcpKaos serves as the strict security boundary for reverse-RPCs sent to the client (like Zed). It intercepts FS operations and runs assertPathInRoots before asking the editor to read/write files to prevent sandbox escapes.

Because AcpKaos is instantiated and injected before the session is created, it cannot dynamically query core for the current workspace scope. If we don't pass additionalDirectories to maybeBuildAcpKaos , its internal effectiveRoots will only contain [cwd] . Consequently, AcpKaos will forcefully reject any legal file reads to additionalDirectories with a KaosError , even if core considers them within the scope.

Therefore, injecting it into Kaos at creation time is necessary to keep the underlying FS abstraction's boundaries aligned with core 's scope.

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.

faga295 · 2026-07-03T16:08:31Z

If we don't pass additionalDirectories to maybeBuildAcpKaos , its internal effectiveRoots will only contain [cwd] . Consequently, AcpKaos will forcefully reject any legal file reads to additionalDirectories with a KaosError , even if core considers them within the scope.

@luckzylp Since AcpKaos doesn’t currently have effectiveRoots, I don’t think we should introduce the concept of roots just yet. Passing additionalDirs expands the file read/write scope to [workDir, ...additionalDirs], and I believe that behavior is expected.

Remove the assertPathInRoots boundary checks from every AcpKaos file operation (stat, readText, writeText, etc.) and the associated resolveCanonicalRoots machinery. Keep the validateAdditionalDirectories function for input-level validation of the additionalDirectories parameter.

luckzylp · 2026-07-04T14:20:15Z

@faga295 That makes total sense, and I agree with your point.

I did a quick look into how Claude Code and Codex handle this, and they both treat the ACP adapter as a thin protocol layer—delegating boundary enforcement entirely to the underlying Agent SDK/Runtime rather than doing path validation inside the adapter itself.

Given this, introducing effectiveRoots into AcpKaos is indeed unnecessary. I'll make sure our implementation aligns with this by handling the workspace scope on the core/SDK side instead.

I have updated the PR, and let me know if you have any further feedback!

feat(acp-adapter): enforce path boundary for additionalDirectories

faga295 · 2026-07-04T20:33:18Z

@codex

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 45e0462096

ℹ️ 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".

chatgpt-codex-connector · 2026-07-04T20:38:12Z

      cwd: params.cwd,
      sessionId: params.sessionId,
      mcpServers: params.mcpServers,
+      additionalDirs,


Clear omitted ACP roots on resume

When an ACP client resumes or loads an already-cached session without sending additionalDirectories, this forwards undefined into the SDK. If the client also lacks ACP fs capabilities, KimiHarness.resumeSession returns the cached session without calling core, so any prior additionalDirs remain active even though the ACP additional-directories spec says omitted or empty on session/load/session/resume means the resulting additional-root list is []. That can leave old workspace roots available after a client intentionally resumes with none; pass an explicit empty array for load/resume when the field is absent.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-07-04T20:38:12Z

+export function validateAdditionalDirectories(
+  dirs: unknown,
+): string[] | undefined {
+  if (dirs === undefined || dirs === null) return undefined;


Reject null additionalDirectories

This treats additionalDirectories: null as if the field were omitted, but the ACP field is only valid when absent or an array of non-empty absolute path strings; null is a present non-array value and should produce invalid_params. Accepting it silently makes malformed clients look successful and bypasses the validation this helper is meant to provide.

Useful? React with 👍 / 👎.

…ad/resume, reject null

… omitted additionalDirs

luckzylp · 2026-07-05T04:54:19Z

@faga295 I have submitted a pull request to fix these two review issues, and let me know if you have any further feedback!

fix(acp-adapter): normalize omitted additionalDirectories to [] on lo…

faga295 · 2026-07-05T17:12:57Z

@codex

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 661d7a15fc

ℹ️ 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 · 2026-07-06T03:01:42Z

@codex

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ae1f8506f6

ℹ️ 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 · 2026-07-06T03:32:37Z

@codex

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b2d8516f13

ℹ️ 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".

When an unauthenticated client sends session/load or session/resume with a malformed additionalDirectories value, this validation previously threw invalid_params before setupSessionFromExisting could run the existing auth gate. This changes the unauthenticated error to an auth_required error for these inputs.

luckzylp · 2026-07-06T04:41:32Z

@faga295 I have submitted a pull request to fix this review issue, and let me know if you have any further feedback!

fix(acp-adapter): preserve auth_required before validating extra roots

faga295 · 2026-07-06T06:20:51Z

@codex

chatgpt-codex-connector · 2026-07-06T06:25:40Z

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 59c4ea6d69

ℹ️ 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".

feat: mutlti root workspace support

8718f5b

chatgpt-codex-connector Bot reviewed Jul 3, 2026

View reviewed changes

Comment thread packages/acp-adapter/src/server.ts Outdated

luckzylp and others added 2 commits July 3, 2026 18:20

Merge branch 'main' into feat/multi_root_workspace_support

99bb778

fix: active session resume when additionDirectories exist

5b49c73

Merge pull request #1 from luckzylp/feat/multi_root_workspace_support

45e0462

feat(acp-adapter): enforce path boundary for additionalDirectories

chatgpt-codex-connector Bot reviewed Jul 4, 2026

View reviewed changes

luckzylp added 2 commits July 5, 2026 12:18

fix(acp-adapter): normalize omitted additionalDirectories to [] on lo…

9df47bc

…ad/resume, reject null

test(acp-adapter): update tests to expect [] instead of undefined for…

dcb7faa

… omitted additionalDirs

Merge pull request #2 from luckzylp/feat/multi_root_workspace_support

661d7a1

fix(acp-adapter): normalize omitted additionalDirectories to [] on lo…

chatgpt-codex-connector Bot reviewed Jul 5, 2026

View reviewed changes

Comment thread packages/node-sdk/src/kimi-harness.ts Outdated

feat: update summary

ae1f850

chatgpt-codex-connector Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread .changeset/remove-acp-path-boundary.md Outdated

chore: update changeset

b2d8516

chatgpt-codex-connector Bot reviewed Jul 6, 2026

View reviewed changes

Comment thread packages/acp-adapter/src/server.ts Outdated

Merge pull request #3 from luckzylp/feat/multi_root_workspace_support

59c4ea6

fix(acp-adapter): preserve auth_required before validating extra roots

Uh oh!

Conversation

faga295 commented Jul 3, 2026

Related Issue

Problem

What changed

Checklist

Uh oh!

changeset-bot Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

luckzylp commented Jul 3, 2026

Uh oh!

faga295 commented Jul 3, 2026

Uh oh!

faga295 commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luckzylp commented Jul 3, 2026

Uh oh!

faga295 commented Jul 3, 2026

Uh oh!

luckzylp commented Jul 4, 2026

Uh oh!

faga295 commented Jul 4, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 4, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 4, 2026

Choose a reason for hiding this comment

Uh oh!

luckzylp commented Jul 5, 2026

Uh oh!

faga295 commented Jul 5, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

faga295 commented Jul 6, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

faga295 commented Jul 6, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

luckzylp commented Jul 6, 2026

Uh oh!

faga295 commented Jul 6, 2026

Uh oh!

chatgpt-codex-connector Bot commented Jul 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

changeset-bot Bot commented Jul 3, 2026 •

edited

Loading

faga295 commented Jul 3, 2026 •

edited

Loading