diff --git a/clients/tui/__tests__/SkillsTab.test.tsx b/clients/tui/__tests__/SkillsTab.test.tsx index 03638e32..2513d374 100644 --- a/clients/tui/__tests__/SkillsTab.test.tsx +++ b/clients/tui/__tests__/SkillsTab.test.tsx @@ -750,6 +750,50 @@ describe("SkillsTab (#2248)", () => { ); }); + it("says a verification was INCOMPLETE rather than merely failed", async () => { + // `verifySkills` sets `incomplete` so a consumer can tell "not fully + // checked" from a real failure; printing only "Verification FAILED" threw + // that distinction away, and the entries beyond the cap stayed marked `·` + // with nothing explaining why (Copilot). + // + // Truncation is triggered by the BYTE budget rather than the 512-entry one + // so the manifest stays three rows long: a 512-row pane pushes the status + // line off the frame, which would make this assert the test's viewport + // rather than the pane's behaviour. + const big = "x".repeat(6 * 1024 * 1024); + const fat: SkillEntry = { + uri: "skill://fat/SKILL.md", + frontmatter: { name: "fat", description: "Understates its sizes" }, + resources: Array.from({ length: 3 }, (_, i) => ({ + uri: i === 0 ? "skill://fat/SKILL.md" : `skill://fat/f${i}.md`, + digest: CLEAN_DIGEST, + size: 1, + })), + }; + const { lastFrame, stdin } = render( + ({ + result: { contents: [{ uri, text: big }] }, + })), + )} + width={160} + height={40} + focusedPane="list" + />, + ); + stdin.write(ENTER); + await tick(); + await tick(); + const frame = lastFrame() ?? ""; + expect(frame).toContain("Incomplete:"); + expect(frame).toContain("actually served"); + expect(frame).toContain("Verification INCOMPLETE"); + expect(frame).not.toContain("Verification FAILED"); + }); + it("shows a read failure the manifest does not cover", async () => { // A dynamic skill has no manifest rows, so the synthetic read-error row // `verifySkills` records for its own SKILL.md was rendered nowhere and the diff --git a/clients/tui/src/components/SkillsTab.tsx b/clients/tui/src/components/SkillsTab.tsx index b1bf292d..cdbeecff 100644 --- a/clients/tui/src/components/SkillsTab.tsx +++ b/clients/tui/src/components/SkillsTab.tsx @@ -402,6 +402,26 @@ export function SkillsTab({ ))} + {/* ABOVE the manifest, because it explains the list that + follows: only the first N rows were fetched, and the rest + stay marked `·` because nobody looked at them. Below a + 512-row manifest it would be off-screen, which is the same as + absent. `verifySkills` sets `incomplete` precisely so a + consumer can tell "not fully checked" from a real failure + (Copilot). */} + {activeReport?.incomplete && ( + <> + + + Incomplete: + + + + {activeReport.incomplete} + + + )} + Manifest @@ -504,7 +524,9 @@ export function SkillsTab({ : activeReport ? activeReport.ok ? "[Verified — Enter to re-verify]" - : "[Verification FAILED — Enter to re-verify]" + : activeReport.incomplete + ? "[Verification INCOMPLETE — Enter to re-verify]" + : "[Verification FAILED — Enter to re-verify]" : "[Enter to verify digests and frontmatter]"} diff --git a/clients/web/src/test/core/mcp/skillsVerification.test.ts b/clients/web/src/test/core/mcp/skillsVerification.test.ts index de12de11..6f3b9018 100644 --- a/clients/web/src/test/core/mcp/skillsVerification.test.ts +++ b/clients/web/src/test/core/mcp/skillsVerification.test.ts @@ -702,6 +702,38 @@ describe("verifySkills (#2248)", () => { expect(report.files[0].status).toBe("mismatch"); }); + it("reports the capped self-entry under the URI the MANIFEST declared", async () => { + // A manifest may write its self-entry in a normalized-equivalent form. The + // fallback recorded `entry.uri`, so a consumer matching rows against the + // manifest found nothing — while a normalized "extra files" filter + // suppressed it as already covered. The verdict existed in the report and + // appeared nowhere on screen (Copilot). + const declaredSpelling = "skill://huge/x/../SKILL.md"; + const skill: SkillEntry = { + uri: "skill://huge/SKILL.md", + frontmatter: { name: "huge", description: "Listed" }, + resources: [ + ...Array.from({ length: 600 }, (_, i) => ({ + uri: `skill://huge/f${i}.md`, + digest: `sha256:${"a".repeat(64)}`, + size: 1, + })), + { uri: declaredSpelling, digest: `sha256:${"b".repeat(64)}`, size: 1 }, + ], + }; + const readResource = vi.fn(async (uri: string) => ({ + result: { contents: [{ uri, text: "x" }] }, + })); + const client = { readResource } as unknown as InspectorClientProtocol; + const [report] = await verifySkills(client, [skill]); + const self = report.files.find((f) => f.uri === declaredSpelling); + expect(self?.status).toBe("mismatch"); + // …and NOT under the entry's own spelling, which no manifest row carries. + expect(report.files.some((f) => f.uri === "skill://huge/SKILL.md")).toBe( + false, + ); + }); + it("does not truncate a conforming manifest", async () => { // A conforming skill totals at most 16 MiB by definition, so the bound // must never shorten one — otherwise it would trade a hostile-server diff --git a/core/mcp/skillsVerification.ts b/core/mcp/skillsVerification.ts index 370c4bc2..e64f1955 100644 --- a/core/mcp/skillsVerification.ts +++ b/core/mcp/skillsVerification.ts @@ -376,7 +376,13 @@ export async function verifySkills( ); if (declaredSelf && entryBytes !== undefined) { files.push({ - uri: entry.uri, + // The DECLARED spelling, not the entry's. They can differ — a + // manifest may write its self-entry in a normalized-equivalent + // form — and a consumer matching rows against the manifest then + // finds nothing, while a normalized "extra files" filter suppresses + // it as already covered. The result was a verdict that existed in + // the report and appeared nowhere on screen (Copilot). + uri: declaredSelf.uri, ...(await verifySkillResource(declaredSelf, entryBytes)), }); }