fix(acp): restore legacy permission compatibility and stabilize ACP

.changeset/acp-sdk-and-cli-fixes.md

@@ -0,0 +1,8 @@
+---
+"@moonshot-ai/acp-adapter": patch
+"@moonshot-ai/agent-core": patch
+"@moonshot-ai/kimi-code-sdk": patch
+"@moonshot-ai/kimi-code": patch
+---
+
+Fix ACP slash skill routing, bootstrap context reads, file and permission edge cases, subagent event handling, and stale-file edit messaging.

apps/kimi-code/src/cli/sub/acp.ts

@@ -21,11 +21,17 @@
 
 import type { Command } from 'commander';
 
-import { runAcpServer } from '@moonshot-ai/acp-adapter';
-import { createKimiHarness } from '@moonshot-ai/kimi-code-sdk';
+import {
+  runAcpServer,
+  type AvailableCommand,
+  type SlashCommandsSnapshot,
+} from '@moonshot-ai/acp-adapter';
+import { createKimiHarness, type Session, type SkillSummary } from '@moonshot-ai/kimi-code-sdk';
 
 import { KIMI_CODE_HOME_ENV } from '#/constant/app';
 import { createKimiCodeHostIdentity, getVersion } from '#/cli/version';
+import { BUILTIN_SLASH_COMMANDS } from '#/tui/commands/registry';
+import { buildSkillSlashCommands } from '#/tui/commands/skills';
 
 import { runLoginFlow } from './login-flow';
 
@@ -66,9 +72,45 @@ export function registerAcpCommand(parent: Command): void {
       // client can spawn it with `args:['login']` for the top-level
       // `kimi login` subcommand — matches kimi-cli `acp/server.py:77-96`.
       const legacyCommand = process.argv[1];
+      const builtinCommands: AvailableCommand[] = BUILTIN_SLASH_COMMANDS.map((cmd) => ({
+        name: cmd.name,
+        description: cmd.description,
+      }));
+      // Skills are session-scoped (per-cwd config), so we defer the
+      // listSkills() call until the adapter hands us the just-created
+      // Session — mirrors opencode's per-directory snapshot. A
+      // listSkills() failure degrades to builtins-only so a broken
+      // skill source never blanks the palette.
+      const resolveSlashCommands = async (
+        session: Session,
+      ): Promise<SlashCommandsSnapshot> => {
+        let skills: readonly SkillSummary[] = [];
+        try {
+          skills = await session.listSkills();
+        } catch {
+          skills = [];
+        }
+        // `buildSkillSlashCommands` already returns both views — the
+        // palette entries (advertised via `available_commands_update`)
+        // and the `commandName → skillName` map the adapter uses to
+        // intercept `/skill:<name>` inputs and route them to
+        // `Session.activateSkill`. Passing both through keeps the two
+        // surfaces in lockstep (palette ↔ interceptable set) without
+        // a second `listSkills()` round trip.
+        const built = buildSkillSlashCommands(skills);
+        const skillCommands = built.commands.map((cmd) => ({
+          name: cmd.name,
+          description: cmd.description,
+        }));
+        return {
+          commands: [...builtinCommands, ...skillCommands],
+          skillCommandMap: built.commandMap,
+        };
+      };
       try {
         await runAcpServer(harness, {
           agentInfo: { name: 'Kimi Code CLI', version: getVersion() },
+          slashCommands: resolveSlashCommands,
           ...(terminalAuthEnv ? { terminalAuthEnv } : {}),
           ...(legacyCommand !== undefined && legacyCommand.length > 0
             ? { terminalAuthLegacyCommand: legacyCommand }

packages/acp-adapter/src/approval.ts

@@ -157,8 +157,16 @@ export function permissionResponseToApprovalResponse(
   }
   switch (optionId) {
     case APPROVE_ONCE_OPTION_ID:
+    // Legacy Python kimi-cli (< v0.9.0) used 'approve' as the
+    // allow-once optionId. Keep accepting it so custom ACP clients
+    // built against the old SDK are not silently rejected.
+    case 'approve':
       return { decision: 'approved' };
     case APPROVE_ALWAYS_OPTION_ID:
+    // Legacy Python kimi-cli (< v0.9.0) used 'approve_for_session' as
+    // the allow-always optionId. Same backward-compatibility rationale
+    // as the 'approve' branch above.
+    case 'approve_for_session':
       return { decision: 'approved', scope: 'session' };
     case REJECT_OPTION_ID:
       return { decision: 'rejected' };

packages/acp-adapter/src/index.ts

@@ -1,8 +1,9 @@
-export type { Implementation } from '@agentclientprotocol/sdk';
+export type { AvailableCommand, Implementation } from '@agentclientprotocol/sdk';
 export { CURRENT_VERSION, MIN_PROTOCOL_VERSION, negotiateVersion } from './version';
 export type { AcpVersionSpec } from './version';
 export { TERMINAL_AUTH_METHOD, buildTerminalAuthMethod } from './auth-methods';
 export { AcpServer, runAcpServer, runAcpServerWithStream } from './server';
+export type { SlashCommandsSnapshot } from './server';
 export { AcpSession } from './session';
 export {
   acpBlocksToPromptParts,

packages/acp-adapter/src/kaos-acp.ts

@@ -19,6 +19,7 @@
 import { Buffer } from 'node:buffer';
 
 import type { AgentSideConnection } from '@agentclientprotocol/sdk';
+import { RequestError } from '@agentclientprotocol/sdk';
 import {
   KaosError,
   type Environment,
@@ -130,33 +131,32 @@ export class AcpKaos implements Kaos {
   }
 
   /**
-   * Read up to `n` bytes from the file. Implemented as
-   * `readText → utf8 encode → slice` because ACP only exposes string
-   * content. Callers that store non-text data through this path
-   * (uncommon) will get re-encoded bytes — acceptable per `Kaos.readBytes`
-   * which already permits encoding-dependent return values.
+   * Binary reads bypass the ACP text RPC by design: `fs/readTextFile`
+   * returns a decoded string and would corrupt or reject non-UTF-8
+   * payloads (images, video, archives — anything `ReadMediaFile` may
+   * touch). The ACP bridge only owns the *text* surface; raw bytes
+   * stay on the local filesystem via `inner`.
    */
-  async readBytes(path: string, n?: number): Promise<Buffer> {
+  readBytes(path: string, n?: number): Promise<Buffer> {
+    return this.inner.readBytes(path, n);
+  }
+
+  /**
+   * Return a small UTF-8 header derived from the same ACP text source as
+   * `readText` / `readLines`, used only by text-read callers for sniffing.
+   * Keep `readBytes` local so binary callers such as ReadMediaFile stay safe.
+   */
+  async readTextPreview(path: string, n: number): Promise<Buffer> {
     const text = await this.readText(path);
-    const buf = Buffer.from(text, 'utf8');
-    return n !== undefined ? buf.subarray(0, n) : buf;
+    return Buffer.from(text.slice(0, n), 'utf8');
   }
 
   /**
-   * Yield lines from the file. Emulates Python `splitlines(keepends=False)`:
-   * splits on `\n`, drops the trailing empty token if the file ended with
-   * a newline, and yields nothing for an empty file. Matches
-   * {@link LocalKaos.readLines}'s observable output for the trailing-newline
-   * case (the local version yields `'line\n'` chunks; here we yield without
-   * the `\n` — see below).
-   *
-   * Note on divergence from `LocalKaos.readLines`: the local impl yields
-   * `'a\n'`, `'b\n'`, `'c'` while this impl yields `'a'`, `'b'`, `'c'`.
-   * The interface JSDoc says only "Yield lines from the file at `path`
-   * one by one" without pinning trailing-newline semantics, so both
-   * shapes satisfy it. Tools that depend on the trailing-newline (rare)
-   * should adapt. The Python reference's ACP backend does not implement
-   * `readLines` separately either.
+   * Yield lines from the file, each terminated by its `\n` (the final
+   * line has no terminator if the file did not end with `\n`). Matches
+   * {@link LocalKaos.readLines} so tools that depend on line terminators
+   * (e.g. {@link ReadTool}, which renders CRLF endings) behave identically
+   * whether the underlying Kaos is local or ACP-bridged.
    */
   async *readLines(
     path: string,
@@ -167,7 +167,7 @@ export class AcpKaos implements Kaos {
     let start = 0;
     for (let i = 0; i < text.length; i++) {
       if (text.charCodeAt(i) === 0x0a /* \n */) {
-        yield text.slice(start, i);
+        yield text.slice(start, i + 1);
         start = i + 1;
       }
     }
@@ -181,9 +181,11 @@ export class AcpKaos implements Kaos {
    * always UTF-8 string content. `mode: 'a'` (append) emulates with a
    * read-then-write fallback: ACP has no native append, and the
    * intended audience (unsaved-buffer scratchpads) rarely needs it.
-   * If the prior read fails (e.g. file missing), the write proceeds
-   * as if the existing content were empty — matching Python `open('a')`
-   * which also creates new files.
+   * If the prior read fails because the file does not exist, the write
+   * proceeds as if the existing content were empty — matching Python
+   * `open('a')` which creates new files. Any other read failure
+   * (permission, transport, internal) propagates so we never silently
+   * destroy existing content.
    *
    * Returns `data.length` (chars) to match {@link LocalKaos.writeText}'s
    * contract.
@@ -197,8 +199,8 @@ export class AcpKaos implements Kaos {
       let existing = '';
       try {
         existing = await this.readText(path);
-      } catch {
-        // ENOENT-style failure → treat as empty (mirrors Python open('a')).
+      } catch (err) {
+        if (!isNotFoundError(err)) throw err;
         existing = '';
       }
       await this.acpWrite(path, existing + data);
@@ -254,3 +256,27 @@ function wrapKaosError(prefix: string, cause: unknown): KaosError {
   (err as Error & { cause?: unknown }).cause = cause;
   return err;
 }
+
+/**
+ * Return true iff `err` is a structured "file does not exist" failure on
+ * the read side of an ACP append-mode write. We only trust the ACP SDK's
+ * `RequestError.resourceNotFound` code (`-32002`), optionally wrapped in a
+ * `KaosError` by `readText` above. Message substring matching is intentionally
+ * avoided: wrapper messages include the path, so a path or non-ENOENT failure
+ * mentioning "not found" could otherwise be misclassified and cause append
+ * mode to overwrite existing content.
+ */
+function isNotFoundError(err: unknown): boolean {
+  const visited = new Set<unknown>();
+  let cur: unknown = err;
+  while (cur !== undefined && cur !== null && !visited.has(cur)) {
+    visited.add(cur);
+    if (cur instanceof RequestError && cur.code === -32002) return true;
+    if (cur instanceof Error) {
+      cur = (cur as Error & { cause?: unknown }).cause;
+      continue;
+    }
+    break;
+  }
+  return false;
+}