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

Guard the OpenCode legacy migration against symlinks and home-rooted repos

Browse Source 
Both review bots caught real hazards in the migration: a symlinked
~/.opencode/skills (shared skill storage) would have its target emptied
through the link, and in a home-rooted repo that path is a live
project-scope install, not a stranded pre-#406 global copy. The
migration now requires a real directory (lstat), compares the
just-written dir by realpath instead of string, and skips entirely when
the home dir is itself a repo. Two regression tests cover the symlink
and dotfiles-repo shapes.

Prepared with AI assistance (Claude Code), directed by @pbakaus.

Co-Authored-By: Claude Code <noreply@anthropic.com>
This commit is contained in:
Paul Bakaus
2026-07-25 19:03:32 -07:00
co-authored by Claude Code
parent caef4b8e4c
commit cda1c572d9
2 changed files with 60 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cli/bin/commands/skills.mjs
+13 -1
View File
@@ -1162,9 +1162,21 @@ function copyProviderSkills(bundleDir, root, targets, { scope } = {}) {
// location OpenCode never reads. Now that the real copy sits in the
// config dir, drop exactly the skills just written from the stranded
// location; sibling skills and everything else in ~/.opencode stay.
// Guards (both flagged in review): a symlinked skills dir is shared
// storage whose target must not be emptied through the link, the
// just-written dir must be compared by realpath rather than string,
// and a home-rooted repo makes `.opencode/skills` a live
// project-scope install rather than a stranded global one.
if (scope === 'user' && provider === '.opencode') {
const legacyDir = join(root, '.opencode', 'skills');
if (legacyDir !== localSkillsDir && existsSync(legacyDir)) {
let migratable = false;
try {
migratable = existsSync(legacyDir)
&& !lstatSync(legacyDir).isSymbolicLink()
&& realpathSync(legacyDir) !== realpathSync(localSkillsDir)
&& !existsSync(join(root, '.git'));
} catch { migratable = false; }
if (migratable) {
for (const skill of readdirSync(srcDir, { withFileTypes: true })) {
if (!skill.isDirectory()) continue;
rmSync(join(legacyDir, skill.name), { recursive: true, force: true });
tests/skills-cli.test.js
+47
View File
@@ -956,6 +956,53 @@ describe('skills install/update: local universal bundle e2e', () => {
rmSync(home, { recursive: true, force: true });
}, 15000);
test('OpenCode migration never follows a symlinked legacy skills dir (#406)', () => {
const tmp = mkdtempSync(join(tmpdir(), 'imp-test-oc-symlink-'));
const home = mkdtempSync(join(tmpdir(), 'imp-home-oc-symlink-'));
execSync('git init', { cwd: tmp });
// Shared skill storage with ~/.opencode/skills symlinked at it. Deleting
// "the legacy copy" through the link would destroy the shared original.
writeSkill(join(home, '.config'), 'agents', 'impeccable');
mkdirSync(join(home, '.opencode'), { recursive: true });
symlinkSync(join(home, '.config', 'agents', 'skills'), join(home, '.opencode', 'skills'), 'dir');
const bundleRoot = createFakeUniversalBundle(tmp, ['.opencode']);
const env = { ...process.env, HOME: home, IMPECCABLE_BUNDLE_PATH: bundleRoot };
delete env.OPENCODE_CONFIG_DIR;
delete env.XDG_CONFIG_HOME;
run('skills install -y --providers=opencode --scope=global --no-hooks', { cwd: tmp, env });
expect(existsSync(join(home, '.config', 'opencode', 'skills', 'impeccable', 'SKILL.md'))).toBe(true);
// The shared store behind the symlink is intact, link included.
expect(existsSync(join(home, '.config', 'agents', 'skills', 'impeccable', 'SKILL.md'))).toBe(true);
expect(lstatSync(join(home, '.opencode', 'skills')).isSymbolicLink()).toBe(true);
rmSync(tmp, { recursive: true, force: true });
rmSync(home, { recursive: true, force: true });
}, 15000);
test('OpenCode migration leaves a home-rooted repo project install alone (#406)', () => {
const tmp = mkdtempSync(join(tmpdir(), 'imp-test-oc-homerepo-'));
const home = mkdtempSync(join(tmpdir(), 'imp-home-oc-homerepo-'));
execSync('git init', { cwd: tmp });
// The home dir IS a repo (dotfiles setup): .opencode/skills there is a
// live project-scope install, not a stranded pre-#406 global one.
execSync('git init', { cwd: home });
writeSkill(home, '.opencode', 'impeccable');
const bundleRoot = createFakeUniversalBundle(tmp, ['.opencode']);
const env = { ...process.env, HOME: home, IMPECCABLE_BUNDLE_PATH: bundleRoot };
delete env.OPENCODE_CONFIG_DIR;
delete env.XDG_CONFIG_HOME;
run('skills install -y --providers=opencode --scope=global --no-hooks', { cwd: tmp, env });
expect(existsSync(join(home, '.config', 'opencode', 'skills', 'impeccable', 'SKILL.md'))).toBe(true);
expect(existsSync(join(home, '.opencode', 'skills', 'impeccable', 'SKILL.md'))).toBe(true);
rmSync(tmp, { recursive: true, force: true });
rmSync(home, { recursive: true, force: true });
}, 15000);
test('global install detects OpenCode from ~/.config/opencode alone (#406)', () => {
const tmp = mkdtempSync(join(tmpdir(), 'imp-test-oc-detect-'));
const home = mkdtempSync(join(tmpdir(), 'imp-home-oc-detect-'));