From b259709590edf0939eea372e0e91eaa3e03cbd31 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Wed, 2 Sep 2026 22:45:07 -0400 Subject: [PATCH] fix(core): attribute deferred skill plugin failures --- .../client/src/promise/generated/client.ts | 2 +- .../client/src/promise/generated/types.ts | 9 ++ packages/client/test/effect.test.ts | 17 +++ packages/client/test/promise.test.ts | 17 ++- packages/core/src/plugin.ts | 4 +- packages/core/src/plugin/callback.ts | 14 ++ packages/core/src/plugin/host.ts | 20 +++ .../core/test/plugin/skill-failures.test.ts | 95 +++++++++++++ packages/protocol/openapi.json | 31 +++++ packages/protocol/src/errors.ts | 11 ++ packages/protocol/src/groups/skill.ts | 2 + packages/server/src/handlers/skill.ts | 23 +++- packages/server/test/skill-failures.test.ts | 129 ++++++++++++++++++ packages/www/openapi.json | 31 +++++ packages/www/public/openapi.json | 31 +++++ 15 files changed, 430 insertions(+), 6 deletions(-) create mode 100644 packages/core/src/plugin/callback.ts create mode 100644 packages/core/test/plugin/skill-failures.test.ts create mode 100644 packages/server/test/skill-failures.test.ts diff --git a/packages/client/src/promise/generated/client.ts b/packages/client/src/promise/generated/client.ts index 33f55761c798..74a31e32b318 100644 --- a/packages/client/src/promise/generated/client.ts +++ b/packages/client/src/promise/generated/client.ts @@ -1634,7 +1634,7 @@ export function make(options: ClientOptions) { path: `/api/skill`, query: { location: input?.["location"] }, successStatus: 200, - declaredStatuses: [400, 401], + declaredStatuses: [400, 401, 500], empty: false, }, requestOptions, diff --git a/packages/client/src/promise/generated/types.ts b/packages/client/src/promise/generated/types.ts index e8036b9f5417..3baeb3360ff4 100644 --- a/packages/client/src/promise/generated/types.ts +++ b/packages/client/src/promise/generated/types.ts @@ -2488,6 +2488,15 @@ export type PermissionNotFoundError = { export const isPermissionNotFoundError = (value: unknown): value is PermissionNotFoundError => typeof value === "object" && value !== null && "_tag" in value && value["_tag"] === "PermissionNotFoundError" +export type PluginCallbackError = { + readonly _tag: "PluginCallbackError" + readonly pluginID: string + readonly operation: "skill.transform" + readonly message: string +} +export const isPluginCallbackError = (value: unknown): value is PluginCallbackError => + typeof value === "object" && value !== null && "_tag" in value && value["_tag"] === "PluginCallbackError" + export type RpcError = { readonly _tag: "RpcError" readonly type: string diff --git a/packages/client/test/effect.test.ts b/packages/client/test/effect.test.ts index 5935dcb3d024..0e6b2d801482 100644 --- a/packages/client/test/effect.test.ts +++ b/packages/client/test/effect.test.ts @@ -15,6 +15,23 @@ import { const synced = { type: "log.synced" as const, aggregateID: "ses_test", seq: Event.Seq.make(1) } +test("skill.list decodes a declared plugin callback failure", async () => { + const failure = { + _tag: "PluginCallbackError", + pluginID: "broken-skills", + operation: "skill.transform", + message: 'Plugin "broken-skills" failed during skill.transform. Check server logs for details.', + } + const httpClient = HttpClient.make((request) => + Effect.succeed(HttpClientResponse.fromWeb(request, Response.json(failure, { status: 500 }))), + ) + const error = await Effect.gen(function* () { + const client = yield* OpenCode.make({ baseUrl: "http://localhost:3000" }) + return yield* client.skill.list().pipe(Effect.flip) + }).pipe(Effect.provideService(HttpClient.HttpClient, httpClient), Effect.runPromise) + expect(error).toMatchObject(failure) +}) + test("health.get decodes the readiness response", async () => { const httpClient = HttpClient.make((request) => Effect.succeed(HttpClientResponse.fromWeb(request, Response.json({ healthy: true, version: "old", pid: 123 }))), diff --git a/packages/client/test/promise.test.ts b/packages/client/test/promise.test.ts index 989900e83f5b..dffbf71315c8 100644 --- a/packages/client/test/promise.test.ts +++ b/packages/client/test/promise.test.ts @@ -1,5 +1,20 @@ import { expect, test } from "bun:test" -import { isSessionNotFoundError, isUnauthorizedError, OpenCode } from "../src/promise/index" +import { isPluginCallbackError, isSessionNotFoundError, isUnauthorizedError, OpenCode } from "../src/promise/index" + +test("skill.list preserves a declared plugin callback failure", async () => { + const failure = { + _tag: "PluginCallbackError", + pluginID: "broken-skills", + operation: "skill.transform", + message: 'Plugin "broken-skills" failed during skill.transform. Check server logs for details.', + } + const client = OpenCode.make({ + baseUrl: "http://localhost:3000", + fetch: async () => Response.json(failure, { status: 500 }), + }) + await expect(client.skill.list()).rejects.toEqual(failure) + expect(isPluginCallbackError(failure)).toBe(true) +}) test("exposes every standard HTTP API group", () => { const client = OpenCode.make({ baseUrl: "http://localhost:3000" }) diff --git a/packages/core/src/plugin.ts b/packages/core/src/plugin.ts index 24913ec63972..6e7a81fe4395 100644 --- a/packages/core/src/plugin.ts +++ b/packages/core/src/plugin.ts @@ -43,9 +43,7 @@ const layer = Layer.effect( const load = Effect.fnUntraced(function* (plugin: Generation) { const child = yield* Scope.fork(scope) const inherit = yield* State.inherit() - const loaded = yield* Effect.suspend(() => - plugin.effect({ ...host, storage: PluginHost.storage(kv, plugin.id) }), - ).pipe( + const loaded = yield* Effect.suspend(() => plugin.effect(PluginHost.forPlugin(host, kv, plugin.id))).pipe( inherit, Effect.updateContext((context: Context.Context) => Context.make(Scope.Scope, child).pipe( diff --git a/packages/core/src/plugin/callback.ts b/packages/core/src/plugin/callback.ts new file mode 100644 index 000000000000..c40f9efc75b1 --- /dev/null +++ b/packages/core/src/plugin/callback.ts @@ -0,0 +1,14 @@ +export * as PluginCallback from "./callback.js" + +import { Data } from "effect" + +/** Local failure detail. Transport boundaries must explicitly select public fields. */ +export class Error extends Data.TaggedError("PluginCallbackError")<{ + readonly pluginID: string + readonly operation: "skill.transform" + readonly cause: unknown +}> { + override get message() { + return `Plugin "${this.pluginID}" failed during ${this.operation}.` + } +} diff --git a/packages/core/src/plugin/host.ts b/packages/core/src/plugin/host.ts index f9cf0b762356..0354e66f7f46 100644 --- a/packages/core/src/plugin/host.ts +++ b/packages/core/src/plugin/host.ts @@ -33,6 +33,7 @@ import { WebSearch } from "../websearch.js" import { Generate } from "../generate.js" import { Permission } from "../permission.js" import { PluginHooks } from "./hooks.js" +import { PluginCallback } from "./callback.js" import type { Interface } from "../plugin.js" import { LayerNode } from "@opencode-ai/util/effect/layer-node" @@ -518,6 +519,25 @@ export const requirements = LayerNode.group([ LocationServiceMap.node, ]) +export function forPlugin(host: Plugin.Context, kv: KV.Interface, pluginID: string): Plugin.Context { + return { + ...host, + storage: storage(kv, pluginID), + skill: { + ...host.skill, + transform: (callback) => + host.skill.transform((editor) => { + try { + callback(editor) + } catch (cause) { + // Replay happens after setup, potentially in a different consumer's Effect context. + throw new PluginCallback.Error({ pluginID, operation: "skill.transform", cause }) + } + }), + }, + } +} + export function storage(kv: KV.Interface, pluginID: string): Plugin.Context["storage"] { const namespace = `plugin:${pluginID .split("") diff --git a/packages/core/test/plugin/skill-failures.test.ts b/packages/core/test/plugin/skill-failures.test.ts new file mode 100644 index 000000000000..cfd9d08aa38f --- /dev/null +++ b/packages/core/test/plugin/skill-failures.test.ts @@ -0,0 +1,95 @@ +import { expect } from "bun:test" +import { Cause, Effect, Exit } from "effect" +import { Plugin } from "@opencode-ai/core/plugin" +import { Skill } from "@opencode-ai/core/skill" +import { AbsolutePath } from "@opencode-ai/schema/schema" +import { testEffect } from "../lib/effect" +import { PluginTestLayer } from "./fixture" + +const it = testEffect(PluginTestLayer) +const skill = { + id: Skill.ID.make("review"), + name: Skill.Name.make("Review"), + description: "Review changes", + location: AbsolutePath.make("/fixture/review.md"), + content: "Review changes", +} + +for (const cause of [new TypeError("synthetic-private-detail"), "synthetic-private-detail"]) { + it.effect(`attributes a deferred skill transform throwing ${typeof cause}`, () => + Effect.gen(function* () { + const plugins = yield* Plugin.Service + const skills = yield* Skill.Service + let setup = false + const activation = yield* plugins + .activate([ + { + id: "healthy", + revision: "1", + effect: (ctx) => ctx.skill.transform((editor) => editor.add(skill)).pipe(Effect.asVoid), + }, + { + id: "broken-skills", + revision: "1", + effect: (ctx) => + Effect.gen(function* () { + yield* ctx.skill.transform((editor) => { + editor.remove(skill.id) + throw cause + }) + setup = true + }), + }, + ]) + .pipe(Effect.exit) + + expect(setup).toBe(true) + // Neither a partial fold nor the old value is returned after failure. Every read retries. + for (const exit of [ + activation, + yield* skills.list().pipe(Effect.asVoid, Effect.exit), + yield* skills.get(skill.id).pipe(Effect.asVoid, Effect.exit), + ]) { + if (Exit.isSuccess(exit)) throw new Error("Expected a failed skill fold") + expect(Cause.hasFails(exit.cause)).toBe(false) + expect(Cause.squash(exit.cause)).toMatchObject({ + _tag: "PluginCallbackError", + pluginID: "broken-skills", + operation: "skill.transform", + message: 'Plugin "broken-skills" failed during skill.transform.', + cause, + }) + } + + // Only an explicit registration change removes the failure; nothing is silently disabled. + yield* plugins.activate([ + { + id: "healthy", + revision: "1", + effect: (ctx) => ctx.skill.transform((editor) => editor.add(skill)).pipe(Effect.asVoid), + }, + ]) + expect(yield* skills.list()).toEqual([skill]) + }), + ) +} + +it.effect("keeps setup failures distinct from deferred callback failures", () => + Effect.gen(function* () { + const plugins = yield* Plugin.Service + const skills = yield* Skill.Service + yield* plugins.activate([ + { id: "setup-failure", revision: "1", effect: () => Effect.die(new Error("fixture setup failed")) }, + { + id: "healthy", + revision: "1", + effect: (ctx) => ctx.skill.transform((editor) => editor.add(skill)).pipe(Effect.asVoid), + }, + ]) + expect((yield* plugins.list()).find((plugin) => plugin.id === "setup-failure")?.state).toMatchObject({ + status: "failed", + error: expect.stringContaining("fixture setup failed"), + }) + expect(yield* skills.list()).toEqual([skill]) + }), +) diff --git a/packages/protocol/openapi.json b/packages/protocol/openapi.json index 97b3282be2d7..a2c0da1be39a 100644 --- a/packages/protocol/openapi.json +++ b/packages/protocol/openapi.json @@ -9259,6 +9259,16 @@ } } } + }, + "500": { + "description": "PluginCallbackError", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PluginCallbackErrorEncoded" + } + } + } } }, "description": "Retrieve currently registered skills.", @@ -16984,6 +16994,27 @@ } ] }, + "PluginCallbackErrorEncoded": { + "type": "object", + "properties": { + "_tag": { + "type": "string", + "enum": ["PluginCallbackError"] + }, + "pluginID": { + "type": "string" + }, + "operation": { + "type": "string", + "enum": ["skill.transform"] + }, + "message": { + "type": "string" + } + }, + "required": ["_tag", "pluginID", "operation", "message"], + "additionalProperties": false + }, "Project": { "type": "object", "properties": { diff --git a/packages/protocol/src/errors.ts b/packages/protocol/src/errors.ts index 54f8b396638f..9f352be9e287 100644 --- a/packages/protocol/src/errors.ts +++ b/packages/protocol/src/errors.ts @@ -1,5 +1,16 @@ import { Schema } from "effect" import { Skill } from "@opencode-ai/schema/skill" +import { Plugin } from "@opencode-ai/schema/plugin" + +export class PluginCallbackError extends Schema.TaggedError()( + "PluginCallbackError", + { + pluginID: Plugin.ID, + operation: Schema.Literal("skill.transform"), + message: Schema.String, + }, + { httpApiStatus: 500 }, +) {} export class InvalidRequestError extends Schema.TaggedError()( "InvalidRequestError", diff --git a/packages/protocol/src/groups/skill.ts b/packages/protocol/src/groups/skill.ts index 9d4e372633d1..a3a8e4119683 100644 --- a/packages/protocol/src/groups/skill.ts +++ b/packages/protocol/src/groups/skill.ts @@ -3,12 +3,14 @@ import { Location } from "@opencode-ai/schema/location" import { Schema } from "effect" import { HttpApiEndpoint, HttpApiGroup, OpenApi } from "effect/unstable/httpapi" import { LocationQuery, locationQueryOpenApi } from "./location.js" +import { PluginCallbackError } from "../errors.js" export const SkillGroup = HttpApiGroup.make("server.skill") .add( HttpApiEndpoint.get("skill.list", "/api/skill", { query: LocationQuery, success: Location.response(Schema.Array(Skill.Info)), + error: PluginCallbackError, }) .annotateMerge(locationQueryOpenApi) .annotateMerge( diff --git a/packages/server/src/handlers/skill.ts b/packages/server/src/handlers/skill.ts index 2b22851253dd..3ddeda7e74f3 100644 --- a/packages/server/src/handlers/skill.ts +++ b/packages/server/src/handlers/skill.ts @@ -1,8 +1,29 @@ import { Skill } from "@opencode-ai/core/skill" +import { PluginCallback } from "@opencode-ai/core/plugin/callback" +import { PluginCallbackError } from "@opencode-ai/protocol/errors" +import { Plugin } from "@opencode-ai/schema/plugin" +import { Effect } from "effect" import { HttpApiBuilder } from "effect/unstable/httpapi" import { Api } from "../api" import { response } from "../location" export const SkillHandler = HttpApiBuilder.group(Api, "server.skill", (handlers) => - handlers.handle("skill.list", () => response(Skill.Service.use((skill) => skill.list()))), + handlers.handle("skill.list", () => + response(Skill.Service.use((skill) => skill.list())).pipe( + Effect.catchDefect((error) => { + if (!(error instanceof PluginCallback.Error)) return Effect.die(error) + return Effect.logError("Plugin callback failed", error).pipe( + Effect.andThen( + Effect.fail( + new PluginCallbackError({ + pluginID: Plugin.ID.make(error.pluginID), + operation: error.operation, + message: `${error.message} Check server logs for details.`, + }), + ), + ), + ) + }), + ), + ), ) diff --git a/packages/server/test/skill-failures.test.ts b/packages/server/test/skill-failures.test.ts new file mode 100644 index 000000000000..bfcbbafee0f8 --- /dev/null +++ b/packages/server/test/skill-failures.test.ts @@ -0,0 +1,129 @@ +import { expect } from "bun:test" +import path from "node:path" +import { SdkPlugins } from "@opencode-ai/core/plugin/sdk" +import { Skill } from "@opencode-ai/core/skill" +import { Plugin } from "@opencode-ai/plugin/effect" +import { Context, Effect, Layer, Logger } from "effect" +import { HttpEffect, HttpRouter, HttpServer } from "effect/unstable/http" +import { tmpdirScoped } from "../../core/test/fixture/tmpdir" +import { it } from "../../core/test/lib/effect" +import { createRoutes } from "../src/routes" + +it.live("skill.list reports the failing plugin without exposing its exception", () => + Effect.gen(function* () { + const tmp = yield* tmpdirScoped("opencode-skill-failures-") + const messages: unknown[] = [] + const logger = Logger.make((options) => messages.push(options.message)) + const context = yield* Layer.build( + createRoutes({ + password: "secret", + database: { path: ":memory:" }, + models: { fetch: false }, + fs: { filewatcher: false }, + config: { directory: path.join(tmp.path, "config"), project: false }, + }).pipe( + Layer.provide(HttpServer.layerServices), + Layer.provideMerge(Logger.layer([logger], { mergeWithExisting: false })), + ), + ) + const sdk = Context.get(context, SdkPlugins.Service) + const cause = new TypeError("synthetic-private-detail") + yield* sdk.register( + Plugin.define({ + id: "broken-skills", + effect: (ctx) => + ctx.skill + .transform(() => { + throw cause + }) + .pipe(Effect.asVoid), + }), + ) + const handler = Context.get(context, HttpRouter.HttpRouter) + .asHttpEffect() + .pipe(HttpEffect.toWebHandlerWith(context)) + const request = (method: string, route: string) => + Effect.promise(() => + handler( + new Request(`http://opencode.local${route}?location[directory]=${encodeURIComponent(tmp.path)}`, { + method, + headers: { authorization: `Basic ${btoa("opencode:secret")}` }, + }), + ), + ) + expect((yield* request("POST", "/api/plugin/await-activation")).status).toBe(204) + // The directory is valid. A skill failure is not a location-not-found error. + expect((yield* request("GET", "/api/location")).status).toBe(200) + for (const attempt of [1, 2]) { + const response = yield* request("GET", "/api/skill") + expect(response.status).toBe(500) + const body = yield* Effect.promise(() => response.text()) + expect(body).toContain('"PluginCallbackError"') + expect(JSON.parse(body)).toEqual({ + _tag: "PluginCallbackError", + pluginID: "broken-skills", + operation: "skill.transform", + message: 'Plugin "broken-skills" failed during skill.transform. Check server logs for details.', + }) + expect(body).not.toContain("synthetic-private-detail") + expect(body).not.toContain("TypeError") + expect(body).not.toContain(tmp.path) + expect( + messages.filter((message) => Array.isArray(message) && message[0] === "Plugin callback failed"), + ).toHaveLength(attempt) + } + expect(messages).toContainEqual([ + "Plugin callback failed", + expect.objectContaining({ pluginID: "broken-skills", operation: "skill.transform", cause }), + ]) + }).pipe(Effect.timeout("10 seconds")), +) + +for (const scenario of [ + { name: "unrelated defects", effect: Effect.die(new Error("unrelated-private-detail")), status: 500 }, + // Effect's HTTP boundary maps server interruption to 503 (a client abort is 499). + { name: "interruption", effect: Effect.interrupt, status: 503 }, +]) { + it.live(`skill.list does not label ${scenario.name} as plugin callback failures`, () => + Effect.gen(function* () { + const tmp = yield* tmpdirScoped("opencode-skill-control-") + const context = yield* Layer.build( + createRoutes( + { + password: "secret", + database: { path: ":memory:" }, + models: { fetch: false }, + fs: { filewatcher: false }, + config: { directory: tmp.path, project: false }, + }, + () => [], + [ + Skill.node.replace( + Layer.succeed( + Skill.Service, + Skill.Service.of({ + list: () => scenario.effect, + get: () => Effect.undefined, + reload: () => Effect.void, + transform: () => Effect.succeed({ dispose: Effect.void }), + }), + ), + ), + ], + ).pipe(Layer.provide(HttpServer.layerServices)), + ) + const handler = Context.get(context, HttpRouter.HttpRouter) + .asHttpEffect() + .pipe(HttpEffect.toWebHandlerWith(context)) + const response = yield* Effect.promise(() => + handler( + new Request(`http://opencode.local/api/skill?location[directory]=${encodeURIComponent(tmp.path)}`, { + headers: { authorization: `Basic ${btoa("opencode:secret")}` }, + }), + ), + ) + expect(response.status).toBe(scenario.status) + expect(yield* Effect.promise(() => response.text())).toBe("") + }).pipe(Effect.timeout("10 seconds")), + ) +} diff --git a/packages/www/openapi.json b/packages/www/openapi.json index 97b3282be2d7..a2c0da1be39a 100644 --- a/packages/www/openapi.json +++ b/packages/www/openapi.json @@ -9259,6 +9259,16 @@ } } } + }, + "500": { + "description": "PluginCallbackError", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PluginCallbackErrorEncoded" + } + } + } } }, "description": "Retrieve currently registered skills.", @@ -16984,6 +16994,27 @@ } ] }, + "PluginCallbackErrorEncoded": { + "type": "object", + "properties": { + "_tag": { + "type": "string", + "enum": ["PluginCallbackError"] + }, + "pluginID": { + "type": "string" + }, + "operation": { + "type": "string", + "enum": ["skill.transform"] + }, + "message": { + "type": "string" + } + }, + "required": ["_tag", "pluginID", "operation", "message"], + "additionalProperties": false + }, "Project": { "type": "object", "properties": { diff --git a/packages/www/public/openapi.json b/packages/www/public/openapi.json index 97b3282be2d7..a2c0da1be39a 100644 --- a/packages/www/public/openapi.json +++ b/packages/www/public/openapi.json @@ -9259,6 +9259,16 @@ } } } + }, + "500": { + "description": "PluginCallbackError", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PluginCallbackErrorEncoded" + } + } + } } }, "description": "Retrieve currently registered skills.", @@ -16984,6 +16994,27 @@ } ] }, + "PluginCallbackErrorEncoded": { + "type": "object", + "properties": { + "_tag": { + "type": "string", + "enum": ["PluginCallbackError"] + }, + "pluginID": { + "type": "string" + }, + "operation": { + "type": "string", + "enum": ["skill.transform"] + }, + "message": { + "type": "string" + } + }, + "required": ["_tag", "pluginID", "operation", "message"], + "additionalProperties": false + }, "Project": { "type": "object", "properties": {