From 5fb30405059dc69f9b5df18d4fb7d859030a5a69 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Thu, 27 Aug 2026 11:25:17 -0400 Subject: [PATCH] test(core): scope skill discovery fixtures (#45464) --- packages/core/test/skill-discovery.test.ts | 271 ++++++++++----------- 1 file changed, 126 insertions(+), 145 deletions(-) diff --git a/packages/core/test/skill-discovery.test.ts b/packages/core/test/skill-discovery.test.ts index 850f4c2aea1..b8dc53e2896 100644 --- a/packages/core/test/skill-discovery.test.ts +++ b/packages/core/test/skill-discovery.test.ts @@ -1,173 +1,154 @@ import fs from "fs/promises" import path from "path" -import { describe, expect, test } from "bun:test" +import { createServer } from "node:http" +import { describe, expect } from "bun:test" +import { NodeHttpServer } from "@effect/platform-node" import { Effect } from "effect" +import { HttpServer, HttpServerRequest, HttpServerResponse } from "effect/unstable/http" import { AppNodeBuilder } from "@opencode-ai/core/effect/app-node-builder" import { Global } from "@opencode-ai/util/global" import { SkillDiscovery } from "@opencode-ai/core/skill/discovery" import { tmpdir } from "./fixture/tmpdir" +import { it } from "./lib/effect" -type Fixture = { - tmp: Awaited> - server: Bun.Server - state: { +const fixture = Effect.gen(function* () { + const tmp = yield* Effect.acquireDisposable(Effect.promise(() => tmpdir())) + const state: { skills: unknown[] files: Record requests: string[] - } - base: string -} - -async function pull(skills: unknown[], files: Record = {}, fixture?: Fixture) { - const state = fixture?.state ?? { skills, files, requests: [] } - state.skills = skills - state.files = files - state.requests = [] - const server = - fixture?.server ?? - Bun.serve({ - port: 0, - fetch(request) { - state.requests.push(request.url) - const pathname = new URL(request.url).pathname - const body = - pathname === "/catalog/index.json" ? JSON.stringify({ skills: state.skills }) : state.files[pathname] - return new Response(body ?? "Not Found", { status: body === undefined ? 404 : 200 }) - }, - }) - const tmp = fixture?.tmp ?? (await tmpdir()) - const base = fixture?.base ?? new URL("/catalog/", server.url).href - const skillDiscoveryLayer = AppNodeBuilder.build(SkillDiscovery.node, [ - [Global.node, Global.layerWith({ cache: tmp.path })], - ]) - const directories = await Effect.runPromise( + } = { skills: [], files: {}, requests: [] } + const server = yield* NodeHttpServer.make(createServer, { host: "127.0.0.1", port: 0 }) + const base = new URL("/catalog/", HttpServer.formatAddress(server.address)).href + yield* server.serve( Effect.gen(function* () { - return yield* (yield* SkillDiscovery.Service).pull(base) - }).pipe(Effect.provide(skillDiscoveryLayer)), + const request = yield* HttpServerRequest.HttpServerRequest + const url = new URL(request.url, base) + state.requests.push(url.href) + const body = + url.pathname === "/catalog/index.json" ? JSON.stringify({ skills: state.skills }) : state.files[url.pathname] + return HttpServerResponse.text(body ?? "Not Found", { status: body === undefined ? 404 : 200 }) + }), ) - return { tmp, server, state, base, requests: state.requests, directories } -} - -async function dispose(fixture: Fixture) { - await fixture.server.stop(true) - await fixture.tmp[Symbol.asyncDispose]() -} + return { + cache: tmp.path, + base, + pull: (skills: unknown[], files: Record = {}) => + Effect.gen(function* () { + state.skills = skills + state.files = files + state.requests = [] + // Rebuild per pull so repeated calls exercise the disk cache, not a shared service. + const directories = yield* Effect.gen(function* () { + const discovery = yield* SkillDiscovery.Service + return yield* discovery.pull(base) + }).pipe( + Effect.provide( + AppNodeBuilder.build(SkillDiscovery.node, [[Global.node, Global.layerWith({ cache: tmp.path })]]), + ), + ) + return { directories, requests: state.requests.slice() } + }), + } +}) describe("SkillDiscovery.pull", () => { - test("rejects skill name traversal without fetching files", async () => { - const result = await pull([{ name: "../outside", files: ["SKILL.md"] }]) - try { - expect(result.directories).toEqual([]) - expect(result.requests).toEqual([`${result.base}index.json`]) - expect(await fs.readdir(result.tmp.path)).toEqual([]) - } finally { - await dispose(result) - } - }) + for (const input of [ + { + name: "rejects skill name traversal without fetching files", + skills: [{ name: "../outside", files: ["SKILL.md"] }], + }, + { + name: "rejects file traversal without fetching files", + skills: [{ name: "deploy", files: ["SKILL.md", "../outside.md"] }], + }, + { + name: "rejects absolute file paths without fetching files", + skills: [{ name: "deploy", files: ["SKILL.md", "/tmp/outside.md"] }], + }, + { + name: "rejects cross-origin file URLs without fetching files", + skills: [{ name: "deploy", files: ["SKILL.md", "https://evil.example.test/outside.md"] }], + }, + ]) { + it.live( + input.name, + Effect.gen(function* () { + const catalog = yield* fixture + const result = yield* catalog.pull(input.skills) + expect(result.directories).toEqual([]) + expect(result.requests).toEqual([`${catalog.base}index.json`]) + expect(yield* Effect.promise(() => fs.readdir(catalog.cache))).toEqual([]) + }), + ) + } - test("rejects file traversal without fetching files", async () => { - const result = await pull([{ name: "deploy", files: ["SKILL.md", "../outside.md"] }]) - try { - expect(result.directories).toEqual([]) - expect(result.requests).toEqual([`${result.base}index.json`]) - expect(await fs.readdir(result.tmp.path)).toEqual([]) - } finally { - await dispose(result) - } - }) - - test("rejects absolute file paths without fetching files", async () => { - const result = await pull([{ name: "deploy", files: ["SKILL.md", "/tmp/outside.md"] }]) - try { - expect(result.directories).toEqual([]) - expect(result.requests).toEqual([`${result.base}index.json`]) - expect(await fs.readdir(result.tmp.path)).toEqual([]) - } finally { - await dispose(result) - } - }) - - test("rejects cross-origin file URLs without fetching files", async () => { - const result = await pull([{ name: "deploy", files: ["SKILL.md", "https://evil.example.test/outside.md"] }]) - try { - expect(result.directories).toEqual([]) - expect(result.requests).toEqual([`${result.base}index.json`]) - expect(await fs.readdir(result.tmp.path)).toEqual([]) - } finally { - await dispose(result) - } - }) - - test("downloads safe nested files under the skill root", async () => { - const result = await pull([{ name: "deploy", files: ["SKILL.md", "references/guide.md"] }], { - "/catalog/deploy/SKILL.md": "# Deploy", - "/catalog/deploy/references/guide.md": "# Guide", - }) - try { + it.live( + "downloads safe nested files under the skill root", + Effect.gen(function* () { + const catalog = yield* fixture + const result = yield* catalog.pull([{ name: "deploy", files: ["SKILL.md", "references/guide.md"] }], { + "/catalog/deploy/SKILL.md": "# Deploy", + "/catalog/deploy/references/guide.md": "# Guide", + }) expect(result.directories).toHaveLength(1) expect(result.requests.toSorted()).toEqual( [ - `${result.base}index.json`, - `${result.base}deploy/SKILL.md`, - `${result.base}deploy/references/guide.md`, + `${catalog.base}index.json`, + `${catalog.base}deploy/SKILL.md`, + `${catalog.base}deploy/references/guide.md`, ].toSorted(), ) - expect(await fs.readFile(path.join(result.directories[0], "SKILL.md"), "utf8")).toBe("# Deploy") - expect(await fs.readFile(path.join(result.directories[0], "references", "guide.md"), "utf8")).toBe("# Guide") - } finally { - await dispose(result) - } - }) - - test("refreshes cached files when the version changes", async () => { - const first = await pull([{ name: "deploy", version: "1", files: ["SKILL.md"] }], { - "/catalog/deploy/SKILL.md": "# Old", - }) - try { - const second = await pull( - [{ name: "deploy", version: "2", files: ["SKILL.md"] }], - { "/catalog/deploy/SKILL.md": "# New" }, - first, + expect(yield* Effect.promise(() => Bun.file(path.join(result.directories[0], "SKILL.md")).text())).toBe( + "# Deploy", ) + expect( + yield* Effect.promise(() => Bun.file(path.join(result.directories[0], "references", "guide.md")).text()), + ).toBe("# Guide") + }), + ) - expect(await fs.readFile(path.join(first.directories[0], "SKILL.md"), "utf8")).toBe("# New") - expect(second.requests).toContain(`${first.base}deploy/SKILL.md`) - const third = await pull( - [{ name: "deploy", version: "2", files: ["SKILL.md"] }], - { "/catalog/deploy/SKILL.md": "# Ignored" }, - first, - ) - expect(third.requests).toEqual([`${first.base}index.json`]) - } finally { - await dispose(first) - } - }) + it.live( + "refreshes cached files when the version changes", + Effect.gen(function* () { + const catalog = yield* fixture + const first = yield* catalog.pull([{ name: "deploy", version: "1", files: ["SKILL.md"] }], { + "/catalog/deploy/SKILL.md": "# Old", + }) + const second = yield* catalog.pull([{ name: "deploy", version: "2", files: ["SKILL.md"] }], { + "/catalog/deploy/SKILL.md": "# New", + }) - test("publishes complete updates and removes stale files", async () => { - const first = await pull([{ name: "deploy", version: "1", files: ["SKILL.md", "old.md"] }], { - "/catalog/deploy/SKILL.md": "# Old", - "/catalog/deploy/old.md": "old reference", - }) - try { + expect(yield* Effect.promise(() => Bun.file(path.join(first.directories[0], "SKILL.md")).text())).toBe("# New") + expect(second.requests).toContain(`${catalog.base}deploy/SKILL.md`) + const third = yield* catalog.pull([{ name: "deploy", version: "2", files: ["SKILL.md"] }], { + "/catalog/deploy/SKILL.md": "# Ignored", + }) + expect(third.requests).toEqual([`${catalog.base}index.json`]) + }), + ) + + it.live( + "publishes complete updates and removes stale files", + Effect.gen(function* () { + const catalog = yield* fixture + const first = yield* catalog.pull([{ name: "deploy", version: "1", files: ["SKILL.md", "old.md"] }], { + "/catalog/deploy/SKILL.md": "# Old", + "/catalog/deploy/old.md": "old reference", + }) const root = first.directories[0] - await pull( - [{ name: "deploy", version: "2", files: ["SKILL.md", "missing.md"] }], - { "/catalog/deploy/SKILL.md": "# Partial" }, - first, - ) - expect(await fs.readFile(path.join(root, "SKILL.md"), "utf8")).toBe("# Old") - expect(await fs.readFile(path.join(root, "old.md"), "utf8")).toBe("old reference") + yield* catalog.pull([{ name: "deploy", version: "2", files: ["SKILL.md", "missing.md"] }], { + "/catalog/deploy/SKILL.md": "# Partial", + }) + expect(yield* Effect.promise(() => Bun.file(path.join(root, "SKILL.md")).text())).toBe("# Old") + expect(yield* Effect.promise(() => Bun.file(path.join(root, "old.md")).text())).toBe("old reference") - await pull( - [{ name: "deploy", version: "3", files: ["SKILL.md"] }], - { "/catalog/deploy/SKILL.md": "# New" }, - first, - ) - expect(await fs.readFile(path.join(root, "SKILL.md"), "utf8")).toBe("# New") - expect(await Bun.file(path.join(root, "old.md")).exists()).toBe(false) - } finally { - await dispose(first) - } - }) + yield* catalog.pull([{ name: "deploy", version: "3", files: ["SKILL.md"] }], { + "/catalog/deploy/SKILL.md": "# New", + }) + expect(yield* Effect.promise(() => Bun.file(path.join(root, "SKILL.md")).text())).toBe("# New") + expect(yield* Effect.promise(() => Bun.file(path.join(root, "old.md")).exists())).toBe(false) + }), + ) })