fix(skills): handle failed settings API responses (#3823)

* fix skill settings error handling

* chore: format skills api unit test

* Harden skills API fallback messages

* test: describe rejected skill disable accurately

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
Minh Vu 2026-09-24 03:29:49 +02:00 committed by GitHub
parent 731e8c6eee
commit a0d9744d5a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 138 additions and 2 deletions

View File

@ -0,0 +1,49 @@
import { expect, test } from "@playwright/test";
import { mockLangGraphAPI } from "./utils/mock-api";
test.describe("Skill gallery", () => {
test("shows a failure and keeps the toggle state when disabling a skill is rejected", async ({
page,
}) => {
mockLangGraphAPI(page, {
skills: [
{
name: "toggle-skill",
description: "Test skill",
category: "public",
enabled: true,
},
],
});
void page.route("**/api/suggestions/config", (route) => {
return route.fulfill({
status: 200,
contentType: "application/json",
body: JSON.stringify({ enabled: false }),
});
});
void page.route("**/api/skills/toggle-skill", (route) => {
if (route.request().method() === "PUT") {
return route.fulfill({
status: 403,
contentType: "application/json",
body: JSON.stringify({ detail: "Skill toggle denied" }),
});
}
return route.fallback();
});
await page.goto("/workspace/capabilities?tab=skills");
const toggle = page.getByRole("switch", { name: /toggle-skill/ });
await expect(page.getByText("toggle-skill")).toBeVisible();
await expect(toggle).toBeChecked();
await toggle.click();
await expect(page.getByText("Skill toggle denied")).toBeVisible();
await expect(toggle).toBeChecked();
});
});

View File

@ -12,15 +12,21 @@ import { fetch as fetcher } from "@/core/api/fetcher";
import {
formatSkillSecurityFindings,
SkillRequestError,
enableSkill,
loadSkills,
uploadSkillArchive,
} from "@/core/skills/api";
const mockedFetch = rs.mocked(fetcher);
function jsonResponse(status: number, body: unknown): Response {
function jsonResponse(
status: number,
body: unknown,
statusText = status >= 400 ? "Error" : "OK",
): Response {
return new Response(JSON.stringify(body), {
status,
statusText: status >= 400 ? "Error" : "OK",
statusText,
headers: { "Content-Type": "application/json" },
});
}
@ -151,4 +157,85 @@ describe("skills api", () => {
"... and 2 more",
]);
});
test("loads skills from the gateway", async () => {
mockedFetch.mockResolvedValueOnce(
jsonResponse(200, {
skills: [
{
name: "data-analysis",
description: "Analyze data",
category: "public",
license: "MIT",
enabled: true,
},
],
}),
);
await expect(loadSkills()).resolves.toEqual([
{
name: "data-analysis",
description: "Analyze data",
category: "public",
license: "MIT",
enabled: true,
},
]);
expect(mockedFetch).toHaveBeenCalledWith("/backend/api/skills");
});
test("throws backend detail when loading skills fails", async () => {
mockedFetch.mockResolvedValueOnce(
jsonResponse(403, { detail: "Skills are disabled for this user" }),
);
await expect(loadSkills()).rejects.toThrow(
"Skills are disabled for this user",
);
});
test("falls back to HTTP status when loading skills fails without detail", async () => {
mockedFetch.mockResolvedValueOnce(jsonResponse(503, {}, ""));
await expect(loadSkills()).rejects.toThrow("HTTP 503");
});
test("updates a skill enabled flag", async () => {
mockedFetch.mockResolvedValueOnce(
jsonResponse(200, { success: true, skill_name: "data-analysis" }),
);
await expect(enableSkill("data-analysis", false)).resolves.toMatchObject({
success: true,
skill_name: "data-analysis",
});
expect(mockedFetch).toHaveBeenCalledWith(
"/backend/api/skills/data-analysis",
{
method: "PUT",
headers: {
"Content-Type": "application/json",
},
body: JSON.stringify({
enabled: false,
}),
},
);
});
test("throws backend detail when updating a skill fails", async () => {
mockedFetch.mockResolvedValueOnce(
jsonResponse(403, { detail: "Skill toggle denied" }),
);
await expect(enableSkill("foo", false)).rejects.toThrow(
"Skill toggle denied",
);
});
test("falls back to HTTP status when updating a skill fails without detail", async () => {
mockedFetch.mockResolvedValueOnce(jsonResponse(409, {}, ""));
await expect(enableSkill("foo", false)).rejects.toThrow("HTTP 409");
});
});