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 () => {