* 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
This commit is contained in:
5
.changeset/2018-applysurface-empty-manifest-agents.md
Normal file
5
.changeset/2018-applysurface-empty-manifest-agents.md
Normal file
@@ -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)
|
||||
@@ -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 */ }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
67
tests/surface-empty-manifest-agents.test.cjs
Normal file
67
tests/surface-empty-manifest-agents.test.cjs
Normal file
@@ -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');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user