From 1a319b784740f57a7d4fc73d6b6d07f507a3bde3 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Mar 2026 20:16:57 +0000 Subject: [PATCH] test: add dedupe and boundary file helper coverage --- src/infra/boundary-file-read.test.ts | 204 +++++++++++++++++++++++++++ src/infra/dedupe.test.ts | 57 ++++++++ src/infra/infra-store.test.ts | 40 ------ 3 files changed, 261 insertions(+), 40 deletions(-) create mode 100644 src/infra/boundary-file-read.test.ts create mode 100644 src/infra/dedupe.test.ts diff --git a/src/infra/boundary-file-read.test.ts b/src/infra/boundary-file-read.test.ts new file mode 100644 index 00000000000..6869ace53f0 --- /dev/null +++ b/src/infra/boundary-file-read.test.ts @@ -0,0 +1,204 @@ +import path from "node:path"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const resolveBoundaryPathSyncMock = vi.hoisted(() => vi.fn()); +const resolveBoundaryPathMock = vi.hoisted(() => vi.fn()); +const openVerifiedFileSyncMock = vi.hoisted(() => vi.fn()); + +vi.mock("./boundary-path.js", () => ({ + resolveBoundaryPathSync: (...args: unknown[]) => resolveBoundaryPathSyncMock(...args), + resolveBoundaryPath: (...args: unknown[]) => resolveBoundaryPathMock(...args), +})); + +vi.mock("./safe-open-sync.js", () => ({ + openVerifiedFileSync: (...args: unknown[]) => openVerifiedFileSyncMock(...args), +})); + +const { canUseBoundaryFileOpen, openBoundaryFile, openBoundaryFileSync } = + await import("./boundary-file-read.js"); + +describe("boundary-file-read", () => { + beforeEach(() => { + resolveBoundaryPathSyncMock.mockReset(); + resolveBoundaryPathMock.mockReset(); + openVerifiedFileSyncMock.mockReset(); + }); + + it("recognizes the required sync fs surface", () => { + const validFs = { + openSync() {}, + closeSync() {}, + fstatSync() {}, + lstatSync() {}, + realpathSync() {}, + readFileSync() {}, + constants: {}, + } as never; + + expect(canUseBoundaryFileOpen(validFs)).toBe(true); + expect( + canUseBoundaryFileOpen({ + ...validFs, + openSync: undefined, + } as never), + ).toBe(false); + expect( + canUseBoundaryFileOpen({ + ...validFs, + constants: null, + } as never), + ).toBe(false); + }); + + it("maps sync boundary resolution into verified file opens", () => { + const stat = { size: 3 } as never; + const ioFs = { marker: "io" } as never; + const absolutePath = path.resolve("plugin.json"); + + resolveBoundaryPathSyncMock.mockReturnValue({ + canonicalPath: "/real/plugin.json", + rootCanonicalPath: "/real/root", + }); + openVerifiedFileSyncMock.mockReturnValue({ + ok: true, + path: "/real/plugin.json", + fd: 7, + stat, + }); + + const opened = openBoundaryFileSync({ + absolutePath: "plugin.json", + rootPath: "/workspace", + boundaryLabel: "plugin root", + ioFs, + }); + + expect(resolveBoundaryPathSyncMock).toHaveBeenCalledWith({ + absolutePath, + rootPath: "/workspace", + rootCanonicalPath: undefined, + boundaryLabel: "plugin root", + skipLexicalRootCheck: undefined, + }); + expect(openVerifiedFileSyncMock).toHaveBeenCalledWith({ + filePath: absolutePath, + resolvedPath: "/real/plugin.json", + rejectHardlinks: true, + maxBytes: undefined, + allowedType: undefined, + ioFs, + }); + expect(opened).toEqual({ + ok: true, + path: "/real/plugin.json", + fd: 7, + stat, + rootRealPath: "/real/root", + }); + }); + + it("returns validation errors when sync boundary resolution throws", () => { + const error = new Error("outside root"); + resolveBoundaryPathSyncMock.mockImplementation(() => { + throw error; + }); + + const opened = openBoundaryFileSync({ + absolutePath: "plugin.json", + rootPath: "/workspace", + boundaryLabel: "plugin root", + }); + + expect(opened).toEqual({ + ok: false, + reason: "validation", + error, + }); + expect(openVerifiedFileSyncMock).not.toHaveBeenCalled(); + }); + + it("guards against unexpected async sync-resolution results", () => { + resolveBoundaryPathSyncMock.mockReturnValue( + Promise.resolve({ + canonicalPath: "/real/plugin.json", + rootCanonicalPath: "/real/root", + }), + ); + + const opened = openBoundaryFileSync({ + absolutePath: "plugin.json", + rootPath: "/workspace", + boundaryLabel: "plugin root", + }); + + expect(opened.ok).toBe(false); + if (opened.ok) { + return; + } + expect(opened.reason).toBe("validation"); + expect(String(opened.error)).toContain("Unexpected async boundary resolution"); + }); + + it("awaits async boundary resolution before verifying the file", async () => { + const ioFs = { marker: "io" } as never; + const absolutePath = path.resolve("notes.txt"); + + resolveBoundaryPathMock.mockResolvedValue({ + canonicalPath: "/real/notes.txt", + rootCanonicalPath: "/real/root", + }); + openVerifiedFileSyncMock.mockReturnValue({ + ok: false, + reason: "validation", + error: new Error("blocked"), + }); + + const opened = await openBoundaryFile({ + absolutePath: "notes.txt", + rootPath: "/workspace", + boundaryLabel: "workspace", + aliasPolicy: { allowFinalSymlinkForUnlink: true }, + ioFs, + }); + + expect(resolveBoundaryPathMock).toHaveBeenCalledWith({ + absolutePath, + rootPath: "/workspace", + rootCanonicalPath: undefined, + boundaryLabel: "workspace", + policy: { allowFinalSymlinkForUnlink: true }, + skipLexicalRootCheck: undefined, + }); + expect(openVerifiedFileSyncMock).toHaveBeenCalledWith({ + filePath: absolutePath, + resolvedPath: "/real/notes.txt", + rejectHardlinks: true, + maxBytes: undefined, + allowedType: undefined, + ioFs, + }); + expect(opened).toEqual({ + ok: false, + reason: "validation", + error: expect.any(Error), + }); + }); + + it("maps async boundary resolution failures to validation errors", async () => { + const error = new Error("escaped"); + resolveBoundaryPathMock.mockRejectedValue(error); + + const opened = await openBoundaryFile({ + absolutePath: "notes.txt", + rootPath: "/workspace", + boundaryLabel: "workspace", + }); + + expect(opened).toEqual({ + ok: false, + reason: "validation", + error, + }); + expect(openVerifiedFileSyncMock).not.toHaveBeenCalled(); + }); +}); diff --git a/src/infra/dedupe.test.ts b/src/infra/dedupe.test.ts new file mode 100644 index 00000000000..035324e13c9 --- /dev/null +++ b/src/infra/dedupe.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from "vitest"; +import { createDedupeCache } from "./dedupe.js"; + +describe("createDedupeCache", () => { + it("ignores blank cache keys", () => { + const cache = createDedupeCache({ ttlMs: 1_000, maxSize: 10 }); + + expect(cache.check("", 100)).toBe(false); + expect(cache.check(undefined, 100)).toBe(false); + expect(cache.peek(null, 100)).toBe(false); + expect(cache.size()).toBe(0); + }); + + it("keeps entries indefinitely when ttlMs is zero or negative", () => { + const zeroTtlCache = createDedupeCache({ ttlMs: 0, maxSize: 10 }); + expect(zeroTtlCache.check("a", 100)).toBe(false); + expect(zeroTtlCache.check("a", 10_000)).toBe(true); + + const negativeTtlCache = createDedupeCache({ ttlMs: -100, maxSize: 10 }); + expect(negativeTtlCache.check("b", 100)).toBe(false); + expect(negativeTtlCache.peek("b", 10_000)).toBe(true); + }); + + it("touches duplicate reads so the newest key survives max-size pruning", () => { + const cache = createDedupeCache({ ttlMs: 10_000, maxSize: 2 }); + + expect(cache.check("a", 100)).toBe(false); + expect(cache.check("b", 200)).toBe(false); + expect(cache.check("a", 300)).toBe(true); + expect(cache.check("c", 400)).toBe(false); + + expect(cache.peek("a", 500)).toBe(true); + expect(cache.peek("b", 500)).toBe(false); + expect(cache.peek("c", 500)).toBe(true); + }); + + it("clears itself when maxSize floors to zero", () => { + const cache = createDedupeCache({ ttlMs: 1_000, maxSize: 0.9 }); + + expect(cache.check("a", 100)).toBe(false); + expect(cache.size()).toBe(0); + expect(cache.peek("a", 200)).toBe(false); + }); + + it("supports explicit reset", () => { + const cache = createDedupeCache({ ttlMs: 1_000, maxSize: 10 }); + + expect(cache.check("a", 100)).toBe(false); + expect(cache.check("b", 200)).toBe(false); + expect(cache.size()).toBe(2); + + cache.clear(); + + expect(cache.size()).toBe(0); + expect(cache.peek("a", 300)).toBe(false); + }); +}); diff --git a/src/infra/infra-store.test.ts b/src/infra/infra-store.test.ts index eb503473680..1b7e649db4d 100644 --- a/src/infra/infra-store.test.ts +++ b/src/infra/infra-store.test.ts @@ -2,7 +2,6 @@ import fs from "node:fs/promises"; import path from "node:path"; import { describe, expect, it } from "vitest"; import { withTempDir } from "../test-utils/temp-dir.js"; -import { createDedupeCache } from "./dedupe.js"; import { emitDiagnosticEvent, onDiagnosticEvent, @@ -87,43 +86,4 @@ describe("infra store", () => { expect(types).toEqual(["webhook.received", "message.queued", "session.state"]); }); }); - - describe("createDedupeCache", () => { - it("marks duplicates within TTL", () => { - const cache = createDedupeCache({ ttlMs: 1000, maxSize: 10 }); - expect(cache.check("a", 100)).toBe(false); - expect(cache.check("a", 500)).toBe(true); - }); - - it("expires entries after TTL", () => { - const cache = createDedupeCache({ ttlMs: 1000, maxSize: 10 }); - expect(cache.check("a", 100)).toBe(false); - expect(cache.check("a", 1501)).toBe(false); - }); - - it("evicts oldest entries when over max size", () => { - const cache = createDedupeCache({ ttlMs: 10_000, maxSize: 2 }); - expect(cache.check("a", 100)).toBe(false); - expect(cache.check("b", 200)).toBe(false); - expect(cache.check("c", 300)).toBe(false); - expect(cache.check("a", 400)).toBe(false); - }); - - it("prunes expired entries even when refreshed keys are older in insertion order", () => { - const cache = createDedupeCache({ ttlMs: 100, maxSize: 10 }); - expect(cache.check("a", 0)).toBe(false); - expect(cache.check("b", 50)).toBe(false); - expect(cache.check("a", 120)).toBe(false); - expect(cache.check("c", 200)).toBe(false); - expect(cache.size()).toBe(2); - }); - - it("supports non-mutating existence checks via peek()", () => { - const cache = createDedupeCache({ ttlMs: 1000, maxSize: 10 }); - expect(cache.peek("a", 100)).toBe(false); - expect(cache.check("a", 100)).toBe(false); - expect(cache.peek("a", 200)).toBe(true); - expect(cache.peek("a", 1201)).toBe(false); - }); - }); });