diff --git a/scripts/lib/resolve-ecc-root.js b/scripts/lib/resolve-ecc-root.js index 377268f2..396aac37 100644 --- a/scripts/lib/resolve-ecc-root.js +++ b/scripts/lib/resolve-ecc-root.js @@ -18,6 +18,17 @@ const PLUGIN_ROOT_SEGMENTS = [ ['marketplaces', LEGACY_PLUGIN_SLUG], ]; +// Artifacts that identify a COMPLETE ECC root when the caller gives no explicit +// probe. A real ECC root ships both the script tree AND ECC's skills; a partial +// install (scripts copied, skills not) must not qualify for skill-resolving +// callers, which build `skills/...` paths against the resolved root (#2544). +// Checking "skills/ exists" is not enough — a user's own ~/.claude/skills/ can +// be present with none of ECC's skills — so we probe for a sentinel skill that +// ships in every ECC root and is exactly what the failing skill commands need. +// If that skill is ever renamed, move this sentinel with it. +const DEFAULT_SCRIPT_PROBE = path.join('scripts', 'lib', 'utils.js'); +const DEFAULT_SKILL_PROBE = path.join('skills', 'continuous-learning-v2'); + /** * Resolve the ECC source root directory. * @@ -31,8 +42,14 @@ const PLUGIN_ROOT_SEGMENTS = [ * @param {object} [options] * @param {string} [options.homeDir] Override home directory (for testing) * @param {string} [options.envRoot] Override CLAUDE_PLUGIN_ROOT (for testing) - * @param {string} [options.probe] Relative path used to verify a candidate root - * contains ECC scripts. Default: 'scripts/lib/utils.js' + * @param {string} [options.probe] Relative path used to verify a candidate + * root contains what the caller needs. When + * given, it is honored exactly (script + * consumers pass their own script path). When + * omitted, a candidate must contain BOTH the + * ECC script tree and a sentinel ECC skill, + * so a partial install (scripts without + * skills) is rejected for skill consumers. * @returns {string} Resolved ECC root path */ function resolveEccRoot(options = {}) { @@ -46,10 +63,20 @@ function resolveEccRoot(options = {}) { const homeDir = options.homeDir || os.homedir(); const claudeDir = path.join(homeDir, '.claude'); - const probe = options.probe || path.join('scripts', 'lib', 'utils.js'); + + // Decide whether a candidate directory is a usable ECC root. An explicit + // caller probe is honored exactly (script consumers know the artifact they + // need). With the default probe the caller is a skill consumer, so a + // candidate must contain both ECC's scripts and a sentinel ECC skill — + // otherwise a scripts-only ~/.claude short-circuits and every skill path + // resolves to a location that does not exist (#2544). + const isRoot = options.probe + ? (dir) => fs.existsSync(path.join(dir, options.probe)) + : (dir) => fs.existsSync(path.join(dir, DEFAULT_SCRIPT_PROBE)) + && fs.existsSync(path.join(dir, DEFAULT_SKILL_PROBE)); // Standard install — files are copied directly into ~/.claude/ - if (fs.existsSync(path.join(claudeDir, probe))) { + if (isRoot(claudeDir)) { return claudeDir; } @@ -60,7 +87,7 @@ function resolveEccRoot(options = {}) { ); for (const candidate of legacyPluginRoots) { - if (fs.existsSync(path.join(candidate, probe))) { + if (isRoot(candidate)) { return candidate; } } @@ -86,7 +113,7 @@ function resolveEccRoot(options = {}) { for (const verEntry of versionDirs) { if (!verEntry.isDirectory()) continue; const candidate = path.join(orgPath, verEntry.name); - if (fs.existsSync(path.join(candidate, probe))) { + if (isRoot(candidate)) { return candidate; } } diff --git a/tests/lib/command-plugin-root.test.js b/tests/lib/command-plugin-root.test.js index 688e399d..9630d087 100644 --- a/tests/lib/command-plugin-root.test.js +++ b/tests/lib/command-plugin-root.test.js @@ -6,6 +6,10 @@ const os = require('os'); const assert = require('assert'); const { INLINE_RESOLVE } = require('../../scripts/lib/resolve-ecc-root'); +// Sentinel ECC skill that resolveEccRoot() requires alongside the script tree +// before accepting a root; kept in sync with the module's DEFAULT_SKILL_PROBE. +const ECC_SKILL_SENTINEL = path.join('skills', 'continuous-learning-v2'); + let passed = 0; let failed = 0; @@ -55,6 +59,7 @@ test('resolveEccRoot module covers current and legacy marketplace plugin roots', const legacyRoot = path.join(legacyHomeDir, '.claude', 'plugins', 'marketplaces', 'ecc'); fs.mkdirSync(path.join(legacyRoot, 'scripts', 'lib'), { recursive: true }); fs.writeFileSync(path.join(legacyRoot, 'scripts', 'lib', 'utils.js'), '// stub'); + fs.mkdirSync(path.join(legacyRoot, ECC_SKILL_SENTINEL), { recursive: true }); assert.strictEqual(resolveEccRoot({ envRoot: '', homeDir: legacyHomeDir }), legacyRoot); } finally { fs.rmSync(legacyHomeDir, { recursive: true, force: true }); @@ -65,6 +70,7 @@ test('resolveEccRoot module covers current and legacy marketplace plugin roots', const cacheRoot = path.join(cacheHomeDir, '.claude', 'plugins', 'cache', 'ecc', 'affaan-m', '1.0.0'); fs.mkdirSync(path.join(cacheRoot, 'scripts', 'lib'), { recursive: true }); fs.writeFileSync(path.join(cacheRoot, 'scripts', 'lib', 'utils.js'), '// stub'); + fs.mkdirSync(path.join(cacheRoot, ECC_SKILL_SENTINEL), { recursive: true }); assert.strictEqual(resolveEccRoot({ envRoot: '', homeDir: cacheHomeDir }), cacheRoot); } finally { fs.rmSync(cacheHomeDir, { recursive: true, force: true }); diff --git a/tests/lib/resolve-ecc-root.test.js b/tests/lib/resolve-ecc-root.test.js index 765fd01f..36a8c0c1 100644 --- a/tests/lib/resolve-ecc-root.test.js +++ b/tests/lib/resolve-ecc-root.test.js @@ -19,6 +19,11 @@ const CURRENT_PACKAGE_VERSION = JSON.parse( const { resolveEccRoot, INLINE_RESOLVE } = require('../../scripts/lib/resolve-ecc-root'); +// Sentinel ECC skill that resolveEccRoot() requires (alongside the script tree) +// before accepting a root for skill consumers. Kept in sync with the module's +// DEFAULT_SKILL_PROBE; the #2544 regression test guards the behaviour. +const ECC_SKILL_SENTINEL = path.join('skills', 'continuous-learning-v2'); + function test(name, fn) { try { fn(); @@ -40,6 +45,7 @@ function setupStandardInstall(homeDir) { const scriptDir = path.join(claudeDir, 'scripts', 'lib'); fs.mkdirSync(scriptDir, { recursive: true }); fs.writeFileSync(path.join(scriptDir, 'utils.js'), '// stub'); + fs.mkdirSync(path.join(claudeDir, ECC_SKILL_SENTINEL), { recursive: true }); return claudeDir; } @@ -48,6 +54,7 @@ function setupLegacyPluginInstall(homeDir, segments) { const scriptDir = path.join(legacyDir, 'scripts', 'lib'); fs.mkdirSync(scriptDir, { recursive: true }); fs.writeFileSync(path.join(scriptDir, 'utils.js'), '// stub'); + fs.mkdirSync(path.join(legacyDir, ECC_SKILL_SENTINEL), { recursive: true }); return legacyDir; } function setupPluginCache(homeDir, pluginSlug, orgName, version) { @@ -58,6 +65,7 @@ function setupPluginCache(homeDir, pluginSlug, orgName, version) { const scriptDir = path.join(cacheDir, 'scripts', 'lib'); fs.mkdirSync(scriptDir, { recursive: true }); fs.writeFileSync(path.join(scriptDir, 'utils.js'), '// stub'); + fs.mkdirSync(path.join(cacheDir, ECC_SKILL_SENTINEL), { recursive: true }); return cacheDir; } @@ -277,6 +285,84 @@ function runTests() { } })) passed++; else failed++; + // ─── Partial install (#2544) ─── + + if (test('rejects a partial ~/.claude (scripts + non-ECC skills) and prefers a complete root (#2544)', () => { + const homeDir = createTempDir(); + try { + // ~/.claude has ECC's scripts and a user's OWN skills/ dir, but not ECC's + // skills. The old script-only probe accepted it and every skill path 404'd. + const claudeDir = path.join(homeDir, '.claude'); + const scriptDir = path.join(claudeDir, 'scripts', 'lib'); + fs.mkdirSync(scriptDir, { recursive: true }); + fs.writeFileSync(path.join(scriptDir, 'utils.js'), '// stub'); + fs.mkdirSync(path.join(claudeDir, 'skills', 'my-own-skill'), { recursive: true }); + // A COMPLETE ECC root exists in the plugin cache (scripts + ECC skill). + const expected = setupPluginCache(homeDir, 'ecc', 'affaan-m', CURRENT_PACKAGE_VERSION); + const result = resolveEccRoot({ envRoot: '', homeDir }); + assert.strictEqual(result, expected, + 'a scripts-only ~/.claude must not shadow a complete plugin-cache root'); + } finally { + fs.rmSync(homeDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('a scripts-only ~/.claude with no complete root falls back to ~/.claude (#2544)', () => { + const homeDir = createTempDir(); + try { + // No complete root anywhere: the resolver still returns ~/.claude as a + // last resort (unchanged fallback), so callers fail loudly at the missing + // path rather than the resolver inventing one. + const claudeDir = path.join(homeDir, '.claude'); + const scriptDir = path.join(claudeDir, 'scripts', 'lib'); + fs.mkdirSync(scriptDir, { recursive: true }); + fs.writeFileSync(path.join(scriptDir, 'utils.js'), '// stub'); + const result = resolveEccRoot({ envRoot: '', homeDir }); + assert.strictEqual(result, claudeDir); + } finally { + fs.rmSync(homeDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('rejects a partial exact plugin root (scripts, no ECC skill) and prefers a complete root (#2544)', () => { + const homeDir = createTempDir(); + try { + // An exact plugin root under ~/.claude/plugins/ecc ships ECC's scripts but + // not ECC's skills. The stricter predicate must reject it on the + // exact-plugin branch too, not only for ~/.claude. + const partialScripts = path.join(homeDir, '.claude', 'plugins', 'ecc', 'scripts', 'lib'); + fs.mkdirSync(partialScripts, { recursive: true }); + fs.writeFileSync(path.join(partialScripts, 'utils.js'), '// stub'); + // A COMPLETE ECC root exists in the plugin cache (scripts + ECC skill). + const expected = setupPluginCache(homeDir, 'ecc', 'affaan-m', CURRENT_PACKAGE_VERSION); + const result = resolveEccRoot({ envRoot: '', homeDir }); + assert.strictEqual(result, expected, + 'a scripts-only exact plugin root must not shadow a complete plugin-cache root'); + } finally { + fs.rmSync(homeDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + + if (test('rejects a partial plugin-cache root (scripts, no ECC skill) and falls back to ~/.claude (#2544)', () => { + const homeDir = createTempDir(); + try { + // A versioned plugin-cache root ships ECC's scripts but not ECC's skills. + // The stricter predicate must reject it on the cache branch, so the + // resolver returns the last-resort ~/.claude rather than the partial root. + const cacheScripts = path.join( + homeDir, '.claude', 'plugins', 'cache', 'ecc', 'affaan-m', CURRENT_PACKAGE_VERSION, + 'scripts', 'lib' + ); + fs.mkdirSync(cacheScripts, { recursive: true }); + fs.writeFileSync(path.join(cacheScripts, 'utils.js'), '// stub'); + const result = resolveEccRoot({ envRoot: '', homeDir }); + assert.strictEqual(result, path.join(homeDir, '.claude'), + 'a scripts-only plugin-cache root must not be returned; fall back to ~/.claude'); + } finally { + fs.rmSync(homeDir, { recursive: true, force: true }); + } + })) passed++; else failed++; + // ─── INLINE_RESOLVE ─── if (test('INLINE_RESOLVE is a non-empty string', () => {