diff --git a/packages/core/package.json b/packages/core/package.json index 8de83c0..ab03334 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -9,7 +9,7 @@ "db": "bun drizzle-kit", "migration": "bun run script/migration.ts", "fix-node-pty": "bun run script/fix-node-pty.ts", - "test": "bun test --only-failures", + "test": "bun test --timeout 30000 --only-failures", "typecheck": "tsgo --noEmit" }, "bin": { diff --git a/packages/opencode/src/server/routes/instance/httpapi/groups/file.ts b/packages/opencode/src/server/routes/instance/httpapi/groups/file.ts index 389bf91..31031e7 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/groups/file.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/groups/file.ts @@ -2,7 +2,7 @@ import { FileSystem } from "@opencode-ai/core/filesystem" import { NonNegativeInt } from "@opencode-ai/core/schema" import { LSP } from "@/lsp/lsp" import { Schema } from "effect" -import { HttpApi, HttpApiEndpoint, HttpApiGroup, OpenApi } from "effect/unstable/httpapi" +import { HttpApi, HttpApiEndpoint, HttpApiError, HttpApiGroup, OpenApi } from "effect/unstable/httpapi" import { Authorization } from "../middleware/authorization" import { InstanceContextMiddleware } from "../middleware/instance-context" import { @@ -108,6 +108,7 @@ export const FileApi = HttpApi.make("file") HttpApiEndpoint.get("findText", FilePaths.findText, { query: FindTextQuery, success: described(Schema.Array(LegacyMatch), "Matches"), + error: HttpApiError.BadRequest, }).annotateMerge( OpenApi.annotations({ identifier: "find.text", @@ -118,6 +119,7 @@ export const FileApi = HttpApi.make("file") HttpApiEndpoint.get("findFile", FilePaths.findFile, { query: FindFileQuery, success: described(Schema.Array(Schema.String), "File paths"), + error: HttpApiError.BadRequest, }).annotateMerge( OpenApi.annotations({ identifier: "find.files", @@ -128,6 +130,7 @@ export const FileApi = HttpApi.make("file") HttpApiEndpoint.get("findSymbol", FilePaths.findSymbol, { query: FindSymbolQuery, success: described(Schema.Array(LSP.Symbol), "Symbols"), + error: HttpApiError.BadRequest, }).annotateMerge( OpenApi.annotations({ identifier: "find.symbols", @@ -138,6 +141,7 @@ export const FileApi = HttpApi.make("file") HttpApiEndpoint.get("list", FilePaths.list, { query: FileQuery, success: described(Schema.Array(LegacyEntry), "Files and directories"), + error: HttpApiError.BadRequest, }).annotateMerge( OpenApi.annotations({ identifier: "file.list", @@ -148,6 +152,7 @@ export const FileApi = HttpApi.make("file") HttpApiEndpoint.get("content", FilePaths.content, { query: FileQuery, success: described(LegacyContent, "File content"), + error: HttpApiError.BadRequest, }).annotateMerge( OpenApi.annotations({ identifier: "file.read", @@ -158,6 +163,7 @@ export const FileApi = HttpApi.make("file") HttpApiEndpoint.get("status", FilePaths.status, { query: WorkspaceRoutingQuery, success: described(Schema.Array(LegacyStatus), "File status"), + error: HttpApiError.BadRequest, }).annotateMerge( OpenApi.annotations({ identifier: "file.status", diff --git a/packages/opencode/src/server/routes/instance/httpapi/handlers/file.ts b/packages/opencode/src/server/routes/instance/httpapi/handlers/file.ts index 6a82602..02cebc3 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/file.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/file.ts @@ -8,7 +8,7 @@ import { AbsolutePath, RelativePath } from "@opencode-ai/core/schema" import { Effect, Layer, Option } from "effect" import ignore from "ignore" import path from "path" -import { HttpApiBuilder } from "effect/unstable/httpapi" +import { HttpApiBuilder, HttpApiError } from "effect/unstable/httpapi" import { InstanceHttpApi } from "../api" export const fileHandlers = HttpApiBuilder.group(InstanceHttpApi, "file", (handlers) => @@ -96,7 +96,7 @@ export const fileHandlers = HttpApiBuilder.group(InstanceHttpApi, "file", (handl const content = Effect.fn("FileHttpApi.content")(function* (ctx: { query: { path: string } }) { const directory = (yield* InstanceState.context).directory const file = path.resolve(directory, ctx.query.path) - if (!FSUtil.contains(directory, file)) return yield* Effect.die(new Error("Path escapes the location")) + if (!FSUtil.contains(directory, file)) return yield* new HttpApiError.BadRequest({}) if (!(yield* FSUtil.Service.use((fs) => fs.existsSafe(file)))) return { type: "text" as const, content: "" } return yield* filesystem( FileSystem.Service.use((fs) => fs.read({ path: RelativePath.make(ctx.query.path) })), diff --git a/packages/opencode/test/cli/acp/lifecycle.test.ts b/packages/opencode/test/cli/acp/lifecycle.test.ts index 9f2558e..9806c08 100644 --- a/packages/opencode/test/cli/acp/lifecycle.test.ts +++ b/packages/opencode/test/cli/acp/lifecycle.test.ts @@ -18,7 +18,7 @@ describe("opencode acp lifecycle subprocess", () => { const acp = yield* opencode.acp() acp.close() - const code = yield* Effect.promise(() => acp.exited).pipe(Effect.timeout(Duration.seconds(5))) + const code = yield* Effect.promise(() => acp.exited).pipe(Effect.timeout(Duration.seconds(15))) expect(code).toBe(0) }), 60_000, diff --git a/packages/opencode/test/control-plane/workspace.test.ts b/packages/opencode/test/control-plane/workspace.test.ts index a0d3aad..6cd1a06 100644 --- a/packages/opencode/test/control-plane/workspace.test.ts +++ b/packages/opencode/test/control-plane/workspace.test.ts @@ -1696,6 +1696,6 @@ describe("workspace waitForSync", () => { ) }), { git: true }, - 7000, + 30_000, ) }) diff --git a/packages/opencode/test/project/instance-bootstrap.test.ts b/packages/opencode/test/project/instance-bootstrap.test.ts index f855024..69c5a32 100644 --- a/packages/opencode/test/project/instance-bootstrap.test.ts +++ b/packages/opencode/test/project/instance-bootstrap.test.ts @@ -4,8 +4,9 @@ import path from "node:path" import { pathToFileURL } from "node:url" import { LayerNode } from "@opencode-ai/core/effect/layer-node" import { CrossSpawnSpawner } from "@opencode-ai/core/cross-spawn-spawner" -import { Cause, Effect, Exit, Fiber } from "effect" +import { Cause, Effect, Exit, Fiber, Layer } from "effect" import { bootstrap as cliBootstrap } from "../../src/cli/bootstrap" +import { InstanceState } from "../../src/effect/instance-state" import { InstanceBootstrap } from "../../src/project/bootstrap" import { InstanceStore } from "../../src/project/instance-store" import { disposeAllInstances, tmpdirScoped } from "../fixture/fixture" @@ -17,6 +18,25 @@ const it = testEffect( [InstanceStore.bootstrapNode, InstanceBootstrap.node], ]), ) +// The provide boundary only needs to prove InstanceStore runs the bootstrap +// service before user effects. Keep it isolated from the slower full bootstrap +// graph; the CLI/reload tests below still cover the real bootstrap wiring. +const provideBoundaryIt = testEffect( + LayerNode.compile(LayerNode.group([InstanceStore.node, CrossSpawnSpawner.node]), [ + [ + InstanceStore.bootstrapNode, + Layer.succeed( + InstanceBootstrap.Service, + InstanceBootstrap.Service.of({ + run: Effect.gen(function* () { + const ctx = yield* InstanceState.context + yield* Effect.promise(() => Bun.write(path.join(ctx.directory, "config-hook-fired"), "ran")) + }), + }), + ), + ], + ]), +) // InstanceBootstrap must run before any code touches the instance — // originally tracked by PRs #25389 and #25449, now a permanent @@ -30,9 +50,15 @@ afterEach(async () => { await disposeAllInstances() }) -const bootstrapFixture = Effect.gen(function* () { +const markerFixture = Effect.gen(function* () { const dir = yield* tmpdirScoped({ git: true }) const marker = path.join(dir, "config-hook-fired") + return { directory: dir, marker } +}) + +const bootstrapFixture = Effect.gen(function* () { + const fixture = yield* markerFixture + const { directory: dir, marker } = fixture const pluginFile = path.join(dir, "plugin.ts") yield* Effect.promise(() => Bun.write( @@ -57,7 +83,7 @@ const bootstrapFixture = Effect.gen(function* () { }), ), ) - return { directory: dir, marker } + return fixture }) function waitDisposed(directory: string) { @@ -67,14 +93,17 @@ function waitDisposed(directory: string) { }) } -it.live("InstanceStore.provide runs InstanceBootstrap before effect", () => +provideBoundaryIt.live("InstanceStore.provide runs InstanceBootstrap before effect", () => Effect.gen(function* () { - const tmp = yield* bootstrapFixture + const tmp = yield* markerFixture const store = yield* InstanceStore.Service - yield* store.provide({ directory: tmp.directory }, Effect.succeed("ok")) - - expect(existsSync(tmp.marker)).toBe(true) + yield* store.provide( + { directory: tmp.directory }, + Effect.sync(() => { + expect(existsSync(tmp.marker)).toBe(true) + }), + ) }), ) diff --git a/packages/opencode/test/server/httpapi-file.test.ts b/packages/opencode/test/server/httpapi-file.test.ts index ed882ad..c2728dd 100644 --- a/packages/opencode/test/server/httpapi-file.test.ts +++ b/packages/opencode/test/server/httpapi-file.test.ts @@ -9,11 +9,13 @@ import { pollWithTimeout } from "../lib/effect" const context = Context.empty() as Context.Context -function request(route: string, directory: string, query?: Record) { +type QueryParams = Record + +function request(route: string, directory: string, query?: QueryParams) { const url = new URL(`http://localhost${route}`) - for (const [key, value] of Object.entries(query ?? {})) { - url.searchParams.set(key, value) - } + Object.entries(query ?? {}).forEach(([key, value]) => + (Array.isArray(value) ? value : [value]).forEach((item) => url.searchParams.append(key, item)), + ) return HttpApiApp.webHandler().handler( new Request(url, { headers: { @@ -80,4 +82,33 @@ describe("file HttpApi", () => { expect(symbols.status).toBe(200) expect(await symbols.json()).toEqual([]) }) + + test("returns bad request for invalid route queries", async () => { + await using tmp = await tmpdir({ git: true }) + + const cases: Array<{ route: string; query?: QueryParams }> = [ + { route: FilePaths.findText }, + { route: FilePaths.findFile }, + { route: FilePaths.findSymbol }, + { route: FilePaths.list }, + { route: FilePaths.content }, + { route: FilePaths.status, query: { directory: [tmp.path, tmp.path] } }, + ] + const responses = await Promise.all(cases.map((item) => request(item.route, tmp.path, item.query))) + + await Promise.all( + responses.map(async (response, index) => { + expect(response.status, cases[index]!.route).toBe(400) + expect(await response.json()).toMatchObject({ name: "BadRequest" }) + }), + ) + }) + + test("rejects file content paths outside the routed directory", async () => { + await using tmp = await tmpdir({ git: true }) + + const response = await request(FilePaths.content, tmp.path, { path: "../outside.txt" }) + + expect(response.status).toBe(400) + }) }) diff --git a/packages/opencode/test/server/httpapi-public-openapi.test.ts b/packages/opencode/test/server/httpapi-public-openapi.test.ts index 63527d6..5139077 100644 --- a/packages/opencode/test/server/httpapi-public-openapi.test.ts +++ b/packages/opencode/test/server/httpapi-public-openapi.test.ts @@ -245,6 +245,18 @@ describe("PublicApi OpenAPI v2 errors", () => { ) }) + test("documents legacy file route query errors", () => { + const spec = OpenApi.fromApi(PublicApi) as OpenApiSpec + + const fileRoutes = ["/find", "/find/file", "/find/symbol", "/file", "/file/content", "/file/status"] + + fileRoutes.forEach((route) => { + expect(componentNames(spec.paths[route]?.get?.responses?.["400"]), route).toContain( + "effect_HttpApiError_BadRequest", + ) + }) + }) + test("documents v2 unfinished session mutation errors", () => { const spec = OpenApi.fromApi(PublicApi) as OpenApiSpec