Problem Statement
crates/core/src/skills.rs uses a bundled_skills! macro that emits multiple parallel const arrays — SKILL_REGISTRY, BUNDLED_SKILLS, SKILL_CATEGORY_MAP, plus filtered subsets like DEFAULT_ENABLED_SKILLS and HIDDEN_SKILLS. They are projections of one shape, but lookups happen by string name across them in different places. A typo or drift between arrays is silently miss-handled. Adding a metadata field requires editing every projection.
Solution
Replace the parallel arrays with a single &'static [SkillMetadata] carrying every field per skill, plus a lazily-built HashMap<&'static str, &'static SkillMetadata> for lookup. Callers iterate the slice or query the map; no parallel arrays.
User Stories
- As a Borg contributor, I want bundled skill metadata to live in a single source of truth, so that adding a field (category, default-enabled, hidden, requires) is one struct edit.
- As a Borg contributor, I want skill lookup to go through one indexed map, so that a typo or drift between parallel arrays is impossible.
- As a Borg contributor, I want invariants over the skill list (every skill has a category, names are unique, every
default_enabled skill is registered) asserted in tests, so that bundled-skill drift is caught in CI.
- As a Borg user, I want the same skills available with the same defaults after the refactor, so that nothing changes at runtime.
Implementation Decisions
- Define a
SkillMetadata struct: name: &'static str, content: &'static str (include_str!), category, default_enabled: bool, hidden: bool, plus existing requires metadata (bins, env vars).
- Replace the parallel const arrays with a single
&'static [SkillMetadata] (or OnceCell<Vec<SkillMetadata>> if any field needs runtime construction).
- Build a
OnceCell<HashMap<&'static str, &'static SkillMetadata>> for name-keyed lookup. Callers stop searching parallel arrays.
- Update consumers in
crates/core/src/skills.rs to query the unified slice/map.
- User skills at
~/.borg/skills/<name>/ continue to override by name with identical semantics.
- The
bundled_skills! macro either goes away or becomes a thin sugar that emits one SkillMetadata literal per skill.
Testing Decisions
A good test asserts an invariant over the unified slice that the compiler cannot enforce.
- Invariant tests over the slice: unique names, every entry has non-empty content, every
default_enabled = true entry is reachable via lookup, no skill name collides with a built-in tool name.
- Existing skill-loading tests continue to cover override behavior from
~/.borg/skills/.
- Per-array tests that previously asserted on individual projections collapse into one.
- Prior art: existing tests in
crates/core/src/skills.rs.
Out of Scope
- Adding new bundled skills or removing existing ones.
- Changing the user-skills override mechanism or the on-disk skill format.
- Reworking progressive skill loading or token budgets.
Further Notes
- Smallest refactor in this group; useful as a contained, low-risk change.
- If any parallel array currently encodes ordering (e.g., display order in
/list), preserve it explicitly as a field on SkillMetadata rather than relying on slice order.
Problem Statement
crates/core/src/skills.rsuses abundled_skills!macro that emits multiple parallel const arrays —SKILL_REGISTRY,BUNDLED_SKILLS,SKILL_CATEGORY_MAP, plus filtered subsets likeDEFAULT_ENABLED_SKILLSandHIDDEN_SKILLS. They are projections of one shape, but lookups happen by string name across them in different places. A typo or drift between arrays is silently miss-handled. Adding a metadata field requires editing every projection.Solution
Replace the parallel arrays with a single
&'static [SkillMetadata]carrying every field per skill, plus a lazily-builtHashMap<&'static str, &'static SkillMetadata>for lookup. Callers iterate the slice or query the map; no parallel arrays.User Stories
default_enabledskill is registered) asserted in tests, so that bundled-skill drift is caught in CI.Implementation Decisions
SkillMetadatastruct:name: &'static str,content: &'static str(include_str!),category,default_enabled: bool,hidden: bool, plus existingrequiresmetadata (bins, env vars).&'static [SkillMetadata](orOnceCell<Vec<SkillMetadata>>if any field needs runtime construction).OnceCell<HashMap<&'static str, &'static SkillMetadata>>for name-keyed lookup. Callers stop searching parallel arrays.crates/core/src/skills.rsto query the unified slice/map.~/.borg/skills/<name>/continue to override by name with identical semantics.bundled_skills!macro either goes away or becomes a thin sugar that emits oneSkillMetadataliteral per skill.Testing Decisions
A good test asserts an invariant over the unified slice that the compiler cannot enforce.
default_enabled = trueentry is reachable via lookup, no skill name collides with a built-in tool name.~/.borg/skills/.crates/core/src/skills.rs.Out of Scope
Further Notes
/list), preserve it explicitly as a field onSkillMetadatarather than relying on slice order.