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

Plugin rewrite: follow Setup step 1 to the engine launcher (#723)

Browse Source 
* Plugin rewrite: follow Setup step 1 to the engine launcher

The plugin subtree rewrite still keyed on the Node-era Setup sentence
and allowed-tools line, so bun run build:release failed its drift check
on main after #714 merged (Sync Generated Provider Output run
33902030478). The fallback sentence, its plugin replacement, the
pre-approval line removal, and the drift verifier now follow the
launcher form, and the launcher path is quoted so a base directory with
spaces survives, with the verb left outside the quotes.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY

* Tests: pin the legacy node pre-approval rejection too

Co-Authored-By: Claude Code <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Vau2X53xGTjjTCXWMVBoNY

---------

Co-authored-by: Claude Code <noreply@anthropic.com>
This commit is contained in:
Paul Bakaus
2026-09-04 11:13:15 -07:00
committed by GitHub
co-authored by Claude Code
parent e2ff625b63
commit e6e4bce3d5
2 changed files with 45 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
scripts/lib/plugin-paths.js
+11 -7
View File
@@ -33,7 +33,7 @@ export const PLUGIN_SCRIPTS_PATH = '<skill-base-dir>/scripts';
// no variable bound to the loaded plugin root (CLAUDE_PLUGIN_ROOT is
// hook-only). The plugin copy drops the rule and script runs go through
// the normal Bash confirmation.
export const PROJECT_ALLOWED_TOOLS_LINE = ` - Bash(node ${CLAUDE_PROJECT_SCRIPTS_PATH}/*)\n`;
export const PROJECT_ALLOWED_TOOLS_LINE = ` - Bash(${CLAUDE_PROJECT_SCRIPTS_PATH}/impeccable *)\n`;
// Setup step 1's second sentence names the project path as the fallback
// when the runtime reports no base directory. A plugin install has no
@@ -41,10 +41,10 @@ export const PROJECT_ALLOWED_TOOLS_LINE = ` - Bash(node ${CLAUDE_PROJECT_SCRIPT
// fix), and every instruction in the plugin copy already carries the
// token, so the sentence loses its fallback clause.
const SETUP_FALLBACK_TEXT =
'That base directory resolves every `node .claude/skills/impeccable/scripts/...` command in this skill and its references, ' +
'That base directory resolves every `.claude/skills/impeccable/scripts/impeccable <verb>` command in this skill and its references, ' +
'and `.claude/skills/impeccable/scripts` is the fallback only when the runtime reports no base directory.';
const SETUP_PLUGIN_TEXT =
'Every `node "<skill-base-dir>/scripts/..."` command in this skill and its references resolves against that base directory.';
'Every `"<skill-base-dir>/scripts/impeccable" <verb>` command in this skill and its references resolves against that base directory.';
// Agent files are subagent system prompts: a spawned agent never loads
// SKILL.md, so Setup's <skill-base-dir> token is undefined in the one
@@ -84,7 +84,11 @@ export function rewritePluginMarkdown(content) {
// script argument, including the token-form commands SKILL.src.md
// carries natively (Setup step 1). Runs after the path replacement so
// one pattern covers both origins; already-quoted forms don't match.
.replace(/node <skill-base-dir>\/scripts\/([^\s`"]+)/g, 'node "<skill-base-dir>/scripts/$1"');
.replace(/node <skill-base-dir>\/scripts\/([^\s`"]+)/g, 'node "<skill-base-dir>/scripts/$1"')
// The engine launcher is the command itself now (`<skill-base-dir>/scripts/impeccable <verb>`,
// or `impeccable.cmd` on a Windows shell without sh), so the launcher path
// is what gets quoted; the verb and its arguments follow unquoted.
.replace(/(?<!["\w/])<skill-base-dir>\/scripts\/impeccable(\.cmd)?(?=[\s`])/g, '"<skill-base-dir>/scripts/impeccable$1"');
}
/**
@@ -150,11 +154,11 @@ export function verifyPluginSkillRewrite(skillMdPath) {
'scripts/lib/plugin-paths.js (issue #523); update SETUP_FALLBACK_TEXT to the new wording.',
);
}
if (content.includes('Bash(node ')) {
if (/Bash\((?:node |[^)]*scripts\/impeccable)/.test(content)) {
throw new Error(
`Plugin rewrite drift: ${skillMdPath} still pre-approves a node script path. ` +
`Plugin rewrite drift: ${skillMdPath} still pre-approves an engine launcher or node script path. ` +
"SKILL.src.md's allowed-tools entry no longer matches the removal in " +
'scripts/lib/plugin-paths.js (issue #523); the plugin ships no node pre-approval.',
'scripts/lib/plugin-paths.js (issue #523); the plugin ships no launcher pre-approval.',
);
}
if (content.includes(CLAUDE_PROJECT_SCRIPTS_PATH)) {
tests/plugin-paths.test.js
+34 -12
View File
@@ -44,6 +44,16 @@ describe('rewritePluginMarkdown', () => {
expect(output).toContain('node "<skill-base-dir>/scripts/live-poll.mjs" --reply EVENT_ID done');
});
test('quotes the engine launcher path and leaves the verb outside the quotes', () => {
const output = rewritePluginMarkdown(
'Run `<skill-base-dir>/scripts/impeccable context` once, then `<skill-base-dir>/scripts/impeccable.cmd doctor --json`; ' +
'already quoted: `"<skill-base-dir>/scripts/impeccable" hooks on`.',
);
expect(output).toContain('`"<skill-base-dir>/scripts/impeccable" context`');
expect(output).toContain('`"<skill-base-dir>/scripts/impeccable.cmd" doctor --json`');
expect(output).not.toContain('""<skill-base-dir>');
});
test('quotes commands already in the skill-base-dir form without double-quoting', () => {
// SKILL.src.md's Setup step 1 carries the token form natively; a base
// directory with spaces splits an unquoted path before node sees it.
@@ -60,15 +70,15 @@ describe('rewritePluginMarkdown', () => {
const frontmatter = [
'allowed-tools:',
' - Bash(npx impeccable *)',
' - Bash(node .claude/skills/impeccable/scripts/*)',
' - Bash(.claude/skills/impeccable/scripts/impeccable *)',
'---',
'',
].join('\n');
const output = rewritePluginMarkdown(frontmatter);
// The generic path rewrite alone would leave Bash(node <skill-base-dir>/scripts/*),
// The generic path rewrite alone would leave Bash(<skill-base-dir>/scripts/impeccable *),
// a dead literal, and any wildcard replacement would auto-approve
// same-shaped paths outside the plugin. The line must go entirely.
expect(output).not.toContain('Bash(node ');
expect(output).not.toContain('scripts/impeccable *');
expect(output).toContain(' - Bash(npx impeccable *)\n---');
});
@@ -76,12 +86,12 @@ describe('rewritePluginMarkdown', () => {
const input =
'1. Run `node <skill-base-dir>/scripts/context.mjs` once per session, where `<skill-base-dir>` is the ' +
"loaded base directory the runtime reports for this skill; keep cwd at the user's project. " +
'That base directory resolves every `node .claude/skills/impeccable/scripts/...` command in this skill ' +
'That base directory resolves every `.claude/skills/impeccable/scripts/impeccable <verb>` command in this skill ' +
'and its references, and `.claude/skills/impeccable/scripts` is the fallback only when the runtime ' +
'reports no base directory. Pass a named source file or route as `--target <path>`.';
const output = rewritePluginMarkdown(input);
expect(output).toContain(
'Every `node "<skill-base-dir>/scripts/..."` command in this skill and its references resolves against that base directory.',
'Every `"<skill-base-dir>/scripts/impeccable" <verb>` command in this skill and its references resolves against that base directory.',
);
// The naive rewrite would keep the fallback clause and name the token as
// its own fallback for when there is no base directory to resolve it.
@@ -247,11 +257,11 @@ describe('verifyPluginSkillRewrite', () => {
const goodSkill = [
'allowed-tools:',
' - Bash(node .claude/skills/impeccable/scripts/*)',
' - Bash(.claude/skills/impeccable/scripts/impeccable *)',
'',
'1. Run `node <skill-base-dir>/scripts/context.mjs` once per session, where `<skill-base-dir>` is the ' +
'1. Run `<skill-base-dir>/scripts/impeccable context` once per session, where `<skill-base-dir>` is the ' +
"loaded base directory the runtime reports for this skill; keep cwd at the user's project. " +
'That base directory resolves every `node .claude/skills/impeccable/scripts/...` command in this skill ' +
'That base directory resolves every `.claude/skills/impeccable/scripts/impeccable <verb>` command in this skill ' +
'and its references, and `.claude/skills/impeccable/scripts` is the fallback only when the runtime ' +
'reports no base directory.',
].join('\n');
@@ -269,13 +279,25 @@ describe('verifyPluginSkillRewrite', () => {
expect(() => verifyPluginSkillRewrite(p)).toThrow(/Setup step 1 fallback sentence/);
});
test('fails the build when a node pre-approval survives the removal', () => {
test('fails the build when a launcher pre-approval survives the removal', () => {
const reworded = goodSkill.replace(
'Bash(node .claude/skills/impeccable/scripts/*)',
'Bash(node .claude/skills/impeccable/scripts/**)',
'Bash(.claude/skills/impeccable/scripts/impeccable *)',
'Bash(.claude/skills/impeccable/scripts/impeccable.cmd *)',
);
const p = writeSkill(rewritePluginMarkdown(reworded));
expect(() => verifyPluginSkillRewrite(p)).toThrow(/pre-approves a node script path/);
expect(() => verifyPluginSkillRewrite(p)).toThrow(/pre-approves an engine launcher/);
});
test('fails the build when a legacy node pre-approval survives', () => {
// The Node-era line is gone from SKILL.src.md, but a copy that still
// carries one must fail the same way as a surviving launcher line.
const p = writeSkill(
rewritePluginMarkdown(goodSkill).replace(
'allowed-tools:\n',
'allowed-tools:\n - Bash(node <skill-base-dir>/scripts/*)\n',
),
);
expect(() => verifyPluginSkillRewrite(p)).toThrow(/pre-approves an engine launcher or node script path/);
});
test('fails the build when the project-relative scripts path survives at all', () => {