diff --git a/clients/web/src/lib/oauthResume.test.ts b/clients/web/src/lib/oauthResume.test.ts index 6977bafa..51b11c9f 100644 --- a/clients/web/src/lib/oauthResume.test.ts +++ b/clients/web/src/lib/oauthResume.test.ts @@ -606,6 +606,7 @@ describe("oauthResume", () => { Prompts: EMPTY_PROMPTS_UI, Resources: EMPTY_RESOURCES_UI, Apps: EMPTY_APPS_UI, + Skills: EMPTY_SKILLS_UI, Tasks: EMPTY_TASKS_UI, Logs: EMPTY_LOGS_UI, Protocol: EMPTY_PROTOCOL_UI, @@ -617,12 +618,39 @@ describe("oauthResume", () => { expect(setters.setPromptsUi).toHaveBeenCalledWith(EMPTY_PROMPTS_UI); expect(setters.setResourcesUi).toHaveBeenCalledWith(EMPTY_RESOURCES_UI); expect(setters.setAppsUi).toHaveBeenCalledWith(EMPTY_APPS_UI); + expect(setters.setSkillsUi).toHaveBeenCalledWith(EMPTY_SKILLS_UI); expect(setters.setTasksUi).toHaveBeenCalledWith(EMPTY_TASKS_UI); expect(setters.setLogsUi).toHaveBeenCalledWith(EMPTY_LOGS_UI); expect(setters.setProtocolUi).toHaveBeenCalledWith(EMPTY_PROTOCOL_UI); expect(setters.setNetworkUi).toHaveBeenCalledWith(EMPTY_NETWORK_UI); }); + it("restoreTabUiFromSnapshot restores a SAVED Skills selection, not the default", () => { + // The other cases restore each tab's EMPTY value, so a bug that always + // wrote the default would pass them. This one saves a real selection. + const saved = { + ...EMPTY_SKILLS_UI, + selectedSkillUri: "skill://data-analysis/SKILL.md", + search: "analysis", + }; + const setSkillsUi = vi.fn(); + restoreTabUiFromSnapshot( + { Skills: saved }, + { + setToolsUi: vi.fn(), + setPromptsUi: vi.fn(), + setResourcesUi: vi.fn(), + setAppsUi: vi.fn(), + setSkillsUi, + setTasksUi: vi.fn(), + setLogsUi: vi.fn(), + setProtocolUi: vi.fn(), + setNetworkUi: vi.fn(), + }, + ); + expect(setSkillsUi).toHaveBeenCalledWith(saved); + }); + it("restoreTabUiFromSnapshot falls back to EMPTY state for undefined tab values", () => { const setters = { setToolsUi: vi.fn(), @@ -641,6 +669,7 @@ describe("oauthResume", () => { Prompts: undefined, Resources: undefined, Apps: undefined, + Skills: undefined, Tasks: undefined, Logs: undefined, Protocol: undefined, @@ -652,6 +681,7 @@ describe("oauthResume", () => { expect(setters.setPromptsUi).toHaveBeenCalledWith(EMPTY_PROMPTS_UI); expect(setters.setResourcesUi).toHaveBeenCalledWith(EMPTY_RESOURCES_UI); expect(setters.setAppsUi).toHaveBeenCalledWith(EMPTY_APPS_UI); + expect(setters.setSkillsUi).toHaveBeenCalledWith(EMPTY_SKILLS_UI); expect(setters.setTasksUi).toHaveBeenCalledWith(EMPTY_TASKS_UI); expect(setters.setLogsUi).toHaveBeenCalledWith(EMPTY_LOGS_UI); expect(setters.setProtocolUi).toHaveBeenCalledWith(EMPTY_PROTOCOL_UI); diff --git a/clients/web/src/test/core/mcp/skills.test.ts b/clients/web/src/test/core/mcp/skills.test.ts index 8b085370..8686a171 100644 --- a/clients/web/src/test/core/mcp/skills.test.ts +++ b/clients/web/src/test/core/mcp/skills.test.ts @@ -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", diff --git a/core/mcp/skills.ts b/core/mcp/skills.ts index d7378d86..e13de763 100644 --- a/core/mcp/skills.ts +++ b/core/mcp/skills.ts @@ -137,7 +137,27 @@ export function normalizeSkillUri(uri: string): string | undefined { } catch { return undefined; } - return parsed.pathname.startsWith("/") ? parsed.href : undefined; + if (!parsed.pathname.startsWith("/")) return undefined; + return canonicalizePercentEncoding(parsed.href); +} + +/** + * The two percent-encoding normalizations RFC 3986 §6.2.2 calls for and the + * URL parser does **not** do: decode an escape that stands for an *unreserved* + * character, and upper-case the hex of every escape that remains. + * + * `URL.href` leaves `%72eference.md` encoded, so without this a server echoing + * an RFC-equivalent form of the URI we asked for would be rejected by + * `onReadSkillFile` as a different resource, and an encoded skill-name segment + * would produce a false `name-path-mismatch`. Both are the tool calling a + * conforming server wrong, which is the failure mode this module works hardest + * to avoid. + */ +function canonicalizePercentEncoding(value: string): string { + return value.replace(/%[0-9a-fA-F]{2}/g, (escape) => { + const char = String.fromCharCode(Number.parseInt(escape.slice(1), 16)); + return /[A-Za-z0-9\-._~]/.test(char) ? char : escape.toUpperCase(); + }); } /**