Repository navigation
fix(registry): install harness-neutral skills for Claude Code too - #32
Conversation
brainstorming, macos-storage-cleanup, and windows-cleanup were limited to the .agents target with no harness dependency behind it. brainstorming inherited the restriction from the old per-harness manifest, where it was added only to the Codex list; the cleanup skills were registered as Codex skills by default. Drop their targetExceptions so sjskills places them in both .agents and .claude. brainstorming's dispatch step named Codex's fork_turns parameter only; it now states the intent (a fresh, non-forked subagent) with the Codex and Claude Code forms. codex-cleanup, delegate-ui-to-claude, and impeccable keep their single target because each is genuinely tied to one harness. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 53d4c09016
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -129,9 +129,6 @@ | |||
| } | |||
| }, | |||
| "targetExceptions": { | |||
There was a problem hiding this comment.
After brainstorming and the KICPA profile's windows-cleanup are reconciled into .claude/skills, both remain eligible for automatic selection because their frontmatter lacks disable-model-invocation: true; Claude Code defaults this field to false, regardless of agents/openai.yaml (Claude Code skills reference). This violates both skills' explicit-only policy and can start cleanup guidance or brainstorming fan-out without the user naming the skill, so add the Claude frontmatter gate before widening their targets.
AGENTS.md reference: AGENTS.md:L46-L51
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0bebe6b: added disable-model-invocation: true to brainstorming, windows-cleanup, and macos-storage-cleanup, matching their allow_implicit_invocation: false. No shared instruction routes to them by name.
| "impeccable": [ | ||
| ".claude" | ||
| ], | ||
| "macos-storage-cleanup": [ | ||
| ".agents" | ||
| ], | ||
| "windows-cleanup": [ | ||
| ".agents" | ||
| ] |
There was a problem hiding this comment.
Removing macos-storage-cleanup from targetExceptions cannot make it visible in Claude Code because the skill appears in neither global.baseline nor any profile; sjskills resolves targets only after a skill is selected. Consequently, the advertised global sync still never installs this skill under either target, so it must be added to an appropriate desired set before this target change has any effect.
AGENTS.md reference: AGENTS.md:L77-L86
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Correct — macos-storage-cleanup is in no desired set, so this change has no install effect for it today. Keeping the exception removal because the skill has no Codex dependency, so if a project or profile selects it later it lands in both targets. Choosing a desired set for it is out of scope here; PR description updated.
| agent rather than a fork. Leave model and reasoning-effort overrides unset so | ||
| both inherit from the caller. Run independent prompts in |
There was a problem hiding this comment.
When CLAUDE_CODE_SUBAGENT_MODEL is configured, leaving the Claude Agent call's model unset does not inherit the caller: Claude resolves a per-invocation model first, then the agent definition, then that environment default, and only then the main conversation (Claude Code model resolution). The stated invariant therefore fails in such sessions and brainstorming workers may run on a different model; pass model: "inherit" for the Claude invocation rather than instructing it to omit the override.
AGENTS.md reference: AGENTS.md:L7-L9
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 0bebe6b: Claude Code now passes the caller's model explicitly. The Agent tool here accepts model aliases (sonnet/opus/haiku/fable), not inherit, so the instruction states the intent rather than that literal.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
Included review availability: This review used your included allowance. 4 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 SummarySummary by CodeRabbit
WalkthroughThe registry removes target exceptions for three skills. The brainstorming skill now specifies runtime-specific subagent instructions for Codex and Claude Code. ChangesDefault skill targets
Brainstorming subagent instructions
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Feature Merge Risk: ⚪ Minimal · up to The three skills can target both skill directories when selected, while global sync continues to install its fixed baseline. No actionable merge risk remains. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Placing brainstorming and the cleanup skills in .claude/skills would make them eligible for automatic selection, because Claude Code reads its invocation policy only from frontmatter. Add disable-model-invocation to all three, matching allow_implicit_invocation: false in their OpenAI metadata; no shared instruction routes to them by name. In Claude Code an unset subagent model can resolve to a configured subagent default instead of the caller's, so brainstorming now tells Claude to pass the caller's model explicitly. The Agent tool takes model aliases rather than "inherit", so the instruction names the intent. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Problem
brainstorming,macos-storage-cleanup, andwindows-cleanupwere restricted to the.agentstarget intargetExceptions, sosjskillsnever placed them in.claude/skillsand they were invisible in Claude Code. (macos-storage-cleanupis currently in no desired set, so for it this only corrects the target policy.) None of them depends on Codex:brainstormingwas added only to the Codex list of the old per-harnessglobal-skills.json(bcc2cc3); the registry migration (7cd9e88) preserved that as a target exception.Change
Drop the three
targetExceptionsentries so the skills install to both.agentsand.claude.Make
brainstorming's dispatch step harness-neutral: it named Codex'sfork_turns: "none"only, and now states the intent (a fresh, non-forked subagent) with the Codex and Claude Code forms.Add
disable-model-invocation: trueto all three so Claude Code keeps them manual-only, matchingallow_implicit_invocation: false.In Claude Code, brainstorming passes the caller's model explicitly so a configured subagent default does not override it.
codex-cleanup,delegate-ui-to-claude, andimpeccablekeep their single target; each is tied to one harness.Validation
node scripts/validate-skills— 31 skills, registry, and global instructions pass.skill-registry,global-instructions,audit-global-skills,sjskills-hook) — 40 pass, 0 fail.go test ./internal/...— pass.After merge, a global
sjskillssync installsbrainstorminginto~/.claude/skills;windows-cleanupfollows in projects that select thekicpaprofile.🤖 Generated with Claude Code