fix(3663): applySurface creates missing dest dirs (Codex P1-1)
Remove the fs.existsSync(dest) guard in applySurface so _syncGsdDir is
always called. _syncGsdDir already does mkdirSync(..., { recursive: true })
so the destination is created when absent — recovering partially-initialized
or user-deleted runtime config dirs. Also threads manifest through to
_syncGsdDir as optional 4th arg (used by P1-2 Hermes fix).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -208,9 +208,7 @@ function applySurface(runtimeConfigDir, layout, manifest, clusterMap) {
|
|||||||
for (const kind of layout.kinds) {
|
for (const kind of layout.kinds) {
|
||||||
const staged = kind.stage(resolved);
|
const staged = kind.stage(resolved);
|
||||||
const dest = path.join(layout.configDir, kind.destSubpath);
|
const dest = path.join(layout.configDir, kind.destSubpath);
|
||||||
if (fs.existsSync(dest)) {
|
_syncGsdDir(staged, dest, kind, manifest);
|
||||||
_syncGsdDir(staged, dest, kind);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
return resolved;
|
return resolved;
|
||||||
}
|
}
|
||||||
@@ -224,11 +222,16 @@ function applySurface(runtimeConfigDir, layout, manifest, clusterMap) {
|
|||||||
* by copying recursively; remove dirs not in staged set. Preserves dirs not matching
|
* by copying recursively; remove dirs not in staged set. Preserves dirs not matching
|
||||||
* the prefix (user-owned skills).
|
* the prefix (user-owned skills).
|
||||||
*
|
*
|
||||||
|
* For Hermes (empty prefix): uses manifest membership to discriminate GSD-owned vs
|
||||||
|
* user-owned dirs. GSD-owned = stem in manifest; removal targets = in manifest AND
|
||||||
|
* not in staged set. User-owned (not in manifest) are always preserved.
|
||||||
|
*
|
||||||
* @param {string} stagedDir source (staged temp dir or original)
|
* @param {string} stagedDir source (staged temp dir or original)
|
||||||
* @param {string} destDir runtime destination
|
* @param {string} destDir runtime destination
|
||||||
* @param {import('./runtime-artifact-layout.cjs').ArtifactKind|'commands'|'agents'} kind
|
* @param {import('./runtime-artifact-layout.cjs').ArtifactKind|'commands'|'agents'} kind
|
||||||
|
* @param {Map<string, string[]>} [manifest] optional; required for Hermes empty-prefix removal
|
||||||
*/
|
*/
|
||||||
function _syncGsdDir(stagedDir, destDir, kind) {
|
function _syncGsdDir(stagedDir, destDir, kind, manifest) {
|
||||||
if (!fs.existsSync(stagedDir)) return;
|
if (!fs.existsSync(stagedDir)) return;
|
||||||
fs.mkdirSync(destDir, { recursive: true });
|
fs.mkdirSync(destDir, { recursive: true });
|
||||||
|
|
||||||
@@ -256,22 +259,35 @@ function _syncGsdDir(stagedDir, destDir, kind) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Empty prefix = destSubpath is the GSD namespace (Hermes: skills/gsd/).
|
// Removal: discriminator depends on prefix shape.
|
||||||
// With no prefix filter, we cannot safely distinguish GSD-owned from user-owned dirs,
|
// Non-empty prefix: GSD namespace IS the prefix; remove prefix-matching dirs not in staged set.
|
||||||
// so we only remove dirs that match the prefix. When kindPrefix === '',
|
// Empty prefix (Hermes): GSD-owned = stem in manifest (i.e. canonically-shipped GSD skill).
|
||||||
// startsWith('') is always true but we must guard: skip removal entirely if prefix is
|
// User-owned skills not in manifest are preserved.
|
||||||
// empty so user dirs under skills/gsd/ are preserved (Hermes user-skill safety).
|
// No manifest available: be conservative, don't remove anything.
|
||||||
if (kindPrefix !== '') {
|
const canonicalStems = manifest
|
||||||
// Remove prefix-matched dirs in dest that are not in staged set
|
? new Set([...manifest.keys()].filter(k => !k.startsWith('_calls_agents_')))
|
||||||
const destEntries = fs.readdirSync(destDir);
|
: null;
|
||||||
for (const entry of destEntries) {
|
|
||||||
const entryPath = path.join(destDir, entry);
|
const destEntries = fs.readdirSync(destDir);
|
||||||
if (!fs.statSync(entryPath).isDirectory()) continue;
|
for (const entry of destEntries) {
|
||||||
if (!entry.startsWith(kindPrefix)) continue; // preserve user-owned dirs
|
const entryPath = path.join(destDir, entry);
|
||||||
if (!stagedDirs.has(entry)) {
|
if (!fs.statSync(entryPath).isDirectory()) continue;
|
||||||
try { fs.rmSync(entryPath, { recursive: true, force: true }); } catch {}
|
|
||||||
}
|
let isGsdOwned;
|
||||||
|
if (kindPrefix !== '') {
|
||||||
|
isGsdOwned = entry.startsWith(kindPrefix);
|
||||||
|
} else if (canonicalStems) {
|
||||||
|
// Hermes: empty prefix, destSubpath is the namespace.
|
||||||
|
// GSD-owned iff the directory name (stem) appears in the canonical manifest.
|
||||||
|
isGsdOwned = canonicalStems.has(entry);
|
||||||
|
} else {
|
||||||
|
// No manifest available: be conservative, don't remove anything.
|
||||||
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!isGsdOwned) continue; // preserve user-owned
|
||||||
|
if (stagedDirs.has(entry)) continue; // current GSD-owned, keep
|
||||||
|
try { fs.rmSync(entryPath, { recursive: true, force: true }); } catch {}
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// commands / agents kind: work with .md files
|
// commands / agents kind: work with .md files
|
||||||
|
|||||||
@@ -202,6 +202,31 @@ describe('applySurface', () => {
|
|||||||
assert.ok(fs.existsSync(foreignDir), 'my-custom-skill dir should be preserved');
|
assert.ok(fs.existsSync(foreignDir), 'my-custom-skill dir should be preserved');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('applySurface recreates missing destination directories', (t) => {
|
||||||
|
// Fixture: layout configDir exists but the dest subdirectory for the kinds does NOT.
|
||||||
|
const base = createTempDir('gsd-surface-missing-dest-');
|
||||||
|
t.after(() => cleanup(base));
|
||||||
|
const runtimeConfigDir = base;
|
||||||
|
// Do NOT pre-create commands/gsd or agents — they are intentionally absent.
|
||||||
|
writeActiveProfile(runtimeConfigDir, 'core');
|
||||||
|
writeSurface(runtimeConfigDir, {
|
||||||
|
baseProfile: 'core',
|
||||||
|
disabledClusters: [],
|
||||||
|
explicitAdds: [],
|
||||||
|
explicitRemoves: [],
|
||||||
|
});
|
||||||
|
const manifest = loadSkillsManifest(REAL_COMMANDS_DIR);
|
||||||
|
const layout = resolveRuntimeArtifactLayout('claude', runtimeConfigDir, 'local');
|
||||||
|
applySurface(runtimeConfigDir, layout, manifest, CLUSTERS);
|
||||||
|
|
||||||
|
// commands/gsd must have been created and populated
|
||||||
|
const commandsDir = path.join(runtimeConfigDir, 'commands', 'gsd');
|
||||||
|
assert.ok(fs.existsSync(commandsDir), 'commands/gsd dir should be created even if initially absent');
|
||||||
|
const files = fs.readdirSync(commandsDir).filter(f => f.endsWith('.md'));
|
||||||
|
assert.ok(files.length > 0, 'commands/gsd should contain staged skill files');
|
||||||
|
assert.ok(files.includes('help.md'), 'help.md should be present after applySurface on missing dest');
|
||||||
|
});
|
||||||
|
|
||||||
test('_syncGsdDir skills kind (hermes): preserves non-GSD user dir under skills/gsd/ when kindPrefix is empty', (t) => {
|
test('_syncGsdDir skills kind (hermes): preserves non-GSD user dir under skills/gsd/ when kindPrefix is empty', (t) => {
|
||||||
const { _syncGsdDir } = require('../get-shit-done/bin/lib/surface.cjs');
|
const { _syncGsdDir } = require('../get-shit-done/bin/lib/surface.cjs');
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user