From 1f4ab5571f7cf4b62a334cc5bdf6e66750f93639 Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 4 Nov 2026 16:27:49 -0600 Subject: [PATCH] fix(evals): inspect authored skills through canonical editor Co-Authored-By: Paperclip --- tests/runner-e2e/CHAT-FEATURES.md | 10 +++++++ .../runner-e2e/chat-catalog-descriptors.json | 6 +++-- .../integrator-skill-authoring-v2.test.ts | 22 +++++++++------- .../integrator-skills.test.ts | 6 ++--- .../chat-catalog-domains/integrator-skills.ts | 26 +++++++++++++------ 5 files changed, 48 insertions(+), 22 deletions(-) diff --git a/tests/runner-e2e/CHAT-FEATURES.md b/tests/runner-e2e/CHAT-FEATURES.md index 73efea89367..33b322eedb2 100644 --- a/tests/runner-e2e/CHAT-FEATURES.md +++ b/tests/runner-e2e/CHAT-FEATURES.md @@ -239,6 +239,16 @@ Domain descriptors declare real setup/actions/independent grading and exact boun Only `steering.preserve-draft` and `tasks.attachment-brief` may currently declare one `queryAttachments` receipt. The canonical typed query remains unchanged; the production composer appends its exact attachment reference. Before and after send, the harness independently reads the public same-source receipt and actual bytes, verifies SHA-256 and human attribution, and binds it to the real canonical comment. Extra markdown, text changes, foreign receipts and hidden followups fail. Attachment fingerprints pin both production composer source hashes as data, without importing the UI graph. +### Authored skill inspection + +`skills.author-skill` v3 follows the normal library card to the exact saved skill. +It accepts the canonical company-scoped detail route or the exact Studio route. +The detail view must expose its normal Edit control. Both paths must show the +editable saved checklist, survive reload, and match the same public current +version. The receipt identifies the actual editor surface. The original case +clock, query, and one-human/one-native limits remain unchanged. Older failed +attempts are retained; component controls do not establish a native pass. + ### Six prerequisite probes are not completed journeys The following six definitions retain explicit coverage gaps and must not be diff --git a/tests/runner-e2e/chat-catalog-descriptors.json b/tests/runner-e2e/chat-catalog-descriptors.json index cf25c60e17a..c73285ce97d 100644 --- a/tests/runner-e2e/chat-catalog-descriptors.json +++ b/tests/runner-e2e/chat-catalog-descriptors.json @@ -5081,9 +5081,11 @@ "chat-catalog-domains/integrator-skill-versions.ts", "chat-catalog-domains/identity-support.ts", "../../ui/src/components/MarkdownEditor.tsx", - "../../ui/src/pages/SkillStudio.tsx" + "../../ui/src/pages/SkillStudio.tsx", + "../../ui/src/pages/CompanySkills.tsx", + "../../ui/src/lib/company-skill-routes.ts" ], - "oracleVersion": "paperclip.chat-catalog.skill-authoring.v2", + "oracleVersion": "paperclip.chat-catalog.skill-authoring.v3", "expectedRunCount": 1, "expectedHumanMessageCount": 1, "turnTimeoutMs": 120000, diff --git a/tests/runner-e2e/chat-catalog-domains/integrator-skill-authoring-v2.test.ts b/tests/runner-e2e/chat-catalog-domains/integrator-skill-authoring-v2.test.ts index 0692d4256db..970cc976b30 100644 --- a/tests/runner-e2e/chat-catalog-domains/integrator-skill-authoring-v2.test.ts +++ b/tests/runner-e2e/chat-catalog-domains/integrator-skill-authoring-v2.test.ts @@ -9,16 +9,17 @@ afterEach(()=>vi.restoreAllMocks()); function fixture(){ const markdown=websiteChecklist.join("\n"),source="https://fixture.test/ACORN/chats/ceo";let current=source,reloads=0,editorReads=0; const version={id:"revision",companyId:"company",companySkillId:"skill",revisionNumber:1,authorAgentId:"ceo",authorUserId:null,fileInventory:[{path:"SKILL.md",kind:"skill",content:markdown}]}; - const skill={id:"skill",companyId:"company",name:"Website review checklist",editable:true,currentVersionId:"revision",markdown,usedByAgents:[]}; + const skill={id:"skill",companyId:"company",name:"Website review checklist",slug:"website-review-checklist",editable:true,currentVersionId:"revision",markdown,usedByAgents:[]}; const state:Record={},o={scenarioId:"skills.author-skill",origin:"https://fixture.test",companyId:"company",leadId:"ceo",state,skills:[skill],versions:{skill:[version]},policy:{},editDecision:{allowed:true},tasks:[],projects:[],goals:[],comments:[],documents:[],revisions:[],humanQueries:["Turn our website review checklist into a reusable skill."],runs:[{id:"native",status:"succeeded",agentId:"ceo",companyId:"company"}],activity:[{action:"company.skill_created",entityId:"skill",agentId:"ceo"}],links:[],reply:"Created the reusable company skill, saved in the library and not assigned to any agent."} as unknown as InternalObservation; observed.base=o; - const ui={library:true,studio:true,editable:true,body:markdown,changedAfterReload:false,wrongRoute:false,delayedHydration:false};const events:string[]=[]; - const editor={get visible(){return ui.studio;},get editable(){return ui.editable;},locator:()=>({evaluateAll:async()=>ui.delayedHydration&&editorReads++===0?[]:(ui.changedAfterReload&&reloads?"Lost checklist":ui.body).split("\n")})}; + const ui={library:true,studio:true,editable:true,body:markdown,changedAfterReload:false,wrongRoute:false,foreignRoute:false,directStudio:false,delayedHydration:false,inEdit:false};const events:string[]=[]; + const editor={get visible(){return ui.studio && (ui.inEdit || ui.directStudio);},get editable(){return ui.editable;},locator:()=>({evaluateAll:async()=>ui.delayedHydration&&editorReads++===0?[]:(ui.changedAfterReload&&reloads?"Lost checklist":ui.body).split("\n")})}; const page={url:()=>current,getByRole:(role:string,options?:{name?:string})=>{ if(role==="textbox")return editor; + if(role==="button" && options?.name==="Edit")return{click:async()=>{if(!ui.editable)throw Error("No Edit control");ui.inEdit=true;events.push("Edit");}}; if(role==="link")return{first:()=>({click:async()=>{events.push(options!.name!);current=options?.name==="Skills"?"https://fixture.test/ACORN/skills":"https://fixture.test/ACORN/skills/studio";}})}; - return{filter:()=>({first:()=>({get visible(){return ui.library;},click:async()=>{events.push("open-skill");current=ui.wrongRoute?"https://fixture.test/ACORN/skills/studio/other":"https://fixture.test/ACORN/skills/studio/skill";}})})}; - },getByText:()=>({first:()=>({get visible(){return ui.studio;}})}),getByTestId:()=>({get visible(){return current===source;}}),reload:async()=>{reloads++;events.push("reload");},goto:async(url:string)=>{current=url;events.push("return-chat");}}; + return{filter:()=>({first:()=>({get visible(){return ui.library;},click:async()=>{events.push("open-skill");current=ui.foreignRoute?"https://fixture.test/FOREIGN/skills/website-review-checklist":ui.wrongRoute?"https://fixture.test/ACORN/skills/other":ui.directStudio?"https://fixture.test/ACORN/skills/studio/skill":"https://fixture.test/ACORN/skills/website-review-checklist";}})})}; + },getByText:()=>({first:()=>({get visible(){return ui.studio;}})}),getByTestId:()=>({get visible(){return current===source;}}),reload:async()=>{reloads++;ui.inEdit=false;events.push("reload");},goto:async(url:string)=>{current=url;events.push("return-chat");}}; const after=structuredClone(skill),get=vi.fn(async(route:string)=>{ if(route.endsWith("/skills"))return[{id:"skill"}]; if(route.endsWith("/skills/skill"))return after; @@ -30,21 +31,22 @@ function fixture(){ const ctx={scenario:{id:"skills.author-skill"},observationDeadlineAt:Date.now()+270000,input:{fixtures:{company:{id:"company",issuePrefix:"ACORN"},agent:{id:"ceo"}},api:{baseURL:o.origin,get,post},page,evidence},refreshIssue:async()=>{},issue:()=>({conversationState:"waiting"}),allRuns:async()=>o.runs} as unknown as ChatCatalogContext; return{ctx,state,o,skill,version,ui,events,after,get,post,evidence}; } -it("the actual authoring act performs readonly normal library/Studio inspection and keeps a single native/human turn",async()=>{ +it("the actual authoring act performs nonmutating canonical-library/Edit inspection and keeps a single native/human turn",async()=>{ const f=fixture(),d=integratorSkillsCases.find(x=>x.scenarioId==="skills.author-skill")!; await d.act!(f.ctx,f.state); - expect(f.events).toEqual(["Skills","My Skills","open-skill","reload","return-chat"]); - expect(f.state.authoredSkillInspection).toMatchObject({skillId:"skill",versionId:"revision",readbackMatched:true}); + expect(f.events).toEqual(["Skills","My Skills","open-skill","Edit","reload","Edit","return-chat"]); + expect(f.state.authoredSkillInspection).toMatchObject({skillId:"skill",versionId:"revision",editorSurface:"detail",editorVisible:true,readbackMatched:true}); expect(gradeIntegratorSkills({...f.o,state:f.state}).passed).toBe(true); expect(d.expectedRunCount).toBe(1);expect(d.expectedHumanMessageCount).toBe(1); expect(f.post).toHaveBeenCalledExactlyOnceWith("/api/companies/company/skill-policy/evaluate",expect.anything()); }); -it.each(["missing library","missing Studio","readonly","wrong UI route","wrong UI content","lost after reload","wrong saved version","wrong author","foreign company","no saved version"])("rejects %s before recording inspection success",async invalid=>{ +it.each(["missing library","missing Studio","readonly","wrong UI route","foreign UI route","wrong UI content","lost after reload","wrong saved version","wrong author","foreign company","no saved version"])("rejects %s before recording inspection success",async invalid=>{ const f=fixture(); if(invalid==="missing library")f.ui.library=false; if(invalid==="missing Studio")f.ui.studio=false; if(invalid==="readonly")f.ui.editable=false; if(invalid==="wrong UI route")f.ui.wrongRoute=true; + if(invalid==="foreign UI route")f.ui.foreignRoute=true; if(invalid==="wrong UI content")f.ui.body="Review the site."; if(invalid==="lost after reload")f.ui.changedAfterReload=true; if(invalid==="wrong saved version")f.after.currentVersionId="other"; @@ -78,3 +80,5 @@ it("inspects real rich-text blocks without applying Markdown marker assumptions expect(preservesVisibleWebsiteChecklist(["Website review checklist","Type here to edit"])).toBe(false); expect(preservesVisibleWebsiteChecklist([...websiteChecklist.slice(0,3),"375 pixel viewport"])).toBe(false); }); + +it("retains exact direct Studio card navigation when the UI uses it", async()=>{ const f=fixture();f.ui.directStudio=true;await inspectAuthoredSkill(f.ctx,f.state,f.o);expect(f.events).toEqual(["Skills","My Skills","open-skill","reload","return-chat"]);expect(f.state.authoredSkillInspection).toMatchObject({readbackMatched:true,editorSurface:"studio"}); }); diff --git a/tests/runner-e2e/chat-catalog-domains/integrator-skills.test.ts b/tests/runner-e2e/chat-catalog-domains/integrator-skills.test.ts index 5d14f9ceaf9..09d28b4f441 100644 --- a/tests/runner-e2e/chat-catalog-domains/integrator-skills.test.ts +++ b/tests/runner-e2e/chat-catalog-domains/integrator-skills.test.ts @@ -6,7 +6,7 @@ function valid(id: string) { const version = { id: "revision", companyId: "company", companySkillId: "skill", authorAgentId: "ceo", fileInventory: [{ path: "SKILL.md", content: markdown }] }; const skill = { id: "skill", companyId: "company", name: "Accessibility review", editable: true, markdown, currentVersionId: "revision", usedByAgents: [] }; const denied = id === "skills.explicit-restriction"; - return { scenarioId: id, origin: "https://fixture.test", companyId: "company", leadId: "ceo", state: denied ? { skill: structuredClone(skill), versions: [structuredClone(version)], policyRevision: 3 } : { authoredSkillInspection: {companyId:"company",skillId:"skill",versionId:"revision",libraryVisible:true,studioVisible:true,editable:true,checklistVisible:true,reloaded:true,readbackMatched:true} }, + return { scenarioId: id, origin: "https://fixture.test", companyId: "company", leadId: "ceo", state: denied ? { skill: structuredClone(skill), versions: [structuredClone(version)], policyRevision: 3 } : { authoredSkillInspection: {companyId:"company",skillId:"skill",versionId:"revision",libraryVisible:true,editorVisible:true,editorSurface:"studio",editable:true,checklistVisible:true,reloaded:true,readbackMatched:true} }, tasks: [], projects: [], goals: [], comments: [], documents: [], revisions: [], humanQueries: [], runs: [{ id: "run", agentId: "ceo", status: "succeeded" }], activity: denied ? [] : [{ id: "create", entityId: "skill", action: "company.skill_created", agentId: "ceo" }], reply: denied ? "Company policy denies my skill edits. Ask an administrator to request a change." : "Created the reusable review skill; it is available in Studio. This does not enable it for all agents.", @@ -63,8 +63,8 @@ it("preserves a numbered viewport check and its actual multiline inspection deta expect(preservesWebsiteChecklist(markdown.replace("Confirm whether", "Ignore whether"))).toBe(false); }); -it("authoring v2 accepts ordinary Skills/Studio inspection without a prescribed reply link",()=>{ +it("authoring v3 accepts ordinary Skills/Studio inspection without a prescribed reply link",()=>{ const o=valid("skills.author-skill");o.links=[]; - Object.assign(o.state,{authoredSkillInspection:{companyId:"company",skillId:"skill",versionId:"revision",libraryVisible:true,studioVisible:true,editable:true,checklistVisible:true,reloaded:true,readbackMatched:true}}); + Object.assign(o.state,{authoredSkillInspection:{companyId:"company",skillId:"skill",versionId:"revision",libraryVisible:true,editorVisible:true,editorSurface:"studio",editable:true,checklistVisible:true,reloaded:true,readbackMatched:true}}); expect(gradeIntegratorSkills(o).passed).toBe(true); }); diff --git a/tests/runner-e2e/chat-catalog-domains/integrator-skills.ts b/tests/runner-e2e/chat-catalog-domains/integrator-skills.ts index 90343be52fe..5f34b73f8b1 100644 --- a/tests/runner-e2e/chat-catalog-domains/integrator-skills.ts +++ b/tests/runner-e2e/chat-catalog-domains/integrator-skills.ts @@ -1,3 +1,4 @@ +import { skillRoute, skillStudioRoute } from "../../../ui/src/lib/company-skill-routes.js"; import { readSkillVersionHistory } from "./integrator-skill-versions.js"; import { saveSkillConversationContext } from "./integrator-skill-context.js"; import { remainingIdentityObservationBudget, waitForReply } from "./identity-support.js"; @@ -74,8 +75,8 @@ export function gradeIntegratorSkills(value: unknown) { c.add("actual-checklist-in-current-revision", !!skill && !!version && version.authorAgentId === o.leadId && typeof markdown === "string" && preservesWebsiteChecklist(markdown) && markdown === skill.markdown, "The current saved SKILL.md revision preserves keyboard-only controls, visible form labels, WCAG AA text contrast and a 375px viewport without horizontal overflow; paraphrase is permitted."); const inspection = o.state.authoredSkillInspection as AuthoredSkillInspection | undefined; - c.add("inspectable-authored-skill", !!skill && inspection?.companyId === o.companyId && inspection.skillId === skill.id && inspection.versionId === skill.currentVersionId && inspection.libraryVisible === true && inspection.studioVisible === true && inspection.editable === true && inspection.checklistVisible === true && inspection.reloaded === true && inspection.readbackMatched === true, - "The actual saved skill is discoverable through ordinary Skills/My Skills, opens editable Studio content, and survives reload with the same saved version. No specific reply-link format is required."); + c.add("inspectable-authored-skill", !!skill && inspection?.companyId === o.companyId && inspection.skillId === skill.id && inspection.versionId === skill.currentVersionId && inspection.libraryVisible === true && inspection.editorVisible === true && (inspection.editorSurface === "detail" || inspection.editorSurface === "studio") && inspection.editable === true && inspection.checklistVisible === true && inspection.reloaded === true && inspection.readbackMatched === true, + "The actual saved skill is discoverable through ordinary Skills/My Skills, opens the actual editable skill content, and survives reload with the same saved version. No specific reply-link format is required."); c.add("creation-not-global-assignment", !!skill && skill.usedByAgents.length === 0 && !/\b(?:enabled|assigned|activated)\s+(?:it|the skill|this skill)?\s*(?:for\s+)?(?:all|every)\b/i.test(o.reply), "Creating a reusable skill does not silently enable it for all agents or claim that happened."); } else if (o.scenarioId === "skills.explicit-restriction") { const before = o.state.skill as CompanySkillDetail; const current = o.skills.find(row => row.id === before?.id); @@ -97,7 +98,7 @@ async function verifySkill(ctx: ChatCatalogContext, state: Record, o: InternalObservation): Promise { const remaining = () => remainingIdentityObservationBudget(ctx); remaining(); @@ -111,8 +112,17 @@ export async function inspectAuthoredSkill(ctx: ChatCatalogContext, state: Recor await ctx.input.page.getByRole("link", { name: "My Skills", exact: true }).first().click({ timeout: remaining() }); const card = ctx.input.page.getByRole("button").filter({ has: ctx.input.page.getByText(skill.name, { exact: true }) }).first(); await expect(card).toBeVisible({ timeout: remaining() }); await card.click({ timeout: remaining() }); - const route = `/${ctx.input.fixtures.company.issuePrefix}/skills/studio/${skill.id}`; - await expect(ctx.input.page).toHaveURL(url => url.pathname === route, { timeout: remaining() }); + const companyPath = `/${encodeURIComponent(ctx.input.fixtures.company.issuePrefix)}`; + const studioRoute = `${companyPath}${skillStudioRoute(skill.id)}`; + const detailRoute = `${companyPath}${skillRoute(skill, observed.skills)}`; + const openSavedEditor = async () => { + await expect(ctx.input.page).toHaveURL(url => url.origin === new URL(source).origin + && (url.pathname === studioRoute || url.pathname === detailRoute), { timeout: remaining() }); + if (new URL(ctx.input.page.url()).pathname === detailRoute) { + await ctx.input.page.getByRole("button", { name: "Edit", exact: true }).click({ timeout: remaining() }); + } + }; + await openSavedEditor(); await expect(ctx.input.page.getByText(skill.name, { exact: true }).first()).toBeVisible({ timeout: remaining() }); const editor = ctx.input.page.getByRole("textbox", { name: "editable markdown", exact: true }); await expect(editor).toBeVisible({ timeout: remaining() }); await expect(editor).toBeEditable({ timeout: remaining() }); @@ -120,13 +130,13 @@ export async function inspectAuthoredSkill(ctx: ChatCatalogContext, state: Recor await expect.poll(async () => preservesVisibleWebsiteChecklist(await readBlocks()), { timeout: remaining() }).toBe(true); const body = await readBlocks(); await ctx.input.page.reload({ waitUntil: "domcontentloaded", timeout: remaining() }); - await expect(ctx.input.page).toHaveURL(url => url.pathname === route, { timeout: remaining() }); + await openSavedEditor(); await expect(editor).toBeVisible({ timeout: remaining() }); await expect(editor).toBeEditable({ timeout: remaining() }); await expect.poll(async () => JSON.stringify(await readBlocks()) === JSON.stringify(body), { timeout: remaining() }).toBe(true); const after = await ctx.input.api.get(`${root(ctx)}/skills/${encodeURIComponent(skill.id)}`, { timeout: remaining() }); if (after.id !== skill.id || after.companyId !== observed.companyId || !after.editable || after.currentVersionId !== skill.currentVersionId || after.markdown !== markdown) throw new Error("Authored skill identity, saved version or bytes changed during inspection"); remaining(); - state.authoredSkillInspection = { companyId: observed.companyId, skillId: skill.id, versionId: skill.currentVersionId, libraryVisible: true, studioVisible: true, editable: true, checklistVisible: true, reloaded: true, readbackMatched: true } satisfies AuthoredSkillInspection; + state.authoredSkillInspection = { companyId: observed.companyId, skillId: skill.id, versionId: skill.currentVersionId, libraryVisible: true, editorVisible: true, editorSurface: new URL(ctx.input.page.url()).pathname === detailRoute ? "detail" : "studio", editable: true, checklistVisible: true, reloaded: true, readbackMatched: true } satisfies AuthoredSkillInspection; await ctx.input.evidence("chat-catalog-authored-skill-ui.json", state.authoredSkillInspection); await ctx.input.page.goto(source, { waitUntil: "domcontentloaded", timeout: remaining() }); await expect(ctx.input.page.getByTestId("task-chat-composer-input")).toBeVisible({ timeout: remaining() }); @@ -135,7 +145,7 @@ export async function inspectAuthoredSkill(ctx: ChatCatalogContext, state: Recor const define = (id: string, setup: ChatCatalogCase["setup"]): ChatCatalogCase => { const base = integratorDefinition("skills", id, setup, observeSkills, gradeIntegratorSkills, verifySkill); base.sourceFiles = [...base.sourceFiles, "chat-catalog-domains/integrator-skill-versions.ts"]; - return id === "skills.author-skill" ? { ...base, authentication: "human-session", oracleVersion: "paperclip.chat-catalog.skill-authoring.v2", sourceFiles: [...base.sourceFiles, "chat-catalog-domains/identity-support.ts", "../../ui/src/components/MarkdownEditor.tsx", "../../ui/src/pages/SkillStudio.tsx"], queryAttachments: factualContextQueryAttachment("skill-context.md"), + return id === "skills.author-skill" ? { ...base, authentication: "human-session", oracleVersion: "paperclip.chat-catalog.skill-authoring.v3", sourceFiles: [...base.sourceFiles, "chat-catalog-domains/identity-support.ts", "../../ui/src/components/MarkdownEditor.tsx", "../../ui/src/pages/SkillStudio.tsx", "../../ui/src/pages/CompanySkills.tsx", "../../ui/src/lib/company-skill-routes.ts"], queryAttachments: factualContextQueryAttachment("skill-context.md"), async act(ctx, state) { remainingIdentityObservationBudget(ctx); await waitForReply(ctx, ctx.observationDeadlineAt); await inspectAuthoredSkill(ctx, state, await observeSkills(ctx, state)); }, async verifyResult(ctx, state, value) { const o = observation(value); if (!o) throw new Error("Missing authored skill inspection evidence"); await inspectAuthoredSkill(ctx, state, o); }, } : base;