Greptile Summary

This PR fixes a layering bug where skills passing shouldIncludeSkill() could still be silently omitted from <available_skills> because a second visibility filter was applied independently in the formatter and workspace layers. The fix moves prompt-visibility policy upstream into loadSkillEntries (via the new SkillExposure.includeInAvailableSkillsPrompt field set to true for all freshly loaded entries) and makes the formatters pure renderers.

The legacy fallback in isSkillVisibleInAvailableSkillsPrompt correctly preserves existing behavior for entries without exposure metadata, and the test coverage is solid across compact/full-format budget tiers. One P2 observation: SkillExposure.includeInRuntimeRegistry and SkillExposure.userInvocable are written during loading but no runtime decision-making code reads them — shouldIncludeSkill() and command-specs.ts continue to use independent paths — so these fields currently carry no behavioral weight.

Confidence Score: 5/5

Safe to merge — the only findings are P2 style/scaffolding concerns with no runtime impact.

All P0/P1 criteria are met: the core behavioral fix is correct and well-tested, legacy compatibility is preserved via the isSkillVisibleInAvailableSkillsPrompt fallback chain, and formatters are now correctly policy-free. The sole P2 note about unused SkillExposure fields does not affect any live code path.

src/agents/skills/types.ts and src/agents/skills/workspace.ts — the includeInRuntimeRegistry and userInvocable fields on SkillExposure are set but not read by runtime decision logic.

This is a comment left during a code review.
Path: src/agents/skills/workspace.ts
Line: 488-494

Comment:
**`includeInRuntimeRegistry` is set but never read**

`exposure.includeInRuntimeRegistry` is always written as `true` here, but no runtime code reads it to make decisions — `shouldIncludeSkill()` in `config.ts` handles actual runtime gating independently. A future caller setting this to `false` on a manually constructed `SkillEntry` would see no effect. The field name implies it gates runtime registry inclusion, but currently it does not.

Similarly, `exposure.userInvocable` mirrors `invocation.userInvocable`, but `command-specs.ts` (line 85) reads `entry.invocation?.userInvocable`, not `entry.exposure?.userInvocable`. If these two fields ever drift on a manually composed entry, `exposure.userInvocable` would be silently ignored.

Consider either wiring these fields into their respective decision paths or documenting them as reserved/forward-looking scaffolding to prevent misuse.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (2): Last reviewed commit: "fix(skills): add changelog entry for pro..." | Re-trigger Greptile}

@greptileai review

Merged via squash.

• Prepared head SHA: 2b48b3a4559696b1535298477d7aa487228d1ad5
• Merge commit: fd71bc04ec2350fd6b0bc77ef684d6c2db275e9c

Thanks @KimGLee!