fix(resolve-ecc-root): require ECC skills, not just scripts, before accepting a root (#2544) (#2577)

* fix(resolve-ecc-root): require ECC skills, not just scripts, before accepting a root (#2544)

resolveEccRoot() accepted a candidate root on script-only evidence
(scripts/lib/utils.js). A partial install that lands ECC's scripts into
~/.claude but not ECC's skills short-circuited at the standard-install
branch, so skill-resolving callers built skills/... paths against a root
where they do not exist and every command failed three layers away.

For the default probe (skill consumers, reached via INLINE_RESOLVE) a
candidate now qualifies only if it contains both the script tree and a
sentinel ECC skill; the same stricter check guards the plugin-root and
plugin-cache branches. An explicit caller probe is still honored exactly,
so script consumers (e.g. session-start-bootstrap, which probes for the
hook runner) are unaffected. Merely checking that skills/ exists is
insufficient — a user's own ~/.claude/skills/ can be present with none of
ECC's skills.

Adds a regression test for the exact partial-install scenario and updates
the resolver test fixtures to build complete roots.

* test(resolve-ecc-root): cover partial exact-plugin and cache roots; DRY skill sentinel (#2544)

Address CodeRabbit review on PR #2577:
- Extend #2544 regression coverage to the exact-plugin and versioned
  plugin-cache branches, asserting the stricter both-sentinels predicate
  rejects a scripts-only root there too (not only for ~/.claude).
- Extract the ECC_SKILL_SENTINEL constant in command-plugin-root.test.js
  and reuse it at both fixture setup sites instead of duplicating the literal.
This commit is contained in:
Gaurav Dubey 2026-07-26 10:51:27 +05:30 committed by GitHub
parent ac30ff3ea2
commit c714dc5654
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 125 additions and 6 deletions

View file

@ -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 });

View file

@ -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', () => {