From cbecbfc78801898a770bfb767ffed0d083b16c1b Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 2 Nov 2026 20:14:53 -0600 Subject: [PATCH] fix(evals): read immutable skill version details for content evidence Version listings contain inventory metadata without file bytes. Fetch source-scoped immutable version details and preserve version, author and inventory identity for content and history observations. Keep link, import, semantic and native feature graders unchanged. Co-Authored-By: Paperclip --- .../integrator-skill-enable.ts | 5 +- .../integrator-skill-imports.ts | 7 +-- .../integrator-skill-update.ts | 7 +-- .../integrator-skill-versions.test.ts | 48 +++++++++++++++++++ .../integrator-skill-versions.ts | 16 +++++++ .../chat-catalog-domains/integrator-skills.ts | 6 ++- 6 files changed, 79 insertions(+), 10 deletions(-) create mode 100644 tests/runner-e2e/chat-catalog-domains/integrator-skill-versions.test.ts create mode 100644 tests/runner-e2e/chat-catalog-domains/integrator-skill-versions.ts diff --git a/tests/runner-e2e/chat-catalog-domains/integrator-skill-enable.ts b/tests/runner-e2e/chat-catalog-domains/integrator-skill-enable.ts index c858dfc1fab..78736649de4 100644 --- a/tests/runner-e2e/chat-catalog-domains/integrator-skill-enable.ts +++ b/tests/runner-e2e/chat-catalog-domains/integrator-skill-enable.ts @@ -1,3 +1,4 @@ +import { readSkillVersionHistory } from "./integrator-skill-versions.js"; import type { CompanySkillDetail, CompanySkillVersion } from "../../../packages/shared/src/types/company-skill.js"; import type { AgentSkillSnapshot } from "../../../packages/shared/src/types/adapter-skills.js"; import { SKILL_POLICY_ACTIONS, type EffectiveSkillPolicy } from "../../../packages/shared/src/validators/skill-policy.js"; @@ -13,7 +14,7 @@ async function observeEnable(ctx: ChatCatalogContext, state: Record(`${root(ctx)}/skills/${(state.skill as CompanySkillDetail).id}`), assignment: await ctx.input.api.get(`/api/agents/${(state.worker as Entity).id}/skills`), effectivePolicy: await ctx.input.api.get(`${root(ctx)}/skill-policy`), - versions: await ctx.input.api.get(`${root(ctx)}/skills/${(state.skill as CompanySkillDetail).id}/versions`) }; + versions: await readSkillVersionHistory(ctx.input.api, ctx.input.fixtures.company.id, (state.skill as CompanySkillDetail).id) }; } export interface SkillEnableAuthorityProof { companyId: string; principalId: string; targetId: string; targetCompanyId: string; status: number; @@ -101,7 +102,7 @@ export const integratorSkillEnableCases = [integratorDefinition("skills", "skill } catch (error) { await cleanupIdentityProbes(ctx); throw error; } }, observeEnable, gradeIntegratorSkillEnable, async (ctx, state) => { const skill = state.skill as CompanySkillDetail; await inspectResult(ctx, `/${ctx.input.fixtures.company.issuePrefix}/skills/studio/${skill.id}`, skill.name); -}, { sourceFiles: ["chat-catalog-domains/integrator-skill-enable.ts", "chat-catalog-domains/identity-support.ts", "chat-catalog-domains/internal-definition.ts", "chat-catalog-domains/internal-support.ts", "chat-catalog-domains/integrator-support.ts", "chat-scenarios/skills.yaml", "../../server/src/routes/agents.ts", "../../server/src/routes/company-skill-policy.ts", "../../server/src/services/company-skills.ts", "../../packages/shared/src/validators/skill-policy.ts"], +}, { sourceFiles: ["chat-catalog-domains/integrator-skill-versions.ts", "chat-catalog-domains/integrator-skill-enable.ts", "chat-catalog-domains/identity-support.ts", "chat-catalog-domains/internal-definition.ts", "chat-catalog-domains/internal-support.ts", "chat-catalog-domains/integrator-support.ts", "chat-scenarios/skills.yaml", "../../server/src/routes/agents.ts", "../../server/src/routes/company-skill-policy.ts", "../../server/src/services/company-skills.ts", "../../packages/shared/src/validators/skill-policy.ts"], expectedRunCount: 2, attemptTimeoutMs: 540_000, oracleVersion: "paperclip.chat-catalog.skill-enable.standard-config.v2", async act(ctx, state) { const first = await pollUntil({ label: "actual Morgan assignment before future admission", deadlineAt: Date.now() + 180_000, intervalMs: 1000, diff --git a/tests/runner-e2e/chat-catalog-domains/integrator-skill-imports.ts b/tests/runner-e2e/chat-catalog-domains/integrator-skill-imports.ts index 3fe0e165bd7..670a983c8e3 100644 --- a/tests/runner-e2e/chat-catalog-domains/integrator-skill-imports.ts +++ b/tests/runner-e2e/chat-catalog-domains/integrator-skill-imports.ts @@ -1,3 +1,4 @@ +import { readSkillVersionHistory } from "./integrator-skill-versions.js"; import { saveSkillConversationContext } from "./integrator-skill-context.js"; import { factualContextQueryAttachment } from "../chat-query-attachments.js"; import type { CompanySkillDetail, CompanySkillFileDetail, CompanySkillListItem, CompanySkillVersion } from "../../../packages/shared/src/types/company-skill.js"; @@ -14,7 +15,7 @@ async function observeImports(ctx: ChatCatalogContext, state: Record(root(ctx)); const skills = await Promise.all(rows.map(row => api.get(`${root(ctx)}/${row.id}`))); - const skillVersions: Record = Object.fromEntries(await Promise.all(rows.map(async row => [row.id, await api.get(`${root(ctx)}/${row.id}/versions`)]))); + const skillVersions: Record = Object.fromEntries(await Promise.all(skills.map(async row => [row.id, await readSkillVersionHistory(api, ctx.input.fixtures.company.id, row.id, row.currentVersionId)]))); let importedFile: CompanySkillFileDetail | undefined; if (ctx.scenario.id === "skills.find-and-install") { const imported = skills.find(row => row.sourceLocator === state.sourceURL); @@ -86,13 +87,13 @@ function define(id: "skills.find-and-install" | "skills.fork-read-only") { } const upstream = await importReadOnlySkill(ctx); const localFork = await editableLocalNotes(ctx, upstream); const selected = await saveSkillConversationContext(ctx, { projectId: p.id, description: `Selected read-only checklist skill: ${upstream.name} (${upstream.id}), source ${upstream.sourceLocator}, revision ${upstream.sourceRef}. Existing editable local notes: ${localFork.name} (${localFork.id}). Our mobile checks: ${mobileChecks.join(" ")}` }); - return { project: p, sourceURL: upstream.sourceLocator, importAllowed: true, upstreamQualified: true, upstream, localFork, factualContextAttachment: selected.factualContextAttachment, upstreamVersions: await ctx.input.api.get(`${root(ctx)}/${upstream.id}/versions`) }; + return { project: p, sourceURL: upstream.sourceLocator, importAllowed: true, upstreamQualified: true, upstream, localFork, factualContextAttachment: selected.factualContextAttachment, upstreamVersions: await readSkillVersionHistory(ctx.input.api, ctx.input.fixtures.company.id, upstream.id, upstream.currentVersionId) }; } catch (error) { await closeVettedSkillSource(ctx); throw error; } }, observeImports, gradeIntegratorSkillImports, async (ctx, _state, value) => { const o = importObservation(value); const skill = o && (id === "skills.find-and-install" ? o.skills.find(row => row.sourceLocator === o.state.sourceURL) : changedFork(o)); if (!o || !skill) throw new Error("No actual saved skill copy for browser inspection"); await inspectResult(ctx, `/${ctx.input.fixtures.company.issuePrefix}/skills/studio/${skill.id}`, skill.name, resultHref(o, skill.id, "studio")); - }, { authentication: "human-session", queryAttachments: factualContextQueryAttachment("skill-context.md"), sourceFiles: ["chat-catalog-domains/integrator-skill-imports.ts", "chat-catalog-domains/integrator-skill-source.ts", "chat-catalog-domains/integrator-skills.ts", "chat-catalog-domains/integrator-support.ts", "chat-catalog-domains/internal-support.ts", "chat-scenarios/skills.yaml"], + }, { authentication: "human-session", queryAttachments: factualContextQueryAttachment("skill-context.md"), sourceFiles: ["chat-catalog-domains/integrator-skill-versions.ts", "chat-catalog-domains/integrator-skill-imports.ts", "chat-catalog-domains/integrator-skill-source.ts", "chat-catalog-domains/integrator-skills.ts", "chat-catalog-domains/integrator-support.ts", "chat-catalog-domains/internal-support.ts", "chat-scenarios/skills.yaml"], async cleanup(ctx) { await closeVettedSkillSource(ctx); } }); } export const integratorSkillImportCases = [define("skills.find-and-install"), define("skills.fork-read-only")]; diff --git a/tests/runner-e2e/chat-catalog-domains/integrator-skill-update.ts b/tests/runner-e2e/chat-catalog-domains/integrator-skill-update.ts index 3904fb276c6..98c4bd0933a 100644 --- a/tests/runner-e2e/chat-catalog-domains/integrator-skill-update.ts +++ b/tests/runner-e2e/chat-catalog-domains/integrator-skill-update.ts @@ -1,3 +1,4 @@ +import { readSkillVersionHistory } from "./integrator-skill-versions.js"; import { saveSkillConversationContext } from "./integrator-skill-context.js"; import { factualContextQueryAttachment } from "../chat-query-attachments.js"; import { createHash } from "node:crypto"; @@ -18,7 +19,7 @@ interface UpdateObservation extends InternalObservation { upstream: CompanySkill async function observeUpdate(ctx: ChatCatalogContext, state: Record): Promise { const base = await observeInternal(ctx, state); const api = ctx.input.api; const upstream = state.upstream as CompanySkillDetail; const local = state.localFork as CompanySkillDetail; return { ...base, upstream: await api.get(`${root(ctx)}/skills/${upstream.id}`), localFork: await api.get(`${root(ctx)}/skills/${local.id}`), - upstreamVersions: await api.get(`${root(ctx)}/skills/${upstream.id}/versions`), forkVersions: await api.get(`${root(ctx)}/skills/${local.id}/versions`), + upstreamVersions: await readSkillVersionHistory(api, ctx.input.fixtures.company.id, upstream.id), forkVersions: await readSkillVersionHistory(api, ctx.input.fixtures.company.id, local.id), source: await api.get(`${root(ctx)}/skill-sources/${(state.source as SkillSource).id}`), updateStatus: await api.get(`${root(ctx)}/skills/${upstream.id}/update-status`) }; } function updateObservation(value: unknown): UpdateObservation | undefined { const o = value as UpdateObservation; return observation(value) && !!o.upstream && !!o.localFork && Array.isArray(o.upstreamVersions) && Array.isArray(o.forkVersions) && !!o.source && !!o.updateStatus ? o : undefined; } @@ -66,10 +67,10 @@ export const integratorSkillUpdateCases = [integratorDefinition("skills", "skill const source = await ctx.input.api.patch(`${root(ctx)}/skill-sources/${imported.source.id}/settings`, { revision: imported.source.revision, automaticUpdates: false, importNewSkills: false, trackingRef: accessibilityUpdatePins.newCommit }); if (source.lastScanCommit !== accessibilityUpdatePins.oldCommit || source.trackingRef !== accessibilityUpdatePins.newCommit || source.automaticUpdates !== false) throw new Error("Normal source settings applied an update prematurely"); const selected = await saveSkillConversationContext(ctx, { description: `Selected upstream accessibility skill: ${upstream.name} (${upstream.id}), tracked source ${source.id}. Installed source revision ${upstream.sourceRef}; tracking reference ${source.trackingRef}. Separate editable local copy: ${localFork.name} (${localFork.id}).` }); - return { upstream, localFork, source, factualContextAttachment: selected.factualContextAttachment, localVersions: await ctx.input.api.get(`${root(ctx)}/skills/${localFork.id}/versions`) }; + return { upstream, localFork, source, factualContextAttachment: selected.factualContextAttachment, localVersions: await readSkillVersionHistory(ctx.input.api, ctx.input.fixtures.company.id, localFork.id, localFork.currentVersionId) }; }, observeUpdate, gradeIntegratorSkillUpdate, async (ctx, state) => { const skill = state.upstream as CompanySkillDetail; await inspectResult(ctx, `/${ctx.input.fixtures.company.issuePrefix}/skills/studio/${skill.id}`, skill.name); }, { authentication: "human-session", queryAttachments: factualContextQueryAttachment("skill-context.md"), expectedRunCount: 2, expectedHumanMessageCount: 2, attemptTimeoutMs: 540_000, - sourceFiles: ["chat-catalog-domains/integrator-skill-update.ts", "chat-catalog-domains/integrator-skill-source.ts", "chat-catalog-domains/integrator-support.ts", "chat-catalog-domains/internal-support.ts", "chat-scenarios/skills.yaml"], + sourceFiles: ["chat-catalog-domains/integrator-skill-versions.ts", "chat-catalog-domains/integrator-skill-update.ts", "chat-catalog-domains/integrator-skill-source.ts", "chat-catalog-domains/integrator-support.ts", "chat-catalog-domains/internal-support.ts", "chat-scenarios/skills.yaml"], async act(ctx, state) { const first = await pollUntil({ label: "actual upstream diff presented before installation", deadlineAt: Date.now() + 180_000, intervalMs: 1000, load: () => observeUpdate(ctx, state), accept: o => o.runs.length === 1 && o.runs[0]?.status === "succeeded" && ctx.issue()?.conversationState === "waiting" }); state.reviewCheckpoint = { ...first, state: structuredClone(state) }; await ctx.input.evidence("chat-skill-source-review-before-update.json", state.reviewCheckpoint); diff --git a/tests/runner-e2e/chat-catalog-domains/integrator-skill-versions.test.ts b/tests/runner-e2e/chat-catalog-domains/integrator-skill-versions.test.ts new file mode 100644 index 00000000000..c65107c514b --- /dev/null +++ b/tests/runner-e2e/chat-catalog-domains/integrator-skill-versions.test.ts @@ -0,0 +1,48 @@ +import { expect, it, vi } from "vitest"; +import type { ChatCatalogContext } from "../chat-catalog-contract.js"; +vi.mock("./integrator-support.js", async importOriginal => ({ ...await importOriginal(), observeInternal: vi.fn(async (_ctx: unknown,state:unknown)=>({scenarioId:"skills.author-skill",companyId:"company",leadId:"ceo",state})) })); +import { integratorSkillsCases } from "./integrator-skills.js"; +const summary={id:"version",companyId:"company",companySkillId:"skill",revisionNumber:2,authorAgentId:"ceo",authorUserId:null,fileInventory:[{path:"SKILL.md",kind:"skill"}]}; +const detail={...summary,fileInventory:[{path:"SKILL.md",kind:"skill",content:"# Actual saved bytes"}]}; +it("the actual skill observer resolves summary metadata into exact saved version bytes",async()=>{ + const get=vi.fn(async(route:string)=>{ + if(route==="/api/companies/company/skills")return[{id:"skill"}]; + if(route==="/api/companies/company/skills/skill")return{id:"skill",companyId:"company",currentVersionId:"version"}; + if(route==="/api/companies/company/skills/skill/versions")return[summary]; + if(route==="/api/companies/company/skills/skill/versions/version")return detail; + if(route==="/api/companies/company/skill-policy")return{}; + throw new Error("Unexpected observer read"); + }); + const ctx={input:{fixtures:{company:{id:"company"}},api:{get,post:async()=>({allowed:true})}}} as unknown as ChatCatalogContext; + const o=await integratorSkillsCases[0]!.observe(ctx,{}) as {versions:Record}; + expect(o.versions.skill?.[0]?.fileInventory[0]?.content).toBe("# Actual saved bytes"); + expect(get).toHaveBeenCalledWith("/api/companies/company/skills/skill/versions/version"); +}); + +import { readSkillVersionHistory } from "./integrator-skill-versions.js"; +it.each(["wrong version","wrong skill","wrong company","wrong author","missing content","missing file","missing detail"])("rejects %s instead of borrowing current mutable skill bytes",async invalid=>{ + const changed=structuredClone(detail) as typeof detail | undefined; + if(changed){ + if(invalid==="wrong version")changed.id="other"; + if(invalid==="wrong skill")changed.companySkillId="other"; + if(invalid==="wrong company")changed.companyId="foreign"; + if(invalid==="wrong author")changed.authorAgentId="other"; + if(invalid==="missing content")delete(changed.fileInventory[0] as {content?:string}).content; + if(invalid==="missing file")changed.fileInventory=[]; + } + const get=vi.fn(async(route:string)=>route.endsWith("/versions")?[summary]:invalid==="missing detail"?undefined:changed); + await expect(readSkillVersionHistory({get} as never,"company","skill","version")).rejects.toThrow("detail identity or saved file bytes"); +}); +it.each(["foreign summary","duplicate summary","missing selected version"])("refuses %s before any detail read",async invalid=>{ + const rows=invalid==="duplicate summary"?[summary,summary]:[{...summary,companyId:invalid==="foreign summary"?"foreign":"company"}]; + const get=vi.fn(async()=>rows); + await expect(readSkillVersionHistory({get} as never,"company","skill",invalid==="missing selected version"?"other":"version")).rejects.toThrow("summary identity"); + expect(get).toHaveBeenCalledOnce(); +}); +it("keeps complete multi-version history and does not invent immutable history for unversioned local imports",async()=>{ + const second={...summary,id:"older",revisionNumber:1,authorAgentId:null}; + const get=vi.fn(async(route:string)=>route.endsWith("/versions")?[summary,second]:route.endsWith("/older")?{...second,fileInventory:[{path:"SKILL.md",kind:"skill",content:"# Older actual bytes"}]}:detail); + const versions=await readSkillVersionHistory({get} as never,"company","skill","version"); + expect(versions.map(row=>row.id)).toEqual(["version","older"]);expect(versions[1]!.fileInventory[0]!.content).toBe("# Older actual bytes"); + await expect(readSkillVersionHistory({get:async()=>[]} as never,"company","skill")).resolves.toEqual([]); +}); diff --git a/tests/runner-e2e/chat-catalog-domains/integrator-skill-versions.ts b/tests/runner-e2e/chat-catalog-domains/integrator-skill-versions.ts new file mode 100644 index 00000000000..5719abc2dbd --- /dev/null +++ b/tests/runner-e2e/chat-catalog-domains/integrator-skill-versions.ts @@ -0,0 +1,16 @@ +import type { CompanySkillVersion, CompanySkillVersionSummary } from "../../../packages/shared/src/types/company-skill.js"; +import type { RunnerApi } from "../api.js"; +/** Version listings intentionally omit bytes. Read each immutable detail before + * evaluating its content or comparing complete revision history. */ +export async function readSkillVersionHistory(api: Pick, companyId: string, skillId: string, currentVersionId?: string | null): Promise { + const root = `/api/companies/${encodeURIComponent(companyId)}/skills/${encodeURIComponent(skillId)}/versions`; + const rows = await api.get(root); + const metadata = ["id", "companyId", "companySkillId", "revisionNumber", "authorAgentId", "authorUserId"] as const; + if (!companyId || !skillId || !Array.isArray(rows) || rows.some(row => !row || metadata.some(key => !Object.hasOwn(row,key)) || typeof row.id !== "string" || !row.id || row.companyId !== companyId || row.companySkillId !== skillId || !Number.isSafeInteger(row.revisionNumber) || row.revisionNumber < 1 || ![row.authorAgentId,row.authorUserId].every(author => author === null || typeof author === "string" && !!author) || !Array.isArray(row.fileInventory) || row.fileInventory.some(file => !file || typeof file.path !== "string" || !file.path || typeof file.kind !== "string")) || new Set(rows.map(row=>row.id)).size !== rows.length || currentVersionId && !rows.some(row=>row.id===currentVersionId)) throw new Error("Skill version summary identity is missing or inconsistent"); + return Promise.all(rows.map(async row => { + const detail = await api.get(`${root}/${encodeURIComponent(row.id)}`); + const inventory = (files: Array<{path:string;kind:string}>) => JSON.stringify(files.map(file=>({path:file.path,kind:file.kind})).sort((a,b)=>a.path.localeCompare(b.path))); + if (!detail || metadata.some(key => !Object.hasOwn(detail,key) || detail[key] !== row[key]) || !Array.isArray(detail.fileInventory) || detail.fileInventory.some(file=>!file || typeof file.path!=="string" || !file.path || typeof file.content!=="string") || new Set(detail.fileInventory.map(file=>file.path)).size!==detail.fileInventory.length || inventory(detail.fileInventory)!==inventory(row.fileInventory)) throw new Error("Skill version detail identity or saved file bytes are missing or inconsistent"); + return detail; + })); +} diff --git a/tests/runner-e2e/chat-catalog-domains/integrator-skills.ts b/tests/runner-e2e/chat-catalog-domains/integrator-skills.ts index 8a63e355067..73ae9eef94a 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 { readSkillVersionHistory } from "./integrator-skill-versions.js"; import { saveSkillConversationContext } from "./integrator-skill-context.js"; import { factualContextQueryAttachment } from "../chat-query-attachments.js"; import type { CompanySkill, CompanySkillDetail, CompanySkillListItem, CompanySkillVersion } from "../../../packages/shared/src/types/company-skill.js"; @@ -28,7 +29,7 @@ async function observeSkills(ctx: ChatCatalogContext, state: Record(`${root(ctx)}/skills`); const skills = await Promise.all(summaries.map(row => api.get(`${root(ctx)}/skills/${row.id}`))); - const versions = Object.fromEntries(await Promise.all(skills.map(async row => [row.id, await api.get(`${root(ctx)}/skills/${row.id}/versions`)]))); + const versions = Object.fromEntries(await Promise.all(skills.map(async row => [row.id, await readSkillVersionHistory(api, ctx.input.fixtures.company.id, row.id, row.currentVersionId)]))); const [policy, editDecision] = await Promise.all([api.get(`${root(ctx)}/skill-policy`), api.post(`${root(ctx)}/skill-policy/evaluate`, { principal: { agentId: base.leadId }, action: "skills.edit", resource: state.skill ? { skillId: (state.skill as CompanySkillDetail).id } : {}, })]); @@ -74,6 +75,7 @@ async function verifySkill(ctx: ChatCatalogContext, state: Record { const base = integratorDefinition("skills", id, setup, observeSkills, gradeIntegratorSkills, verifySkill); + base.sourceFiles.push("chat-catalog-domains/integrator-skill-versions.ts"); return id === "skills.author-skill" ? { ...base, authentication: "human-session", queryAttachments: factualContextQueryAttachment("skill-context.md") } : base; }; export const integratorSkillsCases: readonly ChatCatalogCase[] = [ @@ -87,7 +89,7 @@ export const integratorSkillsCases: readonly ChatCatalogCase[] = [ const api = ctx.input.api; const skill = await api.post(`${root(ctx)}/skills`, { name: "Accessibility review", slug: "accessibility-review", markdown: `---\nname: accessibility-review\ndescription: Review a website for accessibility.\n---\n\n${websiteChecklist.join("\n")}` }); const previous = await api.get(`${root(ctx)}/skill-policy`); const policy = await api.putSensitive(`${root(ctx)}/skill-policy`, { expectedRevision: previous.revision, schemaVersion: 1, defaultEffect: "allow", rules: [{ id: "deny-ceo-skill-edits", priority: 100, effect: "deny", subject: { type: "agents", agentIds: [ctx.input.fixtures.agent.id] }, actions: ["skills.edit", "skills.create", "skills.import", "skills.install", "skills.update", "skills.reset", "skills.remove"] }] }); - const versions = await api.get(`${root(ctx)}/skills/${skill.id}/versions`); + const versions = await readSkillVersionHistory(api, ctx.input.fixtures.company.id, skill.id, skill.currentVersionId); return { skill, versions, policyRevision: policy.revision }; }), ];