Skip to content

Commit 9bf9729

Browse files
authored
fix: prevent skill-active-state collision between OMC and project custom skills (#1741)
When a project defines a custom skill with the same name as an OMC built-in skill (e.g., `.claude/skills/plan/`), the pre-tool hook incorrectly applied OMC's SKILL_PROTECTION map to the custom skill, writing skill-active-state.json with `active: true`. This caused the stop hook to block session termination with reinforcement messages even after the custom skill completed. The fix passes the raw (un-normalized) skill name through the call chain so `getSkillProtection()` can distinguish OMC skills (prefixed with `oh-my-claudecode:`) from project custom or other plugin skills. Non-OMC skills now correctly default to `'none'` protection. Affected files: - src/hooks/skill-state/index.ts: Add rawSkillName param to getSkillProtection/writeSkillActiveState - src/hooks/bridge.ts: Extract and pass raw skill name via getRawSkillName() - scripts/pre-tool-enforcer.mjs: Same fix for the CJS runtime hook Ref: #1581
1 parent 74f9bf8 commit 9bf9729

File tree

4 files changed

+89
-12
lines changed

4 files changed

+89
-12
lines changed

scripts/pre-tool-enforcer.mjs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,14 @@ const SKILL_PROTECTION_MAP = {
360360
deepinit: 'heavy',
361361
};
362362

363-
function getSkillProtectionLevel(skillName) {
363+
function getSkillProtectionLevel(skillName, rawSkillName) {
364+
// When rawSkillName is provided, only apply protection to OMC-prefixed skills.
365+
// Non-prefixed skills are project custom skills or other plugins — no protection.
366+
// See: https://github.com/Yeachan-Heo/oh-my-claudecode/issues/1581
367+
if (rawSkillName != null && typeof rawSkillName === 'string' &&
368+
!rawSkillName.toLowerCase().startsWith('oh-my-claudecode:')) {
369+
return 'none';
370+
}
364371
const normalized = (skillName || '').toLowerCase().replace(/^oh-my-claudecode:/, '');
365372
return SKILL_PROTECTION_MAP[normalized] || 'none';
366373
}
@@ -373,8 +380,8 @@ function extractSkillName(toolInput) {
373380
return normalized.includes(':') ? normalized.split(':').at(-1).toLowerCase() : normalized.toLowerCase();
374381
}
375382

376-
function writeSkillActiveState(directory, skillName, sessionId) {
377-
const protection = getSkillProtectionLevel(skillName);
383+
function writeSkillActiveState(directory, skillName, sessionId, rawSkillName) {
384+
const protection = getSkillProtectionLevel(skillName, rawSkillName);
378385
if (protection === 'none') return;
379386

380387
const config = SKILL_PROTECTION_CONFIGS[protection];
@@ -456,7 +463,10 @@ async function main() {
456463
if (skillName) {
457464
const sid = typeof data.session_id === 'string' ? data.session_id
458465
: typeof data.sessionId === 'string' ? data.sessionId : '';
459-
writeSkillActiveState(directory, skillName, sid);
466+
// Pass rawSkillName to distinguish OMC skills from project custom skills (issue #1581)
467+
const rawSkill = toolInput.skill || toolInput.skill_name || toolInput.skillName || toolInput.command || '';
468+
const rawSkillName = typeof rawSkill === 'string' && rawSkill.trim() ? rawSkill.trim() : undefined;
469+
writeSkillActiveState(directory, skillName, sid, rawSkillName);
460470
}
461471
}
462472

src/hooks/bridge.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1351,14 +1351,17 @@ function processPreToolUse(input: HookInput): HookOutput {
13511351
// Activate skill state when Skill tool is invoked (issue #1033)
13521352
// This writes skill-active-state.json so the Stop hook can prevent premature
13531353
// session termination while a skill is executing.
1354+
// Pass rawSkillName so writeSkillActiveState can distinguish OMC built-in
1355+
// skills from project custom skills with the same name (issue #1581).
13541356
if (input.toolName === "Skill") {
13551357
const skillName = getInvokedSkillName(input.toolInput);
13561358
if (skillName) {
1359+
const rawSkillName = getRawSkillName(input.toolInput);
13571360
// Use the statically-imported synchronous write so it completes before
13581361
// the Stop hook can fire. The previous fire-and-forget .then() raced with
13591362
// the Stop hook in short-lived processes.
13601363
try {
1361-
writeSkillActiveState(directory, skillName, input.sessionId);
1364+
writeSkillActiveState(directory, skillName, input.sessionId, rawSkillName);
13621365
} catch {
13631366
// Skill-state write is best-effort; don't fail the hook on error.
13641367
}
@@ -1539,6 +1542,19 @@ function getInvokedSkillName(toolInput: unknown): string | null {
15391542
return namespaced?.toLowerCase() || null;
15401543
}
15411544

1545+
/**
1546+
* Extract the raw (un-normalized) skill name from Skill tool input.
1547+
* Used to distinguish OMC built-in skills (prefixed with 'oh-my-claudecode:')
1548+
* from project custom skills or other plugin skills with the same bare name.
1549+
* See: https://github.com/Yeachan-Heo/oh-my-claudecode/issues/1581
1550+
*/
1551+
function getRawSkillName(toolInput: unknown): string | undefined {
1552+
if (!toolInput || typeof toolInput !== "object") return undefined;
1553+
const input = toolInput as Record<string, unknown>;
1554+
const raw = input.skill ?? input.skill_name ?? input.skillName ?? input.command ?? null;
1555+
return typeof raw === "string" && raw.trim().length > 0 ? raw.trim() : undefined;
1556+
}
1557+
15421558
async function processPostToolUse(input: HookInput): Promise<HookOutput> {
15431559
const directory = resolveToWorktreeRoot(input.directory);
15441560
const messages: string[] = [];

src/hooks/skill-state/__tests__/skill-state.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,30 @@ describe('skill-state', () => {
107107
expect(getSkillProtection('SKILL')).toBe('light');
108108
expect(getSkillProtection('Plan')).toBe('medium');
109109
});
110+
111+
it('returns none for project custom skills with same name as OMC skills (issue #1581)', () => {
112+
// rawSkillName without oh-my-claudecode: prefix → project custom skill
113+
expect(getSkillProtection('plan', 'plan')).toBe('none');
114+
expect(getSkillProtection('review', 'review')).toBe('none');
115+
expect(getSkillProtection('tdd', 'tdd')).toBe('none');
116+
});
117+
118+
it('returns protection for OMC skills when rawSkillName has prefix', () => {
119+
expect(getSkillProtection('plan', 'oh-my-claudecode:plan')).toBe('medium');
120+
expect(getSkillProtection('deepinit', 'oh-my-claudecode:deepinit')).toBe('heavy');
121+
});
122+
123+
it('returns none for other plugin skills with rawSkillName', () => {
124+
// ouroboros:plan, claude-mem:make-plan etc. should not get OMC protection
125+
expect(getSkillProtection('plan', 'ouroboros:plan')).toBe('none');
126+
expect(getSkillProtection('make-plan', 'claude-mem:make-plan')).toBe('none');
127+
});
128+
129+
it('falls back to map lookup when rawSkillName is not provided', () => {
130+
// Backward compatibility: no rawSkillName → use SKILL_PROTECTION map
131+
expect(getSkillProtection('plan')).toBe('medium');
132+
expect(getSkillProtection('deepinit')).toBe('heavy');
133+
});
110134
});
111135

112136
// -----------------------------------------------------------------------
@@ -177,6 +201,20 @@ describe('skill-state', () => {
177201
expect(state!.skill_name).toBe('plan');
178202
});
179203

204+
it('does not write state for project custom skills with same name as OMC skills (issue #1581)', () => {
205+
// rawSkillName='plan' (no prefix) → project custom skill → no state
206+
const state = writeSkillActiveState(tempDir, 'plan', 'session-1', 'plan');
207+
expect(state).toBeNull();
208+
expect(readSkillActiveState(tempDir, 'session-1')).toBeNull();
209+
});
210+
211+
it('writes state for OMC skills when rawSkillName has prefix', () => {
212+
const state = writeSkillActiveState(tempDir, 'plan', 'session-1', 'oh-my-claudecode:plan');
213+
expect(state).not.toBeNull();
214+
expect(state!.skill_name).toBe('plan');
215+
expect(state!.max_reinforcements).toBe(5);
216+
});
217+
180218
it('overwrites existing state when new skill is invoked', () => {
181219
writeSkillActiveState(tempDir, 'plan', 'session-1');
182220
const state2 = writeSkillActiveState(tempDir, 'external-context', 'session-1');

src/hooks/skill-state/index.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -130,19 +130,28 @@ const SKILL_PROTECTION: Record<string, SkillProtectionLevel> = {
130130
* Anthropic's example-skills, document-skills, superpowers, data, etc.)
131131
* default to 'none' so the Stop hook does not block them.
132132
*
133-
* Note: bridge.ts strips the 'oh-my-claudecode:' prefix before calling
134-
* this function, so skill names arrive in bare form (e.g., 'plan', 'xlsx').
133+
* @param skillName - The normalized (prefix-stripped) skill name.
134+
* @param rawSkillName - The original skill name as invoked (e.g., 'oh-my-claudecode:plan'
135+
* or 'plan'). When provided, only skills invoked with the 'oh-my-claudecode:' prefix
136+
* are eligible for protection. This prevents project custom skills (e.g., a user's
137+
* `.claude/skills/plan/`) from being confused with OMC built-in skills of the same name.
138+
* See: https://github.com/Yeachan-Heo/oh-my-claudecode/issues/1581
135139
*/
136-
export function getSkillProtection(skillName: string): SkillProtectionLevel {
140+
export function getSkillProtection(skillName: string, rawSkillName?: string): SkillProtectionLevel {
141+
// When rawSkillName is provided, only apply protection to OMC-prefixed skills.
142+
// Non-prefixed skills are project custom skills or other plugins — no protection.
143+
if (rawSkillName != null && !rawSkillName.toLowerCase().startsWith('oh-my-claudecode:')) {
144+
return 'none';
145+
}
137146
const normalized = skillName.toLowerCase().replace(/^oh-my-claudecode:/, '');
138147
return SKILL_PROTECTION[normalized] ?? 'none';
139148
}
140149

141150
/**
142151
* Get the protection config for a skill.
143152
*/
144-
export function getSkillConfig(skillName: string): SkillStateConfig {
145-
return PROTECTION_CONFIGS[getSkillProtection(skillName)];
153+
export function getSkillConfig(skillName: string, rawSkillName?: string): SkillStateConfig {
154+
return PROTECTION_CONFIGS[getSkillProtection(skillName, rawSkillName)];
146155
}
147156

148157
/**
@@ -163,13 +172,17 @@ export function readSkillActiveState(
163172
/**
164173
* Write skill active state.
165174
* Called when a skill is invoked via the Skill tool.
175+
*
176+
* @param rawSkillName - The original skill name as invoked, used to distinguish
177+
* OMC built-in skills from project custom skills. See getSkillProtection().
166178
*/
167179
export function writeSkillActiveState(
168180
directory: string,
169181
skillName: string,
170-
sessionId?: string
182+
sessionId?: string,
183+
rawSkillName?: string,
171184
): SkillActiveState | null {
172-
const protection = getSkillProtection(skillName);
185+
const protection = getSkillProtection(skillName, rawSkillName);
173186

174187
// Skills with 'none' protection don't need state tracking
175188
if (protection === 'none') {

0 commit comments

Comments
 (0)