From e9dabdc649ff2a5dafaa43df5d730aba12ca60cc Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 5 Apr 2026 07:44:51 -0400 Subject: [PATCH] fix(resource-sync): prune removed bundled subdirectory extensions on upgrade (#1972) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(resource-sync): prune removed bundled subdirectory extensions on upgrade The managed-resources manifest and pruning system only tracked root-level files, not subdirectory extensions. When a bundled subdirectory extension like mcporter/ was removed from the bundle in a newer GSD version, the previously-synced copy in ~/.gsd/agent/extensions/ persisted indefinitely, causing tool name conflicts with its replacement (mcp-client/). - Add installedExtensionDirs to the manifest alongside installedExtensionRootFiles, recording directory names present in the bundled extensions dir at sync time. - In pruneRemovedBundledExtensions, diff previous installedExtensionDirs against current bundled dirs and rmSync({ recursive: true }) any that were removed. - Add mcporter to the hardcoded stale-entry list for pre-manifest upgrades. - Fix extension conflict error prefix: also match "conflicts with" (not just "supersedes") so extension-vs-extension conflicts are classified as warnings rather than hard errors. Fixes #1955 Co-Authored-By: Claude Opus 4.6 (1M context) * fix(resource-loader): repair mangled lines from conflict resolution The Python regex used to resolve cherry-pick conflicts stripped trailing newlines, causing declarations and comments to merge onto the same line. Replace the file with the upstream/main version which contains all the installedExtensionDirs logic correctly formatted. Co-Authored-By: Claude Sonnet 4.6 * fix(resource-loader): sweep all installed extension dirs not in current bundle The manifest-based pruner only removed dirs it had previously recorded. Extensions installed by pre-manifest versions (or manually) were never tracked, so they survived upgrades. Add a sweep of the actual installed extensions directory that removes any subdirectory absent from the current bundle, regardless of manifest history. Fixes the mcporter stale-dir regression test (#1972). Co-Authored-By: Claude Sonnet 4.6 * fix: check external-state DB path before symlink-resolved handler (#2952) The external-state handler added in c609d813 was placed after the generic symlink-resolved handler, which matches the same /.gsd/projects//worktrees/ pattern and short-circuits to the wrong result. Move the external-state check (which uses the more specific hex-hash regex) first so it takes precedence. Fixes shared-wal test: external-state worktree path resolves to project state DB. Co-Authored-By: Claude Sonnet 4.6 * test: update db-path-worktree-symlink expectations for external-state (#2952) /.gsd/projects//worktrees/ paths now resolve to /gsd.db after the external-state handler from #2952 was placed before the symlink-resolved handler. On POSIX, getcwd() returns canonical paths so /.gsd/projects//worktrees/ would in practice appear as ~/.gsd/projects//worktrees/ after OS symlink resolution — both correctly handled by the external-state behavior. Co-Authored-By: Claude Sonnet 4.6 --------- Co-authored-by: Claude Opus 4.6 (1M context) Co-authored-by: trek-e --- src/cli.ts | 8 +++--- src/resource-loader.ts | 10 +++++++ src/tests/resource-loader.test.ts | 46 +++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/cli.ts b/src/cli.ts index d4f59cb44..ac25bb18c 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -470,8 +470,8 @@ if (isPrintMode) { if (extensionsResult.errors.length > 0) { for (const err of extensionsResult.errors) { // Downgrade conflicts with built-in tools to warnings (#1347) - const isSuperseded = err.error.includes("supersedes"); - const prefix = isSuperseded ? "Extension conflict" : "Extension load error"; + const isConflict = err.error.includes("supersedes") || err.error.includes("conflicts with"); + const prefix = isConflict ? "Extension conflict" : "Extension load error"; process.stderr.write(`[gsd] ${prefix}: ${err.error}\n`) } } @@ -622,8 +622,8 @@ validateConfiguredModel(modelRegistry, settingsManager) if (extensionsResult.errors.length > 0) { for (const err of extensionsResult.errors) { - const isSuperseded = err.error.includes("supersedes"); - const prefix = isSuperseded ? "Extension conflict" : "Extension load error"; + const isConflict = err.error.includes("supersedes") || err.error.includes("conflicts with"); + const prefix = isConflict ? "Extension conflict" : "Extension load error"; process.stderr.write(`[gsd] ${prefix}: ${err.error}\n`) } } diff --git a/src/resource-loader.ts b/src/resource-loader.ts index 041d53143..30ad9c21d 100644 --- a/src/resource-loader.ts +++ b/src/resource-loader.ts @@ -373,6 +373,16 @@ function pruneRemovedBundledExtensions( } } + // Sweep-based: also remove any installed extension subdirectory not in the current bundle, + // even if it was never tracked in the manifest (e.g. installed by a pre-manifest version). + try { + if (existsSync(extensionsDir)) { + for (const e of readdirSync(extensionsDir, { withFileTypes: true })) { + if (e.isDirectory()) removeDirIfStale(e.name) + } + } + } catch { /* non-fatal */ } + // Always remove known stale files regardless of manifest state. // These were installed by pre-manifest versions so they may not appear in // installedExtensionRootFiles even when a manifest exists. diff --git a/src/tests/resource-loader.test.ts b/src/tests/resource-loader.test.ts index 637b9088a..03bbd5db2 100644 --- a/src/tests/resource-loader.test.ts +++ b/src/tests/resource-loader.test.ts @@ -160,3 +160,49 @@ test("initResources prunes stale top-level extension siblings next to bundled co assert.equal(existsSync(staleSiblingPath), false, "stale top-level sibling should be removed during sync"); assert.equal(existsSync(bundledPath), true, "bundled extension should remain after cleanup"); }); + +test("pruneRemovedBundledExtensions removes stale subdirectory extensions not in current bundle", async () => { + const { initResources } = await import("../resource-loader.ts"); + const tmp = mkdtempSync(join(tmpdir(), "gsd-resource-loader-prune-dirs-")); + const fakeAgentDir = join(tmp, "agent"); + + try { + // First sync — seeds the agent dir and writes the manifest. + initResources(fakeAgentDir); + + // Simulate a stale subdirectory extension left from a previous GSD version. + // This mirrors the mcporter scenario: it was bundled before, synced to + // ~/.gsd/agent/extensions/, then removed from the bundle in a newer version. + const staleExtDir = join(fakeAgentDir, "extensions", "mcporter"); + mkdirSync(staleExtDir, { recursive: true }); + writeFileSync(join(staleExtDir, "index.ts"), 'export default { name: "mcporter" };\n'); + assert.equal(existsSync(staleExtDir), true, "stale subdir extension should exist before prune"); + + // Read the manifest to verify subdirectory extensions are tracked. + const manifestPath = join(fakeAgentDir, "managed-resources.json"); + const manifest = JSON.parse(readFileSync(manifestPath, "utf-8")); + + // The manifest must record installed extension directories so the pruner + // can detect when one has been removed from the bundle. + assert.ok( + Array.isArray(manifest.installedExtensionDirs), + "manifest should contain installedExtensionDirs array", + ); + + // Bump the manifest version to force a re-sync (simulates upgrading GSD). + manifest.gsdVersion = "0.0.0-force-resync"; + manifest.contentHash = "0000000000000000"; + writeFileSync(manifestPath, JSON.stringify(manifest)); + + // Second sync — the bundle no longer contains mcporter/, so it must be pruned. + initResources(fakeAgentDir); + + assert.equal( + existsSync(staleExtDir), + false, + "stale subdirectory extension (mcporter/) should be pruned after upgrade", + ); + } finally { + rmSync(tmp, { recursive: true, force: true }); + } +});