Context
Deferred non-blocking follow-up from the #257 review (commit 851b0bc, item #4). Tracking it so it isn't lost in the commit message.
Two near-duplicate skill-name → path resolvers exist after #257:
resolvePath in forge-core/tools/builtins/read_skill.go (tries skills/<name>.md, skills/<name>/SKILL.md, underscore→hyphen variant, then a frontmatter-name index)
SkillDir / SafeSkillJoin in forge-cli/tools/run_skill_script.go (resolves a skill's own directory for skill-relative reads/execution)
They encode overlapping resolution logic. Divergence risks bugs where read_skill resolves a skill but run_skill_script can't (or resolves it to a different path).
Proposal
Extract a single shared skill-name → directory/path resolver and have both call it.
The caveat that makes this non-trivial
Flat single-file skills (skills/<name>.md) have no companion directory, and their frontmatter name may differ from the file/dir name. The unified resolver must:
- resolve both layouts (flat
skills/<name>.md and dir skills/<name>/SKILL.md)
- key on BOTH frontmatter name and dir/file name (as
resolvePath already does)
- define a well-specified answer for "skill directory" of a flat skill (there is none — callers doing skill-relative file ops must handle that case explicitly rather than getting a bogus dir)
Acceptance
- One shared resolver;
read_skill and run_skill_script both use it
- Flat-skill frontmatter/name divergence handled and tested
- No regression in existing read_skill / run_skill_script resolution tests
Depends on #257 merging first (the code lives on that branch). Related: #251, #257
Context
Deferred non-blocking follow-up from the #257 review (commit 851b0bc, item #4). Tracking it so it isn't lost in the commit message.
Two near-duplicate skill-name → path resolvers exist after #257:
resolvePathinforge-core/tools/builtins/read_skill.go(triesskills/<name>.md,skills/<name>/SKILL.md, underscore→hyphen variant, then a frontmatter-name index)SkillDir/SafeSkillJoininforge-cli/tools/run_skill_script.go(resolves a skill's own directory for skill-relative reads/execution)They encode overlapping resolution logic. Divergence risks bugs where
read_skillresolves a skill butrun_skill_scriptcan't (or resolves it to a different path).Proposal
Extract a single shared skill-name → directory/path resolver and have both call it.
The caveat that makes this non-trivial
Flat single-file skills (
skills/<name>.md) have no companion directory, and their frontmatternamemay differ from the file/dir name. The unified resolver must:skills/<name>.mdand dirskills/<name>/SKILL.md)resolvePathalready does)Acceptance
read_skillandrun_skill_scriptboth use itDepends on #257 merging first (the code lives on that branch). Related: #251, #257