Follow-up from the review of #2293 (which closes #2248).
normalizeSkillUri guards with parsed.pathname.startsWith("/"), and for a URI whose authority carries the whole identity — skill://data-analysis — URL.pathname is the empty string. So the function returns undefined for a URI that is not malformed.
Where it shows. The Directory section on the Skills screen refuses a child that is not a direct child of the directory being read. When the offending entry is a bare skill root echoed back, normalizeSkillUri returns undefined, inRoot is therefore false, and the row is labelled "(outside this skill)" rather than "(not a direct child)".
The entry is still refused, so this is a wrong label, not a hole — but it points a reader at the wrong defect, which is the specific failure mode the two labels were separated to avoid.
Reproduce
Read skill://<name> as a directory from a server that lists its own root among the children. The row is shown, non-navigable, and named "outside this skill" while sitting squarely inside it.
Why it was not fixed in #2293
It was found while writing the test for the direct-child contract, in the last review round of the PR. normalizeSkillUri is load-bearing for every URI comparison in core/mcp/skills.ts — containment, manifest matching, the self-entry lookup, contentsFor — so relaxing its guard is a change that wants its own review rather than a late amendment. The test in #2293 exercises the self-echo one level down, where the URI parses, and the discrepancy is called out in the code rather than worked around silently.
What to decide
Whether a path-less skill://name is a legal skill URI at all. SEP-2640 skill URIs in practice always carry /SKILL.md, so it may be that returning undefined is right and only the label should change — the caller could distinguish "unparseable" from "outside the root". Settle that first; the code change is small either way.
Follow-up from the review of #2293 (which closes #2248).
normalizeSkillUriguards withparsed.pathname.startsWith("/"), and for a URI whose authority carries the whole identity —skill://data-analysis—URL.pathnameis the empty string. So the function returnsundefinedfor a URI that is not malformed.Where it shows. The Directory section on the Skills screen refuses a child that is not a direct child of the directory being read. When the offending entry is a bare skill root echoed back,
normalizeSkillUrireturnsundefined,inRootis therefore false, and the row is labelled "(outside this skill)" rather than "(not a direct child)".The entry is still refused, so this is a wrong label, not a hole — but it points a reader at the wrong defect, which is the specific failure mode the two labels were separated to avoid.
Reproduce
Read
skill://<name>as a directory from a server that lists its own root among the children. The row is shown, non-navigable, and named "outside this skill" while sitting squarely inside it.Why it was not fixed in #2293
It was found while writing the test for the direct-child contract, in the last review round of the PR.
normalizeSkillUriis load-bearing for every URI comparison incore/mcp/skills.ts— containment, manifest matching, the self-entry lookup,contentsFor— so relaxing its guard is a change that wants its own review rather than a late amendment. The test in #2293 exercises the self-echo one level down, where the URI parses, and the discrepancy is called out in the code rather than worked around silently.What to decide
Whether a path-less
skill://nameis a legal skill URI at all. SEP-2640 skill URIs in practice always carry/SKILL.md, so it may be that returningundefinedis right and only the label should change — the caller could distinguish "unparseable" from "outside the root". Settle that first; the code change is small either way.