From 829b603a258e02621c63f3ff36c3b706d54df098 Mon Sep 17 00:00:00 2001 From: mangeshraut712 Date: Fri, 31 Jul 2026 00:04:31 +0530 Subject: [PATCH 1/4] fix(agent-core): rename-swap managed plugins to avoid Windows EBUSY Replacing a managed plugin used to rm the live directory in place. On Windows that fails when the plugin's MCP child still holds that directory as cwd. Publish via rename-aside + staging swap and treat cleanup of the previous tree as best-effort. Fixes #2361 --- .changeset/plugin-managed-update-ebusy.md | 5 + packages/agent-core/src/plugin/manager.ts | 178 +++++++++++------- .../agent-core/test/plugin/manager.test.ts | 7 +- 3 files changed, 126 insertions(+), 64 deletions(-) create mode 100644 .changeset/plugin-managed-update-ebusy.md diff --git a/.changeset/plugin-managed-update-ebusy.md b/.changeset/plugin-managed-update-ebusy.md new file mode 100644 index 0000000000..c983892b3e --- /dev/null +++ b/.changeset/plugin-managed-update-ebusy.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix managed plugin updates failing with EBUSY on Windows by rename-swapping the live directory instead of deleting it in place. diff --git a/packages/agent-core/src/plugin/manager.ts b/packages/agent-core/src/plugin/manager.ts index b22efa71ef..86478b7e75 100644 --- a/packages/agent-core/src/plugin/manager.ts +++ b/packages/agent-core/src/plugin/manager.ts @@ -64,81 +64,102 @@ export class PluginManager { async install(source: string): Promise { const resolved = resolveInstallSource(source); - let normalizedRoot: string; + let managedCopy: ManagedPluginCopy | undefined; let originalSource: string; let sourceType: PluginSource; let parsed: ParsedManifestResult; let id: string; let github: PluginGithubMetadata | undefined; + let zipTmpDir: string | undefined; - if (resolved.kind === 'local-path') { - const sourceRoot = await normalizeInstallRoot(resolved.path); - originalSource = resolved.path; - sourceType = 'local-path'; - parsed = await parseManifest(sourceRoot); - if (parsed.manifest === undefined) { - const msg = parsed.diagnostics.find((d) => d.severity === 'error')?.message ?? 'no manifest'; - throw new Error(`Cannot install plugin at ${sourceRoot}: ${msg}`); - } - id = normalizePluginId(parsed.manifest.name); - normalizedRoot = await copyPluginToManagedRoot(this.kimiHomeDir, id, sourceRoot); - parsed = await parseManifest(normalizedRoot); - } else { - let zipUrl: string; - if (resolved.kind === 'github') { - const githubResolution = await resolveGithubSource(resolved); - zipUrl = githubResolution.tarballUrl; - originalSource = source.trim(); - sourceType = 'github'; - github = { - owner: resolved.owner, - repo: resolved.repo, - ref: githubResolution.ref, - }; - } else { - zipUrl = resolved.path; + try { + if (resolved.kind === 'local-path') { + const sourceRoot = await normalizeInstallRoot(resolved.path); originalSource = resolved.path; - sourceType = 'zip-url'; - } - const buffer = await downloadZip(zipUrl); - const tmpDir = await mkdtemp(path.join(tmpdir(), 'kimi-plugin-zip-')); - try { - const detectedRoot = await extractZip(buffer, tmpDir); + sourceType = 'local-path'; + parsed = await parseManifest(sourceRoot); + if (parsed.manifest === undefined) { + const msg = parsed.diagnostics.find((d) => d.severity === 'error')?.message ?? 'no manifest'; + throw new Error(`Cannot install plugin at ${sourceRoot}: ${msg}`); + } + id = normalizePluginId(parsed.manifest.name); + managedCopy = await copyPluginToManagedRoot(this.kimiHomeDir, id, sourceRoot); + parsed = await parseManifest(managedCopy.root); + } else { + let zipUrl: string; + if (resolved.kind === 'github') { + const githubResolution = await resolveGithubSource(resolved); + zipUrl = githubResolution.tarballUrl; + originalSource = source.trim(); + sourceType = 'github'; + github = { + owner: resolved.owner, + repo: resolved.repo, + ref: githubResolution.ref, + }; + } else { + zipUrl = resolved.path; + originalSource = resolved.path; + sourceType = 'zip-url'; + } + const buffer = await downloadZip(zipUrl); + zipTmpDir = await mkdtemp(path.join(tmpdir(), 'kimi-plugin-zip-')); + const detectedRoot = await extractZip(buffer, zipTmpDir); parsed = await parseManifest(detectedRoot); if (parsed.manifest === undefined) { const msg = parsed.diagnostics.find((d) => d.severity === 'error')?.message ?? 'no manifest'; throw new Error(`Cannot install plugin from ${originalSource}: ${msg}`); } id = normalizePluginId(parsed.manifest.name); - normalizedRoot = await copyPluginToManagedRoot(this.kimiHomeDir, id, detectedRoot); - parsed = await parseManifest(normalizedRoot); - } finally { - await rm(tmpDir, { recursive: true, force: true }); + managedCopy = await copyPluginToManagedRoot(this.kimiHomeDir, id, detectedRoot); + parsed = await parseManifest(managedCopy.root); } - } - if (parsed.manifest === undefined) { - const msg = parsed.diagnostics.find((d) => d.severity === 'error')?.message ?? 'no manifest'; - throw new Error(`Cannot install plugin at ${normalizedRoot}: ${msg}`); + if (parsed.manifest === undefined) { + const msg = parsed.diagnostics.find((d) => d.severity === 'error')?.message ?? 'no manifest'; + throw new Error(`Cannot install plugin at ${managedCopy.root}: ${msg}`); + } + id = normalizePluginId(parsed.manifest.name); + const existing = this.records.get(id); + const now = new Date().toISOString(); + const record = await recordFrom({ + id, + root: managedCopy.root, + enabled: existing?.enabled ?? true, + installedAt: existing?.installedAt ?? now, + updatedAt: now, + originalSource, + source: sourceType, + capabilities: existing?.capabilities, + github, + parsed, + }); + this.records.set(id, record); + await this.persist(); + await discardPreviousManagedRoot(managedCopy.previousRoot); + managedCopy = undefined; + return record; + } catch (error) { + if (managedCopy !== undefined) { + try { + await rm(managedCopy.root, { recursive: true, force: true }); + if (managedCopy.previousRoot !== undefined) { + await rename(managedCopy.previousRoot, managedCopy.root); + } + } catch (rollbackError) { + throw new AggregateError( + [error, rollbackError], + 'Plugin installation failed and the previous managed copy could not be restored', + { cause: error }, + ); + } + } + throw error; + } finally { + if (zipTmpDir !== undefined) { + await rm(zipTmpDir, { recursive: true, force: true }); + } } - id = normalizePluginId(parsed.manifest.name); - const existing = this.records.get(id); - const now = new Date().toISOString(); - const record = await recordFrom({ - id, - root: normalizedRoot, - enabled: existing?.enabled ?? true, - installedAt: existing?.installedAt ?? now, - updatedAt: now, - originalSource, - source: sourceType, - capabilities: existing?.capabilities, - github, - parsed, - }); - this.records.set(id, record); - await this.persist(); - return record; } async setEnabled(id: string, enabled: boolean): Promise { @@ -355,24 +376,55 @@ async function normalizeInstallRoot(rootPath: string): Promise { return resolved; } +interface ManagedPluginCopy { + readonly root: string; + readonly previousRoot?: string; +} + +/** + * Publish a plugin into the managed root without deleting the live directory + * in-place. On Windows, an MCP child whose cwd is the managed root holds the + * directory busy (`EBUSY` on `rmdir`); renaming it aside usually succeeds, and + * the previous tree can be deleted later (best-effort) once nothing holds it. + */ async function copyPluginToManagedRoot( kimiHomeDir: string, id: string, sourceRoot: string, -): Promise { +): Promise { const managedRoot = path.join(kimiHomeDir, 'plugins', 'managed', id); const managedDir = path.dirname(managedRoot); await mkdir(managedDir, { recursive: true }); const stagingRoot = await mkdtemp(path.join(managedDir, `${id}-`)); + const previousRoot = `${stagingRoot}-previous`; + let movedPreviousRoot = false; + let published = false; try { await cp(sourceRoot, stagingRoot, { recursive: true }); - await rm(managedRoot, { recursive: true, force: true }); + try { + await rename(managedRoot, previousRoot); + movedPreviousRoot = true; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') throw error; + } await rename(stagingRoot, managedRoot); + published = true; + return { + root: await realpath(managedRoot), + previousRoot: movedPreviousRoot ? previousRoot : undefined, + }; } catch (error) { - await rm(stagingRoot, { recursive: true, force: true }); + await rm(published ? managedRoot : stagingRoot, { recursive: true, force: true }); + if (movedPreviousRoot) await rename(previousRoot, managedRoot); throw error; } - return realpath(managedRoot); +} + +async function discardPreviousManagedRoot(previousRoot: string | undefined): Promise { + if (previousRoot === undefined) return; + // MCP children (or Windows AV) may still hold the old tree; never fail the + // install because deferred cleanup could not finish immediately. + await rm(previousRoot, { recursive: true, force: true }).catch(() => undefined); } async function recordFrom(input: { diff --git a/packages/agent-core/test/plugin/manager.test.ts b/packages/agent-core/test/plugin/manager.test.ts index ce8888c90f..882cf00da7 100644 --- a/packages/agent-core/test/plugin/manager.test.ts +++ b/packages/agent-core/test/plugin/manager.test.ts @@ -1,4 +1,4 @@ -import { mkdir, mkdtemp, realpath, symlink, writeFile } from 'node:fs/promises'; +import { mkdir, mkdtemp, readdir, realpath, symlink, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import path from 'node:path'; @@ -325,6 +325,11 @@ describe('PluginManager', () => { expect(updated.updatedAt).not.toBe(first.updatedAt); expect(updated.originalSource).toBe(updatedRoot); expect(manager.info('demo')?.mcpServers[0]?.enabled).toBe(false); + // Rename-swap publish must not leave sibling `*-previous` trees behind when + // the old managed root is free to delete (Windows EBUSY path defers this). + const managedDir = path.join(home, 'plugins', 'managed'); + const leftover = (await readdir(managedDir)).filter((name) => name.includes('-previous')); + expect(leftover).toEqual([]); }); it('keeps a plugin in error state instead of losing it on a broken manifest', async () => { From edfb72ca5fc76ce347ee0739e897e07b6a184770 Mon Sep 17 00:00:00 2001 From: mangeshraut712 Date: Sat, 1 Aug 2026 16:51:58 +0000 Subject: [PATCH 2/4] fix(agent-core): keep plugin records consistent if persist fails Only publish the in-memory plugin map after writeInstalled succeeds, so a failed update cannot leave records pointing at a rolled-back managed tree. Addresses Codex review on #2430. --- packages/agent-core/src/plugin/manager.ts | 14 ++++++++++++-- .../agent-core/test/plugin/manager.test.ts | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/packages/agent-core/src/plugin/manager.ts b/packages/agent-core/src/plugin/manager.ts index 86478b7e75..b231bd3852 100644 --- a/packages/agent-core/src/plugin/manager.ts +++ b/packages/agent-core/src/plugin/manager.ts @@ -134,8 +134,18 @@ export class PluginManager { github, parsed, }); - this.records.set(id, record); - await this.persist(); + // Persist before publishing the in-memory map: if writeInstalled fails we + // must not leave this.records pointing at a tree that the catch rolls back. + const previousRecords = this.records; + const next = new Map(this.records); + next.set(id, record); + this.records = next; + try { + await this.persist(); + } catch (error) { + this.records = previousRecords; + throw error; + } await discardPreviousManagedRoot(managedCopy.previousRoot); managedCopy = undefined; return record; diff --git a/packages/agent-core/test/plugin/manager.test.ts b/packages/agent-core/test/plugin/manager.test.ts index 882cf00da7..9a25989359 100644 --- a/packages/agent-core/test/plugin/manager.test.ts +++ b/packages/agent-core/test/plugin/manager.test.ts @@ -6,6 +6,7 @@ import { describe, expect, it, vi } from 'vitest'; import yazl from 'yazl'; import { PluginManager } from '../../src/plugin/manager'; +import * as pluginStore from '../../src/plugin/store'; async function makeKimiHome(): Promise { return mkdtemp(path.join(tmpdir(), 'kimi-home-')); @@ -332,6 +333,23 @@ describe('PluginManager', () => { expect(leftover).toEqual([]); }); + it('install() does not publish in-memory records when persistence fails', async () => { + const home = await makeKimiHome(); + const root = await makePlugin('demo', { version: '1.0.0' }); + const manager = new PluginManager({ kimiHomeDir: home }); + await manager.load(); + await manager.install(root); + expect(manager.get('demo')?.manifest?.version).toBe('1.0.0'); + + const updatedRoot = await makePlugin('demo', { version: '2.0.0' }); + const persist = vi.spyOn(pluginStore, 'writeInstalled').mockRejectedValueOnce(new Error('disk full')); + await expect(manager.install(updatedRoot)).rejects.toThrow(/disk full/); + persist.mockRestore(); + + expect(manager.get('demo')?.manifest?.version).toBe('1.0.0'); + expect(manager.list()).toHaveLength(1); + }); + it('keeps a plugin in error state instead of losing it on a broken manifest', async () => { const home = await makeKimiHome(); const root = await makePlugin('demo'); From 9c2bad1828c3bb17238dfbf0f4c3e4e5150938fe Mon Sep 17 00:00:00 2001 From: Mangesh Raut Date: Sat, 1 Aug 2026 17:08:33 +0000 Subject: [PATCH 3/4] fix(agent-core): publish plugin records only after persist succeeds Persist install metadata from a candidate map, then assign this.records, so a failed writeInstalled never exposes a rolled-back managed tree. --- packages/agent-core/src/plugin/manager.ts | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/packages/agent-core/src/plugin/manager.ts b/packages/agent-core/src/plugin/manager.ts index b231bd3852..89f2fac1b2 100644 --- a/packages/agent-core/src/plugin/manager.ts +++ b/packages/agent-core/src/plugin/manager.ts @@ -134,18 +134,12 @@ export class PluginManager { github, parsed, }); - // Persist before publishing the in-memory map: if writeInstalled fails we - // must not leave this.records pointing at a tree that the catch rolls back. - const previousRecords = this.records; + // Persist from a candidate map, then publish it. Never leave this.records + // pointing at a tree that the outer catch rolls back on write failure. const next = new Map(this.records); next.set(id, record); + await this.persist(next); this.records = next; - try { - await this.persist(); - } catch (error) { - this.records = previousRecords; - throw error; - } await discardPreviousManagedRoot(managedCopy.previousRoot); managedCopy = undefined; return record; @@ -337,8 +331,8 @@ export class PluginManager { return record === undefined ? undefined : recordToInfo(record); } - private async persist(): Promise { - const installed: InstalledRecord[] = [...this.records.values()].map((record) => ({ + private async persist(records: ReadonlyMap = this.records): Promise { + const installed: InstalledRecord[] = [...records.values()].map((record) => ({ id: record.id, root: record.root, source: record.source, From 523b048e37cd73fa6760f308ae746426fc7ae9cc Mon Sep 17 00:00:00 2001 From: Mangesh Raut Date: Sat, 1 Aug 2026 17:52:59 +0000 Subject: [PATCH 4/4] ci: retrigger checks