From a09e43ad20b5e71a2ff7a935cdc5d5855dc7a1a0 Mon Sep 17 00:00:00 2001 From: Gabi Date: Wed, 10 Jun 2026 14:48:00 +0200 Subject: [PATCH] fix: rewrite skill paths on install so /pixelslop works in any project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The installer rewrote bin/ paths in agent specs but copied the skill tree verbatim, so SKILL.md and checkpoint-protocol.md kept relative `bin/pixelslop-tools.cjs` references. Those only resolve from the repo checkout, so /pixelslop reported "tools.cjs not installed" in every other project — which is exactly what a global install is for. rewriteSkillTreePaths walks the installed skill and rewrites every .md the same way agents are rewritten, so SKILL.md and the fix guides get absolute paths and the skill runs from anywhere. Includes a regression test plus a guard that the shipped SKILL.md really does ship relative paths, so the test can't pass vacuously. Also filters macOS ._ junk out of the codex-toml spec listing. --- bin/pixelslop.mjs | 31 +++++++++++++- tests/codex-toml.test.js | 5 ++- tests/skill-path-rewrite.test.js | 72 ++++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 tests/skill-path-rewrite.test.js diff --git a/bin/pixelslop.mjs b/bin/pixelslop.mjs index 2a2b06e..9f73ed8 100644 --- a/bin/pixelslop.mjs +++ b/bin/pixelslop.mjs @@ -252,6 +252,31 @@ export function agentMdToCodexToml(md) { ].join('\n'); } +/** + * Rewrite bin/ and resources/ paths in every Markdown file under an installed + * skill tree, exactly the way agent specs are rewritten. Without this, SKILL.md + * and fix guides like checkpoint-protocol.md keep relative `bin/pixelslop-tools.cjs` + * paths that only resolve from the repo checkout — so `/pixelslop` fails in any + * other project ("tools.cjs not installed"). + * + * @param {string} skillDir - The installed skill directory (INSTALL_ROOT/skill) + * @param {string} installRoot - Install root for resolving absolute paths + */ +export function rewriteSkillTreePaths(skillDir, installRoot) { + const walk = (dir) => { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (entry.name.startsWith('._')) continue; + const full = join(dir, entry.name); + if (entry.isDirectory()) { walk(full); continue; } + if (!entry.name.endsWith('.md')) continue; + const raw = readFileSync(full, 'utf8'); + const rewritten = rewriteAgentPaths(raw, installRoot); + if (rewritten !== raw) writeFileSync(full, rewritten); + } + }; + walk(skillDir); +} + // ───────────────────────────────────────────── // Browser Runtime Detection // ───────────────────────────────────────────── @@ -1190,11 +1215,15 @@ function install(options = {}) { // Step 3: Ensure a browser runtime exists before wiring clients const browserRuntime = ensureBrowserRuntime(); - // Step 4: Copy skill directory to install root (source of truth) + // Step 4: Copy skill directory to install root (source of truth), then rewrite + // its bin/ and resources/ paths to absolute — same as agents. Without this the + // skill keeps relative `bin/pixelslop-tools.cjs` paths that only resolve from + // the repo, so /pixelslop reports "not installed" in every other project. copyDir( join(PACKAGE_ROOT, 'dist', 'skill'), join(INSTALL_ROOT, 'skill') ); + rewriteSkillTreePaths(join(INSTALL_ROOT, 'skill'), INSTALL_ROOT); const resourceCount = readdirSync(join(INSTALL_ROOT, 'skill', 'resources')).length; log('✓', `skill/SKILL.md + ${resourceCount} resources`); diff --git a/tests/codex-toml.test.js b/tests/codex-toml.test.js index 3daac75..dd4e580 100644 --- a/tests/codex-toml.test.js +++ b/tests/codex-toml.test.js @@ -80,9 +80,10 @@ describe('agentMdToCodexToml — edge cases', () => { }); describe('every shipped agent spec converts cleanly', () => { + const md = (f) => f.endsWith('.md') && !f.startsWith('._'); const specs = [ - ...readdirSync(join(ROOT, 'dist', 'agents')).filter(f => f.endsWith('.md')), - ...readdirSync(join(ROOT, 'dist', 'agents', 'internal')).filter(f => f.endsWith('.md') && !f.startsWith('._')).map(f => join('internal', f)), + ...readdirSync(join(ROOT, 'dist', 'agents')).filter(md), + ...readdirSync(join(ROOT, 'dist', 'agents', 'internal')).filter(md).map(f => join('internal', f)), ]; for (const rel of specs) { diff --git a/tests/skill-path-rewrite.test.js b/tests/skill-path-rewrite.test.js new file mode 100644 index 0000000..554a195 --- /dev/null +++ b/tests/skill-path-rewrite.test.js @@ -0,0 +1,72 @@ +/** + * Skill Path Rewrite Tests + * + * Regression guard for a real bug: the installer copied the skill tree to the + * install root WITHOUT rewriting its `bin/pixelslop-tools.cjs` references, so + * SKILL.md (and fix guides like checkpoint-protocol.md) kept relative paths. + * That made `/pixelslop` work only from the repo checkout and report + * "tools.cjs not installed" in every other project. + * + * rewriteSkillTreePaths rewrites every .md under the installed skill the same + * way agent specs are rewritten. These tests pin that, and that the shipped + * source genuinely needs it (so the test can't pass vacuously). + * + * Run: node --test tests/skill-path-rewrite.test.js + */ + +import { describe, it } from 'node:test'; +import { strict as assert } from 'node:assert'; +import { readFileSync, writeFileSync, mkdtempSync, mkdirSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { fileURLToPath } from 'node:url'; +import { dirname, join } from 'node:path'; +import { rewriteSkillTreePaths } from '../bin/pixelslop.mjs'; + +const ROOT = join(dirname(fileURLToPath(import.meta.url)), '..'); + +describe('rewriteSkillTreePaths', () => { + it('rewrites relative tool paths in SKILL.md and nested resources to absolute', () => { + const dir = mkdtempSync(join(tmpdir(), 'pxs-skillrw-')); + const installRoot = join(dir, 'root'); + const skillDir = join(installRoot, 'skill'); + mkdirSync(join(skillDir, 'resources'), { recursive: true }); + + writeFileSync(join(skillDir, 'SKILL.md'), 'Run `node bin/pixelslop-tools.cjs init`.\n'); + writeFileSync(join(skillDir, 'resources', 'checkpoint-protocol.md'), + 'Run `node bin/pixelslop-tools.cjs checkpoint create`.\n'); + // a non-md file must be left alone + writeFileSync(join(skillDir, 'resources', 'report-template.html'), '

bin/pixelslop-tools.cjs

'); + + rewriteSkillTreePaths(skillDir, installRoot); + + const skill = readFileSync(join(skillDir, 'SKILL.md'), 'utf8'); + const guide = readFileSync(join(skillDir, 'resources', 'checkpoint-protocol.md'), 'utf8'); + const html = readFileSync(join(skillDir, 'resources', 'report-template.html'), 'utf8'); + + assert.ok(skill.includes(join(installRoot, 'bin', 'pixelslop-tools.cjs')), 'SKILL.md → absolute'); + assert.ok(!/[^/]bin\/pixelslop-tools\.cjs/.test(skill), 'no relative ref left in SKILL.md'); + assert.ok(guide.includes(join(installRoot, 'bin', 'pixelslop-tools.cjs')), 'resource guide → absolute'); + assert.ok(html.includes('bin/pixelslop-tools.cjs'), 'non-md files are untouched'); + + rmSync(dir, { recursive: true, force: true }); + }); + + it('is a no-op on files without paths to rewrite', () => { + const dir = mkdtempSync(join(tmpdir(), 'pxs-skillrw-')); + const skillDir = join(dir, 'skill'); + mkdirSync(skillDir, { recursive: true }); + const original = '# Just docs, no tool calls.\n'; + writeFileSync(join(skillDir, 'notes.md'), original); + rewriteSkillTreePaths(skillDir, dir); + assert.equal(readFileSync(join(skillDir, 'notes.md'), 'utf8'), original); + rmSync(dir, { recursive: true, force: true }); + }); +}); + +describe('the shipped skill genuinely needs rewriting (no vacuous pass)', () => { + it('dist/skill/SKILL.md ships with relative tool paths', () => { + const skill = readFileSync(join(ROOT, 'dist', 'skill', 'SKILL.md'), 'utf8'); + assert.ok(/[^/]bin\/pixelslop-tools\.cjs/.test(skill), + 'source SKILL.md uses relative paths — that is what the install must rewrite'); + }); +});