From 460956bfed079413bce8f791d5610dfb7e5fd8b1 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Sun, 5 Jul 2026 17:28:34 -0400 Subject: [PATCH] fix(#2018): applySurface empty manifest no longer deletes gsd-* agents (#2031) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#2018): applySurface empty manifest no longer deletes gsd-* agents The agent-prune loop in _syncGsdDir deleted any gsd-*.md not in the staged set. When the manifest resolved empty (null/non-object, no entries, no files key, or unresolvable install source root), the staged set was empty → every gsd-* agent was deleted. Skills were guarded by pruneSkillDirs' manifest- membership check; agents had no equivalent. - src/surface.cts: skip the agent-prune loop when the manifest is empty/absent (copy still runs — genuinely new agents are added). - tests/surface-empty-manifest-agents.test.cjs: boundary matrix — empty manifest preserves agents (Map() + undefined); non-empty manifest + empty staged still prunes (boundary); empty manifest + new staged copies new + preserves existing. Closes #2018 * docs(#2018): backfill changeset pr 2031 --- ...2018-applysurface-empty-manifest-agents.md | 5 ++ src/surface.cts | 19 ++++-- tests/surface-empty-manifest-agents.test.cjs | 67 +++++++++++++++++++ 3 files changed, 86 insertions(+), 5 deletions(-) create mode 100644 .changeset/2018-applysurface-empty-manifest-agents.md create mode 100644 tests/surface-empty-manifest-agents.test.cjs diff --git a/.changeset/2018-applysurface-empty-manifest-agents.md b/.changeset/2018-applysurface-empty-manifest-agents.md new file mode 100644 index 000000000..68ddf4024 --- /dev/null +++ b/.changeset/2018-applysurface-empty-manifest-agents.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 2031 +--- +**`applySurface` no longer deletes every `gsd-*` agent when the skills manifest resolves empty** — the agent-prune loop in `_syncGsdDir` deleted any `gsd-*.md` not in the staged set, and when the manifest was empty/unresolvable (null manifest, no array entries, no `files` key, or an unresolvable install source root), the staged set was empty → every agent was pruned. Skills were guarded by `pruneSkillDirs`'s manifest-membership check (conservative preservation on empty manifest); agents had no equivalent. The agent-prune loop is now skipped when the manifest is empty/absent, so agents are preserved while copy (adding genuinely new agents) still runs. (#2018) diff --git a/src/surface.cts b/src/surface.cts index 120944c6f..caa4bcc34 100644 --- a/src/surface.cts +++ b/src/surface.cts @@ -509,11 +509,20 @@ function _syncGsdDir(stagedDir: string, destDir: string, kind: ArtifactKind | st // - agents: only gsd-* are GSD-owned // - flat command dirs: only `${kindPrefix}`-prefixed are GSD-owned // - namespaced command dirs: the whole dir is GSD-owned - for (const file of fs.readdirSync(destDir).filter(f => f.endsWith('.md'))) { - if (kindName === 'agents' && !file.startsWith('gsd-')) continue; - if (kindName === 'commands' && !namespacedByDir && kindPrefix && !file.startsWith(kindPrefix)) continue; - if (!stagedDestNames.has(file)) { - try { fs.unlinkSync(path.join(destDir, file)); } catch { /* ignore */ } + // + // Manifest gate (#2018): when the manifest is empty/absent (e.g. an unresolvable + // install source root yields an empty staged dir), the staged set is untrustworthy. + // Skills are guarded by pruneSkillDirs' manifest-membership check; agents must be + // guarded here — skip the prune loop entirely so an empty manifest never deletes + // every gsd-* agent. Copying (above) still runs so genuinely new agents are added. + const shouldPruneAgents = !(kindName === 'agents' && (!manifest || manifest.size === 0)); + if (shouldPruneAgents) { + for (const file of fs.readdirSync(destDir).filter(f => f.endsWith('.md'))) { + if (kindName === 'agents' && !file.startsWith('gsd-')) continue; + if (kindName === 'commands' && !namespacedByDir && kindPrefix && !file.startsWith(kindPrefix)) continue; + if (!stagedDestNames.has(file)) { + try { fs.unlinkSync(path.join(destDir, file)); } catch { /* ignore */ } + } } } } diff --git a/tests/surface-empty-manifest-agents.test.cjs b/tests/surface-empty-manifest-agents.test.cjs new file mode 100644 index 000000000..ba884511b --- /dev/null +++ b/tests/surface-empty-manifest-agents.test.cjs @@ -0,0 +1,67 @@ +'use strict'; + +// Regression guard for #2018 — applySurface over an empty/unresolvable manifest +// silently deleted every gsd-* agent because the agent-prune loop in _syncGsdDir +// didn't check manifest membership (unlike pruneSkillDirs which conservatively +// preserves everything when the manifest is empty). The fix skips the agent-prune +// loop when the manifest is empty/absent so agents are never bulk-deleted. + +const { describe, test, beforeEach, afterEach } = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); + +const { _syncGsdDir } = require('../gsd-core/bin/lib/surface.cjs'); + +function tmp() { return fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-surface-empty-')); } + +describe('#2018 — empty manifest must not delete gsd-* agents', () => { + let dest, staged; + beforeEach(() => { + dest = tmp(); staged = tmp(); + // Simulate existing agents in dest (what a real install has) + for (const name of ['gsd-executor.md', 'gsd-planner.md', 'gsd-verifier.md']) { + fs.writeFileSync(path.join(dest, name), `# ${name}\n`); + } + // Staged dir is EMPTY (unresolvable manifest → nothing staged) + }); + afterEach(() => { try { fs.rmSync(dest, { recursive: true, force: true }); } catch {} try { fs.rmSync(staged, { recursive: true, force: true }); } catch {} }); + + test('empty manifest → agents preserved (not deleted)', () => { + _syncGsdDir(staged, dest, 'agents', new Map()); + const remaining = fs.readdirSync(dest).filter((f) => f.startsWith('gsd-')); + assert.deepEqual(remaining.sort(), ['gsd-executor.md', 'gsd-planner.md', 'gsd-verifier.md'], + 'all gsd-* agents must survive an empty manifest'); + }); + + test('undefined manifest → agents preserved', () => { + _syncGsdDir(staged, dest, 'agents', undefined); + const remaining = fs.readdirSync(dest).filter((f) => f.startsWith('gsd-')); + assert.ok(remaining.length === 3, 'all agents preserved with undefined manifest'); + }); + + test('non-empty manifest + empty staged → agents still preserved (staged set empty but manifest exists)', () => { + // A populated manifest (even with a different set) is trusted; but staged is empty. + // The manifest gate only fires on EMPTY manifest. With a non-empty manifest but + // empty staged, the prune loop SHOULD run (agents not in staged get pruned). + // This test documents the boundary: non-empty manifest → prune happens. + const manifest = new Map([['gsd-old-agent', []]]); + _syncGsdDir(staged, dest, 'agents', manifest); + const remaining = fs.readdirSync(dest).filter((f) => f.startsWith('gsd-')); + // With a non-empty manifest, staged is empty → all agents pruned (this is the + // existing behavior for a genuine "all agents were un-surfaced" scenario, NOT + // the empty-manifest bug). + assert.deepEqual(remaining, [], 'non-empty manifest + empty staged → agents pruned (expected behavior)'); + }); + + test('empty manifest but new agents staged → new agents added, existing preserved', () => { + // Stage a new agent + fs.writeFileSync(path.join(staged, 'gsd-new-agent.md'), '# New\n'); + _syncGsdDir(staged, dest, 'agents', new Map()); + const remaining = fs.readdirSync(dest).filter((f) => f.startsWith('gsd-')); + assert.ok(remaining.includes('gsd-new-agent.md'), 'new staged agent must be copied'); + assert.ok(remaining.includes('gsd-executor.md'), 'existing agent must be preserved'); + assert.ok(remaining.length === 4, '3 existing + 1 new = 4 agents'); + }); +});