From b7dfdbbefdca8b1f17e1eb7fe3c93ab4c4c41a18 Mon Sep 17 00:00:00 2001 From: Dotta Date: Mon, 2 Nov 2026 20:46:14 -0600 Subject: [PATCH] test(evals): preserve consumed commentary identity response Dispose only unread browser-session response bodies. Exercise the actual callback with real Node Responses so successful JSON is not followed by locked-stream cancellation. Preserve real reader and owner identity checks, restoration, denied-body disposal, malformed JSON failures, feedback privacy oracles, and original clocks. Co-Authored-By: Paperclip --- .../internal-commentary-reader.test.ts | 34 +++++++++++++++---- .../internal-commentary-reader.ts | 2 +- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/tests/runner-e2e/chat-catalog-domains/internal-commentary-reader.test.ts b/tests/runner-e2e/chat-catalog-domains/internal-commentary-reader.test.ts index 8ca74b2b1b8..bb23ae167fe 100644 --- a/tests/runner-e2e/chat-catalog-domains/internal-commentary-reader.test.ts +++ b/tests/runner-e2e/chat-catalog-domains/internal-commentary-reader.test.ts @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi, type MockInstance } from "vitest"; import type { ChatCatalogContext } from "../chat-catalog-contract.js"; const install = vi.hoisted(() => vi.fn(async (context: { restoreOwner(): void }) => context.restoreOwner())); vi.mock("./identity-auth.js", () => ({ installIdentitySession: install })); @@ -14,10 +14,16 @@ function fixture() { let apiUser = "owner"; let browserUser = "owner"; let source: { id: string; companyId: string; conversationUserId: string; conversationAgentId: string } | undefined; const state = { browserOverride: undefined as string | undefined, sourceOverride: undefined as string | undefined }; - const response = { ok: true, json: async () => ({ user: { id: state.browserOverride ?? browserUser } }), body: { cancel: vi.fn(async () => undefined) } }; - vi.stubGlobal("fetch", vi.fn(async () => response)); + let response: Response; + let cancel: MockInstance; + const transport = vi.fn(async () => { + response = Response.json({ user: { id: state.browserOverride ?? browserUser } }); + cancel = vi.spyOn(response.body!, "cancel"); + return response; + }); + vi.stubGlobal("fetch", transport); const context = { cookies: vi.fn(async () => [{ name: "session", value: browserUser === "owner" ? "owned-test-cookie" : "reader-test-cookie" }]), - request: { get: vi.fn(async () => response) }, restoreOwner() { browserUser = "owner"; } }; + request: { get: vi.fn(async () => response!) }, restoreOwner() { browserUser = "owner"; } }; const api = { baseURL: "http://127.0.0.1:3100", get: vi.fn(async () => ({ user: { id: apiUser } })), setBrowserSession: vi.fn(() => { apiUser = "owner"; }), post: vi.fn(async () => source = { id: "source", companyId: "11111111-1111-4111-8111-111111111111", conversationUserId: state.sourceOverride ?? apiUser, conversationAgentId: "lead" }) }; @@ -27,7 +33,7 @@ function fixture() { refreshIssue: vi.fn(async () => undefined), issue: () => source } as unknown as ChatCatalogContext; const person = { id: "casey", installSessionOn: vi.fn(async () => { browserUser = "casey"; }), bindApiSession: vi.fn(() => { apiUser = "casey"; }) } as unknown as Parameters[1]; - return { ctx, person, api, context, state, evidence, response, current: () => ({ apiUser, browserUser }) }; + return { ctx, person, api, context, state, evidence, transport, get response() { return response!; }, get cancel() { return cancel!; }, current: () => ({ apiUser, browserUser }) }; } beforeEach(() => { install.mockClear(); }); afterEach(() => { vi.unstubAllGlobals(); }); @@ -40,7 +46,8 @@ describe("real commentary reader identity lifetime", () => { expect(fetch).toHaveBeenCalledWith(new URL("/api/auth/get-session", f.api.baseURL), expect.objectContaining({ headers: { Cookie: "session=reader-test-cookie", Origin: f.api.baseURL } })); await selected.restore(); await selected.restore(); expect(f.current()).toEqual({ apiUser: "owner", browserUser: "owner" }); - expect(install).toHaveBeenCalledOnce(); expect(f.response.body.cancel).toHaveBeenCalledOnce(); + expect(install).toHaveBeenCalledOnce(); expect(f.cancel).not.toHaveBeenCalled(); + expect(f.response.bodyUsed).toBe(true); expect(f.response.body?.locked).toBe(true); }); it("restores genuine owner sessions after a browser/API identity mismatch", async () => { const f = fixture(); f.state.browserOverride = "different-reader"; @@ -54,6 +61,21 @@ describe("real commentary reader identity lifetime", () => { expect(f.current()).toEqual({ apiUser: "owner", browserUser: "owner" }); expect(f.evidence).not.toHaveBeenCalled(); }); + it("disposes an unread denied browser response without leaking its body or creating a reader", async () => { + const f = fixture(); const denied = new Response("private denial body must not escape", { status: 403 }); + const cancel = vi.spyOn(denied.body!, "cancel"); f.transport.mockImplementation(async () => denied); + const error = await installCommentaryReader(f.ctx, f.person, "owner").catch(error => error); + expect(error.message).toBe("Commentary browser session could not be independently read"); + expect(error.message).not.toContain("private denial"); expect(cancel).toHaveBeenCalledOnce(); + expect(f.current()).toEqual({ apiUser: "owner", browserUser: "owner" }); expect(f.evidence).not.toHaveBeenCalled(); + }); + it("preserves a malformed successful JSON failure instead of masking it with locked-stream cancellation", async () => { + const f = fixture(); const malformed = new Response("invalid JSON"); + const cancel = vi.spyOn(malformed.body!, "cancel"); f.transport.mockImplementation(async () => malformed); + await expect(installCommentaryReader(f.ctx, f.person, "owner")).rejects.toBeInstanceOf(SyntaxError); + expect(cancel).not.toHaveBeenCalled(); expect(f.current()).toEqual({ apiUser: "owner", browserUser: "owner" }); + expect(f.evidence).not.toHaveBeenCalled(); + }); it("rejects substituting the existing private owner for a distinct excluded reader", async () => { const f = fixture(); f.person.id = "owner"; await expect(installCommentaryReader(f.ctx, f.person, "owner")).rejects.toThrow("distinct real owner and reader"); diff --git a/tests/runner-e2e/chat-catalog-domains/internal-commentary-reader.ts b/tests/runner-e2e/chat-catalog-domains/internal-commentary-reader.ts index 84021125da8..2240842d814 100644 --- a/tests/runner-e2e/chat-catalog-domains/internal-commentary-reader.ts +++ b/tests/runner-e2e/chat-catalog-domains/internal-commentary-reader.ts @@ -45,7 +45,7 @@ export async function installCommentaryReader(ctx: ChatCatalogContext, person: P try { if (!response.ok) throw new Error("Commentary browser session could not be independently read"); browserPerson = await response.json(); - } finally { await response.body?.cancel(); } + } finally { if (!response.bodyUsed) await response.body?.cancel(); } if (apiPerson.user?.id !== person.id || browserPerson.user?.id !== person.id) throw new Error("Commentary API and browser do not represent the same real reader"); const source = await api.post<{ id: string; companyId: string; conversationUserId: string; conversationAgentId: string }>(