skills/pbakaus_impeccable
1
0
Fork 0
mirror of https://github.com/pbakaus/impeccable.git synced 2026-09-12 06:06:37 +03:00
Code Issues Packages Projects Releases Wiki Activity

Fix: install missing explicitly selected providers without --force (#536)

Browse Source 
* Fix: install missing explicitly selected providers without --force (#500)

An explicit --providers list now treats "already installed" per selected
target: providers with an existing install take the update path, providers
with none get a fresh install (skills + hooks) in the same run. Previously
any existing install (e.g. .claude) made `install --providers=grok` exit 0
without writing .grok, leaving Grok Build on the Claude-variant fallback.

Written with AI assistance (Cursor agent), reviewed and tested by maintainer.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Fix: copy provider agents for freshly installed mixed-install targets

Bugbot caught that the mixed explicit-providers path installed skills and
hooks for missing targets but skipped copyProviderAgents, which both the
update and fresh-install paths run. Written with AI assistance (Cursor agent).

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Abdul Wahab
2026-08-08 18:33:23 -07:00
committed by GitHub
co-authored by Cursor
parent 5d10bc842c
commit 477484aaee
2 changed files with 78 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cli/bin/commands/skills.mjs
+27 -9
View File
@@ -1100,13 +1100,13 @@ async function chooseInstallScope(projectRoot, targets, detections, { yes, scope
async function chooseInstallPlan(projectRoot, flags, { yes } = {}) {
const providersValue = getFlagValue(flags, '--providers');
const scopeValue = getInstallScopeValue(flags);
const { targets, detections } = await chooseInstallProviders(projectRoot, providersValue, { yes });
const { targets, detections, explicit } = await chooseInstallProviders(projectRoot, providersValue, { yes });
if (targets.length === 0) {
throw new Error('Could not determine a target harness folder.');
}
const scope = await chooseInstallScope(projectRoot, targets, detections, { yes, scopeValue });
const installRoot = installRootForScope(scope, projectRoot);
return { targets, scope, installRoot, hookRoot: projectRoot, detections };
return { targets, scope, installRoot, hookRoot: projectRoot, detections, explicit };
}
/**
@@ -1812,16 +1812,23 @@ async function install(flags) {
process.exit(1);
}
const { targets, installRoot, hookRoot, scope } = plan;
const { targets, installRoot, hookRoot, scope, explicit } = plan;
const existing = isAlreadyInstalled(installRoot, scope);
const installedTargets = existing ? findInstalledProviders(installRoot, scope) : [];
// An explicit --providers list is a per-target request: a selected provider
// with no install yet gets a fresh install instead of tripping the global
// "already installed" early exit (issue #500). When every selected provider
// is missing, skip the update branch entirely and take the fresh-install path.
const missingSelectedTargets = (existing && !force && explicit)
? targets.filter(provider => !installedTargets.includes(provider))
: [];
if (existing && !force) {
if (existing && !force && missingSelectedTargets.length < targets.length) {
console.log(`Impeccable skills are already installed (found in ${existing}/).`);
const installedTargets = findInstalledProviders(installRoot, scope);
const selectedInstalledTargets = targets.filter(provider => installedTargets.includes(provider));
const linkedTargets = findLinkedProviders(installRoot, selectedInstalledTargets, scope);
const copyTargets = selectedInstalledTargets.filter(provider => !linkedTargets.includes(provider));
const hookTargets = selectedInstalledTargets;
const hookTargets = [...selectedInstalledTargets, ...missingSelectedTargets];
const wantHooks = installHooks && await decideHookInstall(hookRoot, hookTargets, { yes });
let bundleDir;
try {
@@ -1836,11 +1843,11 @@ async function install(flags) {
? hookTargets.filter(provider => !hookInstalledForProvider(hookRoot, provider))
: [];
let updateCheckSkipped = false;
if (copyTargets.length > 0 || missingHookTargets.length > 0) {
if (copyTargets.length > 0 || missingHookTargets.length > 0 || missingSelectedTargets.length > 0) {
try {
bundleDir = await downloadAndExtractBundle();
} catch (e) {
if (missingHookTargets.length > 0) throw e;
if (missingHookTargets.length > 0 || missingSelectedTargets.length > 0) throw e;
updateCheckSkipped = true;
console.log(`Could not check for skill updates: ${e.message}`);
}
@@ -1854,6 +1861,17 @@ async function install(flags) {
console.log(`Updated ${updated} skill(s)${v ? ` to v${v}` : ''}.`);
}
let freshWritten = 0;
if (!updateCheckSkipped && missingSelectedTargets.length > 0) {
freshWritten = copyProviderSkills(bundleDir, installRoot, missingSelectedTargets, { scope });
if (freshWritten === 0) {
console.error(`Nothing was installed: the bundle had no variants for ${missingSelectedTargets.join(', ')}.`);
process.exit(1);
}
console.log(`Installed impeccable into: ${missingSelectedTargets.join(', ')} (${scope === 'user' ? 'global' : 'project'})`);
reportProviderAgents(copyProviderAgents(bundleDir, installRoot, missingSelectedTargets, { scope }));
}
const writtenHookTargets = missingHookTargets.length > 0
? copyProviderHooks(bundleDir, hookRoot, missingHookTargets, { skillRoot: installRoot })
: [];
@@ -1862,7 +1880,7 @@ async function install(flags) {
if (updateCheckSkipped) {
console.log('Existing skills were left unchanged.');
console.log('Run with --force to reinstall.\n');
} else if (updated === 0 && writtenHookTargets.length === 0) {
} else if (updated === 0 && writtenHookTargets.length === 0 && freshWritten === 0) {
const v = getSkillsVersion(installRoot, scope);
console.log(`Skills are up to date${v ? ` (v${v})` : ''}.`);
console.log('Run with --force to reinstall.\n');
tests/skills-cli.test.js
+51
View File
@@ -1446,6 +1446,57 @@ describe('skills install/update: local universal bundle e2e', () => {
rmSync(tmp, { recursive: true, force: true });
}, 15000);
test('explicit --providers installs a missing provider without --force (#500)', () => {
const tmp = mkdtempSync(join(tmpdir(), 'imp-test-explicit-missing-'));
execSync('git init', { cwd: tmp });
const bundleRoot = createFakeUniversalBundle(tmp, ['.claude', '.cursor']);
// Seed an existing .claude install; .cursor has nothing yet.
const skillDir = join(tmp, '.claude', 'skills', 'impeccable');
mkdirSync(skillDir, { recursive: true });
writeFileSync(join(skillDir, 'SKILL.md'), '---\nname: impeccable\nversion: 9.9.9-local\n---\nSeeded install.\n');
const output = run('skills install -y --providers=cursor --no-hooks', {
cwd: tmp,
env: { ...process.env, IMPECCABLE_BUNDLE_PATH: bundleRoot },
});
expect(output).toContain('Installed impeccable into: .cursor');
expect(readFileSync(join(tmp, '.cursor', 'skills', 'impeccable', 'SKILL.md'), 'utf8')).toContain('version: 9.9.9-local');
// The unselected .claude install is left alone.
expect(readFileSync(join(skillDir, 'SKILL.md'), 'utf8')).toContain('Seeded install.');
rmSync(tmp, { recursive: true, force: true });
}, 15000);
test('explicit --providers mixes per-target updates and fresh installs (#500)', () => {
const tmp = mkdtempSync(join(tmpdir(), 'imp-test-explicit-mixed-'));
execSync('git init', { cwd: tmp });
const bundleRoot = createFakeUniversalBundle(tmp, ['.claude', '.cursor']);
// Stale .claude install; .cursor has nothing yet.
const skillDir = join(tmp, '.claude', 'skills', 'impeccable');
mkdirSync(join(skillDir, 'scripts'), { recursive: true });
writeFileSync(join(skillDir, 'SKILL.md'), '---\nname: impeccable\nstale: .claude\n---\nOld content.\n');
writeFileSync(join(skillDir, 'scripts', 'context.mjs'), 'console.log("old script");\n');
const output = run('skills install -y --providers=claude,cursor', {
cwd: tmp,
env: { ...process.env, IMPECCABLE_BUNDLE_PATH: bundleRoot },
});
expect(output).toContain('already installed');
expect(output).toContain('Updated');
expect(output).toContain('Installed impeccable into: .cursor');
expect(readFileSync(join(skillDir, 'SKILL.md'), 'utf8')).toContain('version: 9.9.9-local');
expect(readFileSync(join(tmp, '.cursor', 'skills', 'impeccable', 'SKILL.md'), 'utf8')).toContain('version: 9.9.9-local');
// The freshly installed provider gets its hooks and agents too.
expect(existsSync(join(tmp, '.cursor', 'hooks.json'))).toBe(true);
expect(existsSync(join(tmp, '.cursor', 'agents', 'impeccable-finish-reviewer.md'))).toBe(true);
rmSync(tmp, { recursive: true, force: true });
}, 15000);
test('skills update --no-hooks refreshes skills without touching malformed hook manifests', () => {
const tmp = mkdtempSync(join(tmpdir(), 'imp-test-update-no-hooks-'));
execSync('git init', { cwd: tmp });