Skip to content

Commit 352d1bd

Browse files
committed
fix(skills): accept skill.md casefold and diagnose missing SKILL.md
Workspace skill entries must live at skills/<id>/SKILL.md. Discover skill.md case-insensitively with a rename hint, and when the skill directory exists without that entry fall back to G with reason missing-SKILL.md so layout mistakes are obvious on resolve.
1 parent b9c3e26 commit 352d1bd

3 files changed

Lines changed: 143 additions & 13 deletions

File tree

‎hooks/_runtime/skill-resolution.cjs‎

Lines changed: 84 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,62 @@ function fileExists(filePath, fsImpl) {
196196
}
197197
}
198198

199+
/**
200+
* Canonical skill entry is <skillDir>/SKILL.md (Agent Skills / Claude shape).
201+
* On case-insensitive FS, accept skill.md and return the actual path found.
202+
* @returns {{ path: string|null, caseFolded: boolean, dirExists: boolean, hint: string }}
203+
*/
204+
function findSkillMarkdown(skillDir, fsImpl = fs) {
205+
const preferred = path.join(skillDir, 'SKILL.md')
206+
if (!fsImpl.existsSync(skillDir)) {
207+
return { path: null, caseFolded: false, dirExists: false, hint: 'skill-dir-missing' }
208+
}
209+
let isDir = false
210+
try {
211+
isDir = fsImpl.statSync(skillDir).isDirectory()
212+
} catch {
213+
return { path: null, caseFolded: false, dirExists: false, hint: 'skill-dir-missing' }
214+
}
215+
if (!isDir) {
216+
return { path: null, caseFolded: false, dirExists: false, hint: 'skill-dir-not-directory' }
217+
}
218+
// Prefer exact SKILL.md when readdir reports it (case-sensitive correctness)
219+
let names = []
220+
try {
221+
names = fsImpl.readdirSync(skillDir)
222+
} catch {
223+
return { path: null, caseFolded: false, dirExists: true, hint: 'skill-dir-unreadable' }
224+
}
225+
if (names.includes('SKILL.md')) {
226+
const p = path.join(skillDir, 'SKILL.md')
227+
if (fileExists(p, fsImpl)) return { path: p, caseFolded: false, dirExists: true, hint: '' }
228+
}
229+
const folded = names.find(name => String(name).toLowerCase() === 'skill.md')
230+
if (folded) {
231+
const p = path.join(skillDir, folded)
232+
if (fileExists(p, fsImpl)) {
233+
return {
234+
path: p,
235+
caseFolded: folded !== 'SKILL.md',
236+
dirExists: true,
237+
hint: folded !== 'SKILL.md' ? `rename-${folded}-to-SKILL.md` : ''
238+
}
239+
}
240+
}
241+
// last resort: existsSync(SKILL.md) on case-insensitive FS
242+
if (fileExists(preferred, fsImpl)) {
243+
return { path: preferred, caseFolded: true, dirExists: true, hint: 'prefer-exact-SKILL.md' }
244+
}
245+
return {
246+
path: null,
247+
caseFolded: false,
248+
dirExists: true,
249+
hint: names.length
250+
? `missing-SKILL.md-found:${names.join(',')}`
251+
: 'missing-SKILL.md-empty-dir'
252+
}
253+
}
254+
199255
/**
200256
* @returns {{ trace: object, content: string|null }}
201257
*/
@@ -226,9 +282,12 @@ function resolveSkillRead(skillId, options = {}) {
226282
}
227283

228284
const id = String(skillId).trim()
229-
const gPath = path.join(globalSkillsRoot, id, 'SKILL.md')
230-
const wPath = workspaceSkillsRoot ? path.join(workspaceSkillsRoot, id, 'SKILL.md') : null
231-
const gExists = fileExists(gPath, fsImpl) && isUnderPhysical(globalSkillsRoot, gPath, fsImpl)
285+
const gHit = findSkillMarkdown(path.join(globalSkillsRoot, id), fsImpl)
286+
const wDir = workspaceSkillsRoot ? path.join(workspaceSkillsRoot, id) : null
287+
const wHit = wDir ? findSkillMarkdown(wDir, fsImpl) : { path: null, caseFolded: false, dirExists: false, hint: 'no-workspace-root' }
288+
const gPath = gHit.path || path.join(globalSkillsRoot, id, 'SKILL.md')
289+
const wPath = wHit.path || (wDir ? path.join(wDir, 'SKILL.md') : null)
290+
const gExists = Boolean(gHit.path) && isUnderPhysical(globalSkillsRoot, gPath, fsImpl)
232291
const reserved = isReservedSkillId(id)
233292

234293
const finishG = (securityDecision, reasonCode, fallbackReason, skippedByUser = false) => {
@@ -289,26 +348,26 @@ function resolveSkillRead(skillId, options = {}) {
289348
)
290349
}
291350

292-
if (wPath && fileExists(wPath, fsImpl)) {
293-
if (!isUnderPhysical(workspaceSkillsRoot, wPath, fsImpl)) {
351+
if (wHit.path && fileExists(wHit.path, fsImpl)) {
352+
if (!isUnderPhysical(workspaceSkillsRoot, wHit.path, fsImpl)) {
294353
return finishG('rejected-path', 'symlink-escape', 'symlink-escape')
295354
}
296355
let stat
297356
try {
298-
stat = fsImpl.statSync(wPath)
357+
stat = fsImpl.statSync(wHit.path)
299358
} catch {
300359
return finishG('rejected-path', 'stat-failed', 'stat-failed')
301360
}
302361
if (stat.size > (options.maxBytes || MAX_SKILL_BYTES)) {
303362
return finishG('rejected-oversize', 'oversize', 'oversize')
304363
}
305-
const fileText = fsImpl.readFileSync(wPath, 'utf8')
364+
const fileText = fsImpl.readFileSync(wHit.path, 'utf8')
306365
const weaken = detectWeaken(fileText)
307366
if (weaken) {
308367
return finishG('rejected-weaken', 'weaken-pattern', `weaken:${weaken}`)
309368
}
310369
// relative reference guard: optional listed files under skill dir only
311-
const skillDir = path.dirname(wPath)
370+
const skillDir = path.dirname(wHit.path)
312371
const refHits = fileText.match(/(?:scripts|references)\/[A-Za-z0-9._/-]+/g) || []
313372
for (const rel of refHits) {
314373
const target = path.resolve(skillDir, rel)
@@ -322,21 +381,32 @@ function resolveSkillRead(skillId, options = {}) {
322381
content: options.includeContent === false ? null : fileText,
323382
trace: baseTrace(id, localOpts, {
324383
selectedLayer: 'workspace',
325-
selectedPath: portable(wPath),
384+
selectedPath: portable(wHit.path),
326385
digest,
327386
contentBytes,
328387
coversGlobal: gExists,
329388
securityDecision: 'accepted',
330-
reasonCode: 'workspace-accepted',
331-
fallbackReason: '',
389+
reasonCode: wHit.caseFolded ? 'workspace-accepted-casefold' : 'workspace-accepted',
390+
fallbackReason: wHit.hint || '',
332391
distribution: readDistribution(skillDir, fsImpl),
333-
wPath: portable(wPath),
392+
wPath: portable(wHit.path),
334393
gPath: portable(gPath),
335-
reserved: false
394+
reserved: false,
395+
skillFileName: path.basename(wHit.path)
336396
})
337397
}
338398
}
339399

400+
// Directory exists but SKILL.md missing/wrong name — clearer diagnostics
401+
if (wHit.dirExists && !wHit.path) {
402+
return finishG(
403+
'not-applicable',
404+
'missing-SKILL.md',
405+
wHit.hint || 'missing-SKILL.md',
406+
false
407+
)
408+
}
409+
340410
return finishG('not-applicable', gExists ? 'w-absent' : 'missing', gExists ? 'w-absent' : 'missing')
341411
}
342412

@@ -442,5 +512,6 @@ module.exports = {
442512
assertApplyDestinationNotWorkspaceSkills,
443513
isWorkspaceSkillPath,
444514
isUnderPhysical,
515+
findSkillMarkdown,
445516
sha256Text
446517
}

‎scripts/lib/cli-execution-commands.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,12 @@ function buildCliExecutionCommands(ctx) {
374374
console.log(` ${c.cyan('covers'.padEnd(16))} ${plan.workspaceCoverCount}`)
375375
for (const trace of plan.traces) {
376376
console.log(` ${c.cyan(trace.skillId.padEnd(16))} ${trace.selectedLayer} ${trace.securityDecision} ${trace.selectedPath || ''}`)
377+
if (trace.reasonCode === 'missing-SKILL.md' || (trace.fallbackReason && String(trace.fallbackReason).includes('SKILL'))) {
378+
console.log(c.yellow(` hint: put file at .devcodex/workspace/skills/${trace.skillId}/SKILL.md (filename must be SKILL.md, not skill.md)`))
379+
}
380+
if (trace.reasonCode === 'workspace-accepted-casefold' || (trace.fallbackReason && String(trace.fallbackReason).startsWith('rename-'))) {
381+
console.log(c.yellow(` hint: rename to exact SKILL.md for cross-platform compatibility (${trace.fallbackReason || 'casefold'})`))
382+
}
377383
}
378384
console.log()
379385
}

‎scripts/test-skill-resolve.js‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,57 @@ function testClassifyAndApplyGuard() {
182182
})
183183
}
184184

185+
function testLowercaseSkillMd() {
186+
withTemp(base => {
187+
const workspaceRoot = path.join(base, 'ws')
188+
const home = path.join(base, 'home')
189+
const gRoot = path.join(home, '.agents', 'skills')
190+
const wRoot = path.join(workspaceRoot, '.devcodex', 'workspace', 'skills')
191+
fs.mkdirSync(path.join(workspaceRoot, '.devcodex'), { recursive: true })
192+
fs.writeFileSync(
193+
path.join(workspaceRoot, '.devcodex', 'layout.json'),
194+
JSON.stringify({ version: 1, mode: 'workspace-namespace' })
195+
)
196+
const dir = path.join(wRoot, 'demo')
197+
fs.mkdirSync(dir, { recursive: true })
198+
fs.writeFileSync(path.join(dir, 'skill.md'), '# lowercase entry\n', 'utf8')
199+
const r = resolveSkillRead('demo', {
200+
cwd: workspaceRoot,
201+
workspaceRoot,
202+
env: { USERPROFILE: home, HOME: home, DEVCODEX_GLOBAL_SKILLS_ROOT: gRoot }
203+
})
204+
assert.strictEqual(r.trace.selectedLayer, 'workspace')
205+
assert.ok(
206+
r.trace.reasonCode === 'workspace-accepted' || r.trace.reasonCode === 'workspace-accepted-casefold'
207+
)
208+
assert.ok(String(r.trace.selectedPath).toLowerCase().endsWith('skill.md'))
209+
})
210+
}
211+
212+
function testMissingSkillMdHint() {
213+
withTemp(base => {
214+
const workspaceRoot = path.join(base, 'ws')
215+
const home = path.join(base, 'home')
216+
const gRoot = path.join(home, '.agents', 'skills')
217+
const wRoot = path.join(workspaceRoot, '.devcodex', 'workspace', 'skills')
218+
fs.mkdirSync(path.join(workspaceRoot, '.devcodex'), { recursive: true })
219+
fs.writeFileSync(
220+
path.join(workspaceRoot, '.devcodex', 'layout.json'),
221+
JSON.stringify({ version: 1, mode: 'workspace-namespace' })
222+
)
223+
fs.mkdirSync(path.join(wRoot, 'broken'), { recursive: true })
224+
fs.writeFileSync(path.join(wRoot, 'broken', 'README.md'), '# wrong file\n', 'utf8')
225+
writeSkill(gRoot, 'broken', '# g fallback\n')
226+
const r = resolveSkillRead('broken', {
227+
cwd: workspaceRoot,
228+
workspaceRoot,
229+
env: { USERPROFILE: home, HOME: home, DEVCODEX_GLOBAL_SKILLS_ROOT: gRoot }
230+
})
231+
assert.strictEqual(r.trace.selectedLayer, 'global')
232+
assert.strictEqual(r.trace.reasonCode, 'missing-SKILL.md')
233+
})
234+
}
235+
185236
function testOversize() {
186237
withTemp(base => {
187238
const workspaceRoot = path.join(base, 'ws')
@@ -274,6 +325,8 @@ function main() {
274325
testBasicLayers()
275326
testReservedBlocksW()
276327
testKillSwitchAndWeaken()
328+
testLowercaseSkillMd()
329+
testMissingSkillMdHint()
277330
testOversize()
278331
testLayoutDisabled()
279332
testClassifyAndApplyGuard()

0 commit comments

Comments
 (0)