fix: prevent skill-active-state collision between OMC and project custom skills#1741
Conversation
…tom skills 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: Yeachan-Heo#1581
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fda58b843e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| export function getSkillProtection(skillName: string, rawSkillName?: string): SkillProtectionLevel { | ||
| // When rawSkillName is provided, only apply protection to OMC-prefixed skills. | ||
| // Non-prefixed skills are project custom skills or other plugins — no protection. | ||
| if (rawSkillName != null && !rawSkillName.toLowerCase().startsWith('oh-my-claudecode:')) { | ||
| return 'none'; | ||
| } | ||
| const normalized = skillName.toLowerCase().replace(/^oh-my-claudecode:/, ''); | ||
| return SKILL_PROTECTION[normalized] ?? 'none'; |
There was a problem hiding this comment.
This source change never reaches the package entrypoints that users actually run. package.json ships dist/index.js and bridge/cli.cjs, but in this commit the checked-in generated files still contain the old getSkillProtection(skillName) / writeSkillActiveState(directory, skillName, sessionId) logic and bridge/cli.cjs still calls the 3-argument form, so imported processHook() consumers and the bundled CLI continue to reproduce the plan/review collision even though src/hooks/skill-state/index.ts is fixed here.
Useful? React with 👍 / 👎.
2 participants
Problem
When a project defines a custom skill with the same bare name as an OMC built-in skill (e.g.,
.claude/skills/plan/vsoh-my-claudecode:plan), the pre-tool hook applies OMC'sSKILL_PROTECTIONmap to the custom skill becauseextractSkillName()/getInvokedSkillName()strip the namespace prefix before lookup. This writesskill-active-state.jsonwithactive: true, causing the stop hook to block session termination with reinforcement messages — even after the custom skill has completed.Reproduction
.claude/skills/plan/(or any name inSKILL_PROTECTION_MAP)/planskill: "plan"(no OMC prefix)getSkillProtectionLevel("plan")returns'medium'→ state file written[SKILL ACTIVE: plan]reinforcement messagesThis is a follow-up to #1581 — that fix changed the default for unregistered skills to
'none', but skills likeplan,review,tddare explicitly registered inSKILL_PROTECTION_MAPwith non-none levels, so the collision persists.Fix
Pass the raw (un-normalized) skill name through the call chain so
getSkillProtection()can distinguish OMC built-in skills (prefixed withoh-my-claudecode:) from project custom or other plugin skills. Non-OMC-prefixed skills now correctly default to'none'protection regardless of name collisions.Changes
src/hooks/skill-state/index.tsrawSkillNameparam togetSkillProtection()andwriteSkillActiveState()src/hooks/bridge.tsgetRawSkillName()helper; pass raw name towriteSkillActiveState()scripts/pre-tool-enforcer.mjssrc/hooks/skill-state/__tests__/skill-state.test.tsBehavior matrix
rawSkillNameSkill(skill: "oh-my-claudecode:plan")"oh-my-claudecode:plan"medium✅Skill(skill: "plan")(project custom)"plan"none✅Skill(skill: "ouroboros:plan")"ouroboros:plan"none✅Skill(skill: "plan")(no rawSkillName, legacy)undefinedmedium(backward compat)Test plan
skill-state.test.ts: 44 tests passed (7 new)skill-state-stop.test.ts: 8 tests passed (integration)rawSkillNameis optional; omitting it preserves existing behaviorRetargeted from #1739 (was targeting main).