fix: address Copilot review round 11 on #2251

- skills.ts: apply the two RFC 3986 §6.2.2 percent-encoding normalizations
  the URL parser does not — decode an escape standing for an unreserved
  character, and upper-case the hex of every escape that remains. `URL.href`
  leaves `%72eference.md` encoded, so a server echoing an RFC-equivalent
  form of the URI we asked for was rejected by `onReadSkillFile` as a
  different resource, and an encoded name segment produced a false
  `name-path-mismatch`. Both are the tool calling a conforming server wrong.
- oauthResume.test: assert the Skills restore path. It was wired into the
  setters but never checked, and the comprehensive present-value and
  undefined-fallback cases omitted Skills entirely. Adds it to both, plus a
  case restoring a REAL saved selection rather than the EMPTY value — a bug
  that always wrote the default would have passed the others.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018kfrH3uRNUM4JAk1reKtyY
Signed-off-by: cliffhall <cliff@futurescale.com>
This commit is contained in:
cliffhall 2026-09-05 00:21:00 -04:00
commit d0302078d0

View file

@ -166,6 +166,24 @@ describe("normalizeSkillUri", () => {
expect(normalizeSkillUri("skill:demo/SKILL.md")).toBeUndefined();
});
it("decodes escapes that stand for unreserved characters", () => {
// `URL.href` leaves these encoded, so without canonicalizing them a server
// echoing an RFC-equivalent form would be treated as a different resource
// and an encoded name segment would produce a false name/path mismatch.
expect(normalizeSkillUri("skill://demo/%72eference.md")).toBe(
"skill://demo/reference.md",
);
expect(skillNameFromUri("skill://%64emo/SKILL.md")).toBe("demo");
});
it("upper-cases the hex of escapes that must stay encoded", () => {
// A space is not unreserved, so it stays escaped — but in one spelling, so
// two RFC-equivalent URIs compare equal.
expect(normalizeSkillUri("skill://demo/a%2fb.md")).toBe(
normalizeSkillUri("skill://demo/a%2Fb.md"),
);
});
it("leaves an already-normal URI alone", () => {
expect(normalizeSkillUri("skill://demo/SKILL.md")).toBe(
"skill://demo/SKILL.md",