From c4161735dcd649c8548134beb08dfc4da0720a37 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Tue, 16 Jun 2026 21:55:55 -0400 Subject: [PATCH] fix(#1325): scope update backup detection (#1354) --- .changeset/1325-update-backup-scope.md | 6 ++ gsd-core/bin/gsd-tools.cjs | 30 ++++++++-- tests/bug-2942-detect-custom-skills.test.cjs | 46 +++++++++++---- tests/update-custom-backup.test.cjs | 60 +++++++++++++++----- 4 files changed, 112 insertions(+), 30 deletions(-) create mode 100644 .changeset/1325-update-backup-scope.md diff --git a/.changeset/1325-update-backup-scope.md b/.changeset/1325-update-backup-scope.md new file mode 100644 index 000000000..8cf877a9a --- /dev/null +++ b/.changeset/1325-update-backup-scope.md @@ -0,0 +1,6 @@ +--- +type: Fixed +pr: 1354 +--- + +**Update backups now ignore preserved shared skills and hooks** -- `/gsd-update` custom-file detection now mirrors installer cleanup scope for shared runtime roots, so non-`gsd-*` skills and hooks are not copied into backup folders unnecessarily. (#1325) diff --git a/gsd-core/bin/gsd-tools.cjs b/gsd-core/bin/gsd-tools.cjs index ccdb13168..82bef0877 100755 --- a/gsd-core/bin/gsd-tools.cjs +++ b/gsd-core/bin/gsd-tools.cjs @@ -1566,18 +1566,30 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand const manifestKeys = new Set(Object.keys(manifest.files || {})); - // GSD-managed directories to scan for user-added files. - // These are the directories the installer wipes on update. - const GSD_MANAGED_DIRS = [ + // GSD-managed directories to scan for user-added files. Whole-owned + // roots are wiped recursively; shared runtime roots are pruned by the + // same gsd-* top-level prefix used by install.js _removeGsdEntries. + const GSD_WHOLE_MANAGED_DIRS = [ 'gsd-core', - 'agents', path.join('commands', 'gsd'), + ]; + const GSD_PREFIX_MANAGED_DIRS = [ + 'agents', 'hooks', 'skills', ]; function collectCustomFiles(dir, baseDir, manifestKeys, out) { if (!fs.existsSync(dir)) return; + const stat = fs.statSync(dir); + if (stat.isFile()) { + const relPath = path.relative(baseDir, dir).replace(/\\/g, '/'); + if (!manifestKeys.has(relPath)) { + out.push(relPath); + } + return; + } + if (!stat.isDirectory()) return; for (const entry of fs.readdirSync(dir, { withFileTypes: true })) { const fullPath = path.join(dir, entry.name); if (entry.isDirectory()) { @@ -1593,11 +1605,19 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand } const customFiles = []; - for (const managedDir of GSD_MANAGED_DIRS) { + for (const managedDir of GSD_WHOLE_MANAGED_DIRS) { const absDir = path.join(resolvedConfigDir, managedDir); if (!fs.existsSync(absDir)) continue; collectCustomFiles(absDir, resolvedConfigDir, manifestKeys, customFiles); } + for (const managedDir of GSD_PREFIX_MANAGED_DIRS) { + const absDir = path.join(resolvedConfigDir, managedDir); + if (!fs.existsSync(absDir)) continue; + for (const entry of fs.readdirSync(absDir, { withFileTypes: true })) { + if (!entry.name.startsWith('gsd-')) continue; + collectCustomFiles(path.join(absDir, entry.name), resolvedConfigDir, manifestKeys, customFiles); + } + } const out = { custom_files: customFiles, diff --git a/tests/bug-2942-detect-custom-skills.test.cjs b/tests/bug-2942-detect-custom-skills.test.cjs index 250215ffd..5affcefe2 100644 --- a/tests/bug-2942-detect-custom-skills.test.cjs +++ b/tests/bug-2942-detect-custom-skills.test.cjs @@ -2,9 +2,9 @@ * GSD Tools Tests — detect-custom-files misses skills/ directory (#2942) * * After v1.39.0 skill consolidation (#2790), skills/ became a GSD-managed root. - * GSD_MANAGED_DIRS was missing 'skills', so user-added skill directories like - * skills/custom-skill/SKILL.md were never walked and got silently destroyed - * during /gsd-update. + * GSD_MANAGED_DIRS was missing 'skills', so user-added GSD-prefixed skill + * directories like skills/gsd-custom-skill/SKILL.md were never walked and got + * silently destroyed during /gsd-update. */ 'use strict'; @@ -62,14 +62,14 @@ describe('detect-custom-files — skills/ directory missing from GSD_MANAGED_DIR cleanup(tmpDir); }); - // Test 1: detects custom skill in skills//SKILL.md - test('detects custom skill file at skills//SKILL.md', () => { + // Test 1: detects custom GSD-prefixed skill in skills/gsd-/SKILL.md + test('detects custom skill file at skills/gsd-/SKILL.md', () => { writeManifest(tmpDir, { 'skills/gsd-planner/SKILL.md': '# GSD Planner Skill\n', }); - // User-added custom skill — NOT in manifest - writeCustomFile(tmpDir, 'skills/test-custom/SKILL.md', '# My Custom Skill\n'); + // User-added custom GSD-prefixed skill — NOT in manifest + writeCustomFile(tmpDir, 'skills/gsd-test-custom/SKILL.md', '# My Custom Skill\n'); const result = runGsdTools( ['detect-custom-files', '--config-dir', tmpDir], @@ -82,8 +82,30 @@ describe('detect-custom-files — skills/ directory missing from GSD_MANAGED_DIR assert.ok(Array.isArray(json.custom_files), 'custom_files should be an array'); assert.ok(json.custom_count >= 1, `custom_count should be >= 1, got ${json.custom_count}`); assert.ok( - json.custom_files.includes('skills/test-custom/SKILL.md'), - `skills/test-custom/SKILL.md should be in custom_files; got: ${JSON.stringify(json.custom_files)}` + json.custom_files.includes('skills/gsd-test-custom/SKILL.md'), + `skills/gsd-test-custom/SKILL.md should be in custom_files; got: ${JSON.stringify(json.custom_files)}` + ); + }); + + test('does not detect non-gsd shared skills preserved by installer (#1325)', () => { + writeManifest(tmpDir, { + 'skills/gsd-planner/SKILL.md': '# GSD Planner Skill\n', + }); + + writeCustomFile(tmpDir, 'skills/test-custom/SKILL.md', '# My Custom Skill\n'); + + const result = runGsdTools( + ['detect-custom-files', '--config-dir', tmpDir], + tmpDir + ); + + assert.ok(result.success, `Command failed: ${result.error}`); + + const json = JSON.parse(result.output); + assert.ok(Array.isArray(json.custom_files), 'custom_files should be an array'); + assert.ok( + !json.custom_files.includes('skills/test-custom/SKILL.md'), + `non-gsd shared skill should not be in custom_files; got: ${JSON.stringify(json.custom_files)}` ); }); @@ -134,13 +156,13 @@ describe('detect-custom-files — skills/ directory missing from GSD_MANAGED_DIR }); // Test 4: custom_count matches custom_files.length - test('custom_count matches custom_files.length when multiple custom skills exist', () => { + test('custom_count matches custom_files.length when multiple custom gsd-prefixed skills exist', () => { writeManifest(tmpDir, { 'skills/gsd-planner/SKILL.md': '# GSD Planner Skill\n', }); - writeCustomFile(tmpDir, 'skills/test-custom/SKILL.md', '# Custom Skill One\n'); - writeCustomFile(tmpDir, 'skills/another-custom/SKILL.md', '# Custom Skill Two\n'); + writeCustomFile(tmpDir, 'skills/gsd-test-custom/SKILL.md', '# Custom Skill One\n'); + writeCustomFile(tmpDir, 'skills/gsd-another-custom/SKILL.md', '# Custom Skill Two\n'); const result = runGsdTools( ['detect-custom-files', '--config-dir', tmpDir], diff --git a/tests/update-custom-backup.test.cjs b/tests/update-custom-backup.test.cjs index 5e6006c56..1854de2a9 100644 --- a/tests/update-custom-backup.test.cjs +++ b/tests/update-custom-backup.test.cjs @@ -85,13 +85,13 @@ describe('detect-custom-files — update workflow backup detection (#1997)', () ); }); - test('detects custom files added inside agents/', () => { + test('detects custom gsd-prefixed files added inside agents/', () => { writeManifest(tmpDir, { 'agents/gsd-executor.md': '# GSD Executor\n', }); - // Add a user's custom agent (not prefixed with gsd-) - const customAgent = path.join(tmpDir, 'agents/my-custom-agent.md'); + // Add a user's custom GSD-prefixed agent that the installer would prune. + const customAgent = path.join(tmpDir, 'agents/gsd-my-custom-agent.md'); fs.mkdirSync(path.dirname(customAgent), { recursive: true }); fs.writeFileSync(customAgent, '# My Custom Agent\n'); @@ -103,7 +103,7 @@ describe('detect-custom-files — update workflow backup detection (#1997)', () assert.ok(result.success, `Command failed: ${result.error}`); const json = JSON.parse(result.output); - assert.ok(json.custom_files.includes('agents/my-custom-agent.md'), + assert.ok(json.custom_files.includes('agents/gsd-my-custom-agent.md'), `custom agent should be detected; got: ${JSON.stringify(json.custom_files)}`); }); @@ -226,18 +226,16 @@ describe('detect-custom-files — update workflow backup detection (#1997)', () ); }); - // After v1.39.0 skill consolidation (#2790), the installer wipes skills/ on - // update. skills/ is now a GSD-managed directory and must be scanned so that - // user-added skill directories are backed up before the wipe (#2942). - // GSD-owned skills (tracked in manifest) must NOT be flagged as custom. - test('scans skills/ directory and detects user-added skills not in manifest (#2942)', () => { + // skills/ is prefix-selective: the installer prunes gsd-* entries, not every + // skill directory under the shared runtime skill root. + test('scans skills/ directory and detects custom gsd-prefixed skills not in manifest (#2942, #1325)', () => { writeManifest(tmpDir, { 'gsd-core/workflows/execute-phase.md': '# Execute Phase\n', 'skills/gsd-planner/SKILL.md': '# GSD Planner\n', }); - // Simulate user having a custom skill installed — NOT in manifest - const customSkillDir = path.join(tmpDir, 'skills', 'my-custom-skill'); + // Simulate user having a custom GSD-prefixed skill installed — NOT in manifest + const customSkillDir = path.join(tmpDir, 'skills', 'gsd-my-custom-skill'); fs.mkdirSync(customSkillDir, { recursive: true }); fs.writeFileSync(path.join(customSkillDir, 'SKILL.md'), '# My Custom Skill\n'); @@ -250,9 +248,9 @@ describe('detect-custom-files — update workflow backup detection (#1997)', () const json = JSON.parse(result.output); - // The user's custom skill should be detected + // The user's custom GSD-prefixed skill should be detected assert.ok( - json.custom_files.includes('skills/my-custom-skill/SKILL.md'), + json.custom_files.includes('skills/gsd-my-custom-skill/SKILL.md'), `custom skill should be detected; got: ${JSON.stringify(json.custom_files)}` ); @@ -263,6 +261,42 @@ describe('detect-custom-files — update workflow backup detection (#1997)', () ); }); + test('does not report non-gsd shared skills, hooks, or prior backups (#1325)', () => { + writeManifest(tmpDir, { + 'skills/gsd-planner/SKILL.md': '# GSD Planner\n', + 'hooks/gsd-check-update.js': 'console.log("managed");\n', + }); + + fs.mkdirSync(path.join(tmpDir, 'skills', 'gstack-one'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, 'skills', 'gstack-one', 'SKILL.md'), '# GStack\n'); + fs.mkdirSync(path.join(tmpDir, 'hooks', 'user'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, 'hooks', 'user', 'custom.js'), 'console.log("user");\n'); + fs.mkdirSync(path.join(tmpDir, 'gsd-user-files-backup', 'skills', 'gsd-old'), { recursive: true }); + fs.writeFileSync(path.join(tmpDir, 'gsd-user-files-backup', 'skills', 'gsd-old', 'SKILL.md'), '# Old backup\n'); + + const result = runGsdTools( + ['detect-custom-files', '--config-dir', tmpDir], + tmpDir + ); + + assert.ok(result.success, `Command failed: ${result.error}`); + + const json = JSON.parse(result.output); + assert.ok( + !json.custom_files.includes('skills/gstack-one/SKILL.md'), + `non-gsd skill should not be detected; got: ${JSON.stringify(json.custom_files)}` + ); + assert.ok( + !json.custom_files.includes('hooks/user/custom.js'), + `non-gsd hook should not be detected; got: ${JSON.stringify(json.custom_files)}` + ); + assert.strictEqual( + json.custom_files.filter(f => f.startsWith('gsd-user-files-backup/')).length, + 0, + `prior backups should not be detected; got: ${JSON.stringify(json.custom_files)}` + ); + }); + test('does not scan command/ directory (installer does not wipe it)', () => { writeManifest(tmpDir, { 'gsd-core/workflows/execute-phase.md': '# Execute Phase\n',