From d432bc425b6ff2f48efef07ac6290c7b2c157430 Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 2 Nov 2026 20:54:40 -0600 Subject: [PATCH] test(evals): qualify skill update reads before model admission Require the actual pinned update-status prerequisite before the canonical request. Add an exact human-session GET reader that bounds error-body parsing and preserves only three source-owned reason codes and finite provider statuses; generic response-body withholding is unchanged. Fail closed on unknown, oversized, malformed or private error content. Stop retrying typed update-read refusals while retaining all real source, review, update, version and fork assertions. This improves diagnosis and eligibility, not feature grading. Co-Authored-By: Paperclip --- .../api-skill-update-prerequisite.test.ts | 50 ++++++++++++++++++ tests/runner-e2e/api.ts | 38 ++++++++++++++ .../integrator-skill-update.test.ts | 51 ++++++++++++++++++- .../integrator-skill-update.ts | 19 +++++-- 4 files changed, 152 insertions(+), 6 deletions(-) create mode 100644 tests/runner-e2e/api-skill-update-prerequisite.test.ts diff --git a/tests/runner-e2e/api-skill-update-prerequisite.test.ts b/tests/runner-e2e/api-skill-update-prerequisite.test.ts new file mode 100644 index 00000000000..3ebcd1addac --- /dev/null +++ b/tests/runner-e2e/api-skill-update-prerequisite.test.ts @@ -0,0 +1,50 @@ +import { afterEach, expect, it, vi } from "vitest"; +import type { APIRequestContext } from "@playwright/test"; +import { RunnerApi, SkillSourceUpdateReadError } from "./api.js"; +const company = "11111111-1111-4111-8111-111111111111", skill = "22222222-2222-4222-8222-222222222222"; +afterEach(() => { vi.unstubAllGlobals(); vi.unstubAllEnvs(); }); +function fixture() { + vi.stubEnv("PAPERCLIP_RUNNER_E2E_PORT", "3100"); const traced = { get: vi.fn() }; + const api = new RunnerApi(traced as unknown as APIRequestContext); api.setBrowserSession("synthetic-private-session"); + return { api, traced }; +} +it("uses the exact authenticated update endpoint and returns real successful prerequisite bytes unchanged", async () => { + const { api, traced } = fixture(); const data = { supported: true, currentRef: "old", latestRef: "new", hasUpdate: true }; + const fetcher = vi.fn(async (_url: URL, _options: RequestInit) => Response.json(data)); vi.stubGlobal("fetch", fetcher); + await expect(api.getSkillSourceUpdateStatus(company, skill)).resolves.toEqual(data); + expect(fetcher.mock.calls[0]?.[0]).toEqual(new URL(`/api/companies/${company}/skills/${skill}/update-status`, api.baseURL)); + expect(traced.get).not.toHaveBeenCalled(); +}); +it.each(["skill_source_ref_unavailable", "skill_source_github_rate_limited", "skill_source_github_read_failed"])("retains only the supported closed %s/status while withholding private body fields", async code => { + const { api } = fixture(); vi.stubGlobal("fetch", vi.fn(async () => Response.json({ code, error: "Bearer private-error-canary", details: { code, status: 429, token: "secret-value", repository: "private-origin" } }, { status: 422 }))); + const error = await api.getSkillSourceUpdateStatus(company, skill).catch(error => error); + expect(error).toBeInstanceOf(SkillSourceUpdateReadError); expect(error).toMatchObject({ status: 422, reason: code, providerStatus: 429 }); + expect(JSON.stringify(error)).not.toMatch(/private-error|secret-value|private-origin|Bearer/); expect(error.message).not.toMatch(/private-error|secret-value|private-origin|Bearer/); +}); +it.each(["unknown code", "mismatched code", "oversized", "invalid JSON", "missing code", "invalid provider status"])("does not infer a reason or leak response bytes for %s", async fault => { + const { api } = fixture(); const code = "skill_source_ref_unavailable"; + let body = JSON.stringify({ code, details: { code, status: 422 }, error: "private-error-canary" }); + if (fault === "unknown code") body = JSON.stringify({ code: "private-error-canary", details: { status: 429 } }); + if (fault === "mismatched code") body = JSON.stringify({ code, details: { code: "private-error-canary", status: 429 } }); + if (fault === "oversized") body = JSON.stringify({ code, details: { status: 422 }, error: "private-error-canary".repeat(2000) }); + if (fault === "invalid JSON") body = "notJSON private-error-canary"; + if (fault === "missing code") body = JSON.stringify({ error: "private-error-canary", details: { status: 429 } }); + if (fault === "invalid provider status") body = JSON.stringify({ code, details: { status: "private-error-canary" } }); + vi.stubGlobal("fetch", vi.fn(async () => new Response(body, { status: 422 }))); + const error = await api.getSkillSourceUpdateStatus(company, skill).catch(error => error); + expect(error).toBeInstanceOf(SkillSourceUpdateReadError); + expect(error.reason).toBe(fault === "invalid provider status" ? code : "unclassified"); expect(error.providerStatus).toBeNull(); + expect(JSON.stringify(error)).not.toContain("private-error-canary"); expect(error.message).not.toContain("private-error-canary"); +}); +it("cancels an oversized error stream at the bound and never reads its trailing private data", async () => { + const { api } = fixture(); let cancelled = false; const stream = new ReadableStream({ start(controller) { controller.enqueue(new Uint8Array(16_385)); }, cancel() { cancelled = true; } }); + vi.stubGlobal("fetch", vi.fn(async () => new Response(stream, { status: 422 }))); + await expect(api.getSkillSourceUpdateStatus(company, skill)).rejects.toMatchObject({ reason: "unclassified" }); expect(cancelled).toBe(true); +}); +it("refuses missing human auth or malformed identities before any request", async () => { + const { api } = fixture(); const fetcher = vi.fn(); vi.stubGlobal("fetch", fetcher); + await expect(api.getSkillSourceUpdateStatus("https://foreign.invalid", skill)).rejects.toThrow("exact owned"); + await expect(api.getSkillSourceUpdateStatus(company, "../other")).rejects.toThrow("exact owned"); + const noSession = new RunnerApi({} as APIRequestContext); await expect(noSession.getSkillSourceUpdateStatus(company, skill)).rejects.toThrow("human-session"); + expect(fetcher).not.toHaveBeenCalled(); +}); diff --git a/tests/runner-e2e/api.ts b/tests/runner-e2e/api.ts index b8313bd3f19..05abf740887 100644 --- a/tests/runner-e2e/api.ts +++ b/tests/runner-e2e/api.ts @@ -14,6 +14,33 @@ export class RunnerApiHttpError extends Error { constructor(readonly status: number, message: string) { super(message); this.name = "RunnerApiHttpError"; } } +export type SkillSourceUpdateCode = "skill_source_ref_unavailable" | "skill_source_github_rate_limited" | "skill_source_github_read_failed" | "unclassified"; +export class SkillSourceUpdateReadError extends RunnerApiHttpError { + constructor(status: number, readonly reason: SkillSourceUpdateCode, readonly providerStatus: number | null) { + super(status, `Skill source update prerequisite failed (HTTP ${status}; reason=${reason}; providerStatus=${providerStatus ?? "unknown"}); body withheld`); + this.name = "SkillSourceUpdateReadError"; + } +} +async function closedSkillUpdateFailure(response: Response): Promise<{ reason: SkillSourceUpdateCode; providerStatus: number | null }> { + const unknown = { reason: "unclassified" as const, providerStatus: null }; + const reader = response.body?.getReader(); if (!reader) return unknown; + const chunks: Uint8Array[] = []; let bytes = 0, done = false; + try { + for (;;) { + const next = await reader.read(); if (next.done) { done = true; break; } + bytes += next.value.byteLength; if (bytes > 16_384) return unknown; + chunks.push(next.value); + } + const body = JSON.parse(Buffer.concat(chunks).toString("utf8")) as { code?: unknown; details?: { code?: unknown; status?: unknown } }; + if (!body || typeof body !== "object" || Array.isArray(body)) return unknown; + const codes: readonly string[] = ["skill_source_ref_unavailable", "skill_source_github_rate_limited", "skill_source_github_read_failed"]; + if (typeof body.code !== "string" || !codes.includes(body.code) || body.details?.code !== undefined && body.details.code !== body.code) return unknown; + const status = body.details?.status; + return { reason: body.code as SkillSourceUpdateCode, providerStatus: typeof status === "number" && [401, 403, 404, 422, 429, 500, 502, 503, 504].includes(status) ? status : null }; + } catch { return unknown; } + finally { if (!done) await reader.cancel().catch(() => {}); reader.releaseLock(); } +} + /** Admission-only diagnostics. Never retain the rejected value or its cause. */ export class RemoteAdmissionReadError extends Error { constructor( @@ -47,6 +74,17 @@ export class RunnerApi { return response.json() as Promise; } + async getSkillSourceUpdateStatus(companyId: string, skillId: string, options?: { timeout: number }): Promise { + const uuid = /^[a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12}$/i; + if (!this.browserCookie || !uuid.test(companyId) || !uuid.test(skillId)) throw new Error("Skill source update prerequisite requires the exact owned human-session endpoint"); + const response = await this.authenticatedResponse("GET", `/api/companies/${companyId}/skills/${skillId}/update-status`, undefined, options); + if (!response.ok) { + const closed = await closedSkillUpdateFailure(response); + throw new SkillSourceUpdateReadError(response.status, closed.reason, closed.providerStatus); + } + try { return await response.json() as T; } catch { throw new Error("Skill source update prerequisite returned invalid JSON; body withheld"); } + } + async bytes(path: string): Promise { const response = await this.request.get(path, this.sessionOptions()); if (!response.ok()) throw new RunnerApiHttpError(response.status(), await failureMessage(response, "GET")); diff --git a/tests/runner-e2e/chat-catalog-domains/integrator-skill-update.test.ts b/tests/runner-e2e/chat-catalog-domains/integrator-skill-update.test.ts index 0a76fd83bf4..bd354ab2e12 100644 --- a/tests/runner-e2e/chat-catalog-domains/integrator-skill-update.test.ts +++ b/tests/runner-e2e/chat-catalog-domains/integrator-skill-update.test.ts @@ -1,9 +1,15 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; +import { SkillSourceUpdateReadError } from "../api.js"; import { readFileSync } from "node:fs"; -import { accessibilityUpdatePins as pins, gradeIntegratorSkillUpdate } from "./integrator-skill-update.js"; +import { accessibilityUpdatePins as pins, gradeIntegratorSkillUpdate, requireSkillUpdatePrerequisite, integratorSkillUpdateCases } from "./integrator-skill-update.js"; // Unmodified MIT-licensed source bytes, with LICENSE.md in the same fixture // directory. Product setup still fetches the actual GitHub commit snapshots. import { createHash } from "node:crypto"; +import { saveSkillConversationContext } from "./integrator-skill-context.js"; +vi.mock("./integrator-skill-context.js", () => ({ saveSkillConversationContext: vi.fn(async () => ({})) })); +vi.mock("./integrator-skill-source.js", () => ({ editableLocalNotes: async () => ({ id: "local", currentVersionId: "local-version" }) })); +vi.mock("./integrator-skill-versions.js", () => ({ readSkillVersionHistory: async () => [] })); + const bytes = { old: readFileSync(new URL("../fixtures/accessibility-source/0.4.SKILL.md", import.meta.url), "utf8"), next: readFileSync(new URL("../fixtures/accessibility-source/0.5.SKILL.md", import.meta.url), "utf8") }; function valid(old: string, next: string) { const upstream = { id: "up", companyId: "company", name: "accessibility", editable: false, sourceRef: pins.oldCommit, currentVersionId: "old", markdown: old }; @@ -35,3 +41,44 @@ describe("genuine immutable public skill update qualification", () => { const lying = valid(bytes.old, bytes.next); lying.reply = `${lying.state.reviewCheckpoint.reply}\nI haven't updated the accessibility skill, only fetched its source.`; expect(gradeIntegratorSkillUpdate(lying).passed).toBe(false); }); }); + +it("requires the actual pinned update prerequisite before admitting the human request", async () => { + const real = { supported: true, hasUpdate: true, currentRef: pins.oldCommit, latestRef: pins.newCommit, trackingRef: pins.newCommit }; + const evidence = vi.fn(async (_name: string, _value: unknown) => {}); + const read = vi.fn(async (): Promise<{ supported: boolean; hasUpdate: boolean; currentRef: string; latestRef: string; trackingRef: string }> => real); + const ctx = { observationDeadlineAt: Date.now() + 30_000, input: { api: { getSkillSourceUpdateStatus: read }, fixtures: { company: { id: "company" } }, evidence } } as unknown as Parameters[0]; + await requireSkillUpdatePrerequisite(ctx, "selected"); expect(read).toHaveBeenCalledWith("company", "selected", { timeout: expect.any(Number) }); + expect(evidence).toHaveBeenCalledWith("chat-skill-update-prerequisite.json", expect.objectContaining({ currentPinnedRevisionMatched: true, latestPinnedRevisionMatched: true })); + for (const bad of [{ ...real, supported: false }, { ...real, hasUpdate: false }, { ...real, currentRef: pins.newCommit }, { ...real, latestRef: pins.oldCommit }, { ...real, trackingRef: "wrong" }]) { + read.mockResolvedValueOnce(bad); await expect(requireSkillUpdatePrerequisite(ctx, "selected")).rejects.toThrow("unavailable"); + } + read.mockRejectedValueOnce(new Error("closed HTTP error")); await expect(requireSkillUpdatePrerequisite(ctx, "selected")).rejects.toThrow("closed HTTP error"); + expect(evidence).toHaveBeenCalledOnce(); +}); + +function setupFixture(unavailable: boolean) { + const source = { id: "source", revision: 1, entries: [{ path: pins.path, skillId: "up" }], lastScanCommit: pins.oldCommit, trackingRef: pins.newCommit, automaticUpdates: false }; + const prerequisite = vi.fn(async () => { if (unavailable) throw new SkillSourceUpdateReadError(422, "skill_source_ref_unavailable", 422); + return { supported: true, hasUpdate: true, currentRef: pins.oldCommit, latestRef: pins.newCommit, trackingRef: pins.newCommit }; }); + const ctx = { observationDeadlineAt: Date.now() + 30_000, input: { fixtures: { company: { id: "company" } }, evidence: vi.fn(async (_name: string, _value: unknown) => {}), api: { + post: async () => ({ source, imported: [{ id: "up" }] }), patch: async () => source, + get: async () => ({ id: "up", sourceType: "github", sourceRef: pins.oldCommit, markdown: bytes.old, editable: false }), getSkillSourceUpdateStatus: prerequisite, + } } } as unknown as Parameters[0]; + return { ctx, prerequisite }; +} +it("actual review-update setup blocks an unavailable dependency before preparing the canonical human request", async () => { + vi.mocked(saveSkillConversationContext).mockClear(); + vi.stubGlobal("fetch", vi.fn(async (url: string) => new Response(url.includes(pins.oldCommit) ? bytes.old : bytes.next))); + try { + const f = setupFixture(true); await expect(integratorSkillUpdateCases[0]!.setup(f.ctx)).rejects.toMatchObject({ name: "SkillSourceUpdateReadError", reason: "skill_source_ref_unavailable" }); + expect(f.prerequisite).toHaveBeenCalledOnce(); expect(saveSkillConversationContext).not.toHaveBeenCalled(); + } finally { vi.unstubAllGlobals(); } +}); +it("actual review-update setup keeps the real prerequisite and then prepares the unchanged source-aware request", async () => { + vi.mocked(saveSkillConversationContext).mockClear(); + vi.stubGlobal("fetch", vi.fn(async (url: string) => new Response(url.includes(pins.oldCommit) ? bytes.old : bytes.next))); + try { + const f = setupFixture(false); const result = await integratorSkillUpdateCases[0]!.setup(f.ctx); + expect(f.prerequisite).toHaveBeenCalledOnce(); expect(saveSkillConversationContext).toHaveBeenCalledOnce(); expect(result.source).toMatchObject({ trackingRef: pins.newCommit, lastScanCommit: pins.oldCommit }); + } finally { vi.unstubAllGlobals(); } +}); 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 98c4bd0933a..6d715779c1d 100644 --- a/tests/runner-e2e/chat-catalog-domains/integrator-skill-update.ts +++ b/tests/runner-e2e/chat-catalog-domains/integrator-skill-update.ts @@ -5,7 +5,8 @@ import { createHash } from "node:crypto"; import type { CompanySkillDetail, CompanySkillVersion, CompanySkillUpdateStatus } from "../../../packages/shared/src/types/company-skill.js"; import type { SkillSource, SkillSourceRefreshResult } from "../../../packages/shared/src/types/skill-source.js"; import type { ChatCatalogContext } from "../chat-catalog-contract.js"; -import { pollUntil } from "../api.js"; +import { pollUntil, SkillSourceUpdateReadError } from "../api.js"; +import { remainingIdentityObservationBudget } from "./identity-support.js"; import { checks, observation, type InternalObservation } from "./internal-support.js"; import { editableLocalNotes } from "./integrator-skill-source.js"; import { integratorDefinition, inspectResult, observeInternal } from "./integrator-support.js"; @@ -20,7 +21,7 @@ async function observeUpdate(ctx: ChatCatalogContext, state: Record(`${root(ctx)}/skills/${upstream.id}`), localFork: await api.get(`${root(ctx)}/skills/${local.id}`), 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`) }; + source: await api.get(`${root(ctx)}/skill-sources/${(state.source as SkillSource).id}`), updateStatus: await api.getSkillSourceUpdateStatus(ctx.input.fixtures.company.id, upstream.id) }; } 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; } function substantiveDiff(reply: string) { @@ -53,6 +54,15 @@ export function gradeIntegratorSkillUpdate(value: unknown) { && !/\b(?:(?:not|have not|haven't|did not|didn't) (?:yet )?(?:updated|installed|applied)|only fetched|only listed|failed|cannot update|can't update)\b/i.test(later), "The second actual response reports completed installation supported by the saved new version, not merely fetching or listing the source."); return c.result(); } +export async function requireSkillUpdatePrerequisite(ctx: ChatCatalogContext, skillId: string) { + const status = await ctx.input.api.getSkillSourceUpdateStatus(ctx.input.fixtures.company.id, skillId, { timeout: Math.min(30_000, remainingIdentityObservationBudget(ctx)) }); + remainingIdentityObservationBudget(ctx); + if (status.supported !== true || status.hasUpdate !== true || status.currentRef !== accessibilityUpdatePins.oldCommit + || status.latestRef !== accessibilityUpdatePins.newCommit || status.trackingRef !== accessibilityUpdatePins.newCommit) + throw new Error("Actual pinned skill update prerequisite is unavailable; no human request admitted"); + await ctx.input.evidence("chat-skill-update-prerequisite.json", { supported: true, hasUpdate: true, currentPinnedRevisionMatched: true, latestPinnedRevisionMatched: true, trackedPinnedRevisionMatched: true }); + remainingIdentityObservationBudget(ctx); +} export const integratorSkillUpdateCases = [integratorDefinition("skills", "skills.review-update", async ctx => { // Independently requalify immutable public bytes before any model admission. for (const [commit, expected] of [[accessibilityUpdatePins.oldCommit, accessibilityUpdatePins.oldSha256], [accessibilityUpdatePins.newCommit, accessibilityUpdatePins.newSha256]]) { @@ -66,13 +76,14 @@ export const integratorSkillUpdateCases = [integratorDefinition("skills", "skill const localFork = await editableLocalNotes(ctx, upstream); 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"); + await requireSkillUpdatePrerequisite(ctx, upstream.id); 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 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-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"], + sourceFiles: ["chat-catalog-domains/identity-support.ts", "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" }); + const first = await pollUntil({ label: "actual upstream diff presented before installation", deadlineAt: Date.now() + 180_000, intervalMs: 1000, load: () => observeUpdate(ctx, state), retryOnError: error => !(error instanceof SkillSourceUpdateReadError), 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); if (!previewCorrect(first)) throw new Error("The initial actual diff review was wrong or installed prematurely; later updates cannot repair it"); const { sendChatMessage } = await import("../chat-flow.js"); await sendChatMessage(ctx.input.page, "Apply that reviewed upstream accessibility update now, keeping our editable local copy unchanged.");