fix: improve plugin system robustness — agent/command resolution, async errors, hook timing, two-phase init (#18280)
Co-authored-by: Aiden Cline <63023139+rekram1-node@users.noreply.github.com>pull/18992/head
parent
235a82aea9
commit
814a515a8a
|
|
@ -136,7 +136,11 @@ export namespace Plugin {
|
|||
|
||||
// Notify plugins of current config
|
||||
for (const hook of hooks) {
|
||||
await (hook as any).config?.(cfg)
|
||||
try {
|
||||
await (hook as any).config?.(cfg)
|
||||
} catch (err) {
|
||||
log.error("plugin config hook failed", { error: err })
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -19,6 +19,8 @@ import { PermissionID } from "@/permission/schema"
|
|||
import { ModelID, ProviderID } from "@/provider/schema"
|
||||
import { errors } from "../error"
|
||||
import { lazy } from "../../util/lazy"
|
||||
import { Bus } from "../../bus"
|
||||
import { NamedError } from "@opencode-ai/util/error"
|
||||
|
||||
const log = Log.create({ service: "server" })
|
||||
|
||||
|
|
@ -846,7 +848,13 @@ export const SessionRoutes = lazy(() =>
|
|||
return stream(c, async () => {
|
||||
const sessionID = c.req.valid("param").sessionID
|
||||
const body = c.req.valid("json")
|
||||
SessionPrompt.prompt({ ...body, sessionID })
|
||||
SessionPrompt.prompt({ ...body, sessionID }).catch((err) => {
|
||||
log.error("prompt_async failed", { sessionID, error: err })
|
||||
Bus.publish(Session.Event.Error, {
|
||||
sessionID,
|
||||
error: new NamedError.Unknown({ message: err instanceof Error ? err.message : String(err) }).toObject(),
|
||||
})
|
||||
})
|
||||
})
|
||||
},
|
||||
)
|
||||
|
|
|
|||
|
|
@ -418,6 +418,16 @@ export namespace SessionPrompt {
|
|||
)
|
||||
let executionError: Error | undefined
|
||||
const taskAgent = await Agent.get(task.agent)
|
||||
if (!taskAgent) {
|
||||
const available = await Agent.list().then((agents) => agents.filter((a) => !a.hidden).map((a) => a.name))
|
||||
const hint = available.length ? ` Available agents: ${available.join(", ")}` : ""
|
||||
const error = new NamedError.Unknown({ message: `Agent not found: "${task.agent}".${hint}` })
|
||||
Bus.publish(Session.Event.Error, {
|
||||
sessionID,
|
||||
error: error.toObject(),
|
||||
})
|
||||
throw error
|
||||
}
|
||||
const taskCtx: Tool.Context = {
|
||||
agent: task.agent,
|
||||
messageID: assistantMessage.id,
|
||||
|
|
@ -560,6 +570,16 @@ export namespace SessionPrompt {
|
|||
|
||||
// normal processing
|
||||
const agent = await Agent.get(lastUser.agent)
|
||||
if (!agent) {
|
||||
const available = await Agent.list().then((agents) => agents.filter((a) => !a.hidden).map((a) => a.name))
|
||||
const hint = available.length ? ` Available agents: ${available.join(", ")}` : ""
|
||||
const error = new NamedError.Unknown({ message: `Agent not found: "${lastUser.agent}".${hint}` })
|
||||
Bus.publish(Session.Event.Error, {
|
||||
sessionID,
|
||||
error: error.toObject(),
|
||||
})
|
||||
throw error
|
||||
}
|
||||
const maxSteps = agent.steps ?? Infinity
|
||||
const isLastStep = step >= maxSteps
|
||||
msgs = await insertReminders({
|
||||
|
|
@ -964,7 +984,18 @@ export namespace SessionPrompt {
|
|||
}
|
||||
|
||||
async function createUserMessage(input: PromptInput) {
|
||||
const agent = await Agent.get(input.agent ?? (await Agent.defaultAgent()))
|
||||
const agentName = input.agent || (await Agent.defaultAgent())
|
||||
const agent = await Agent.get(agentName)
|
||||
if (!agent) {
|
||||
const available = await Agent.list().then((agents) => agents.filter((a) => !a.hidden).map((a) => a.name))
|
||||
const hint = available.length ? ` Available agents: ${available.join(", ")}` : ""
|
||||
const error = new NamedError.Unknown({ message: `Agent not found: "${agentName}".${hint}` })
|
||||
Bus.publish(Session.Event.Error, {
|
||||
sessionID: input.sessionID,
|
||||
error: error.toObject(),
|
||||
})
|
||||
throw error
|
||||
}
|
||||
|
||||
const model = input.model ?? agent.model ?? (await lastModel(input.sessionID))
|
||||
const full =
|
||||
|
|
@ -1531,6 +1562,16 @@ NOTE: At any point in time through this workflow you should feel free to ask the
|
|||
await SessionRevert.cleanup(session)
|
||||
}
|
||||
const agent = await Agent.get(input.agent)
|
||||
if (!agent) {
|
||||
const available = await Agent.list().then((agents) => agents.filter((a) => !a.hidden).map((a) => a.name))
|
||||
const hint = available.length ? ` Available agents: ${available.join(", ")}` : ""
|
||||
const error = new NamedError.Unknown({ message: `Agent not found: "${input.agent}".${hint}` })
|
||||
Bus.publish(Session.Event.Error, {
|
||||
sessionID: input.sessionID,
|
||||
error: error.toObject(),
|
||||
})
|
||||
throw error
|
||||
}
|
||||
const model = input.model ?? agent.model ?? (await lastModel(input.sessionID))
|
||||
const userMsg: MessageV2.User = {
|
||||
id: MessageID.ascending(),
|
||||
|
|
@ -1783,7 +1824,14 @@ NOTE: At any point in time through this workflow you should feel free to ask the
|
|||
log.info("command", input)
|
||||
const command = await Command.get(input.command)
|
||||
if (!command) {
|
||||
throw new NamedError.Unknown({ message: `Command not found: "${input.command}"` })
|
||||
const available = await Command.list().then((cmds) => cmds.map((c) => c.name))
|
||||
const hint = available.length ? ` Available commands: ${available.join(", ")}` : ""
|
||||
const error = new NamedError.Unknown({ message: `Command not found: "${input.command}".${hint}` })
|
||||
Bus.publish(Session.Event.Error, {
|
||||
sessionID: input.sessionID,
|
||||
error: error.toObject(),
|
||||
})
|
||||
throw error
|
||||
}
|
||||
const agentName = command.agent ?? input.agent ?? (await Agent.defaultAgent())
|
||||
|
||||
|
|
|
|||
|
|
@ -54,3 +54,19 @@ describe("plugin.auth-override", () => {
|
|||
expect(plainMethods[ProviderID.make("github-copilot")][0].label).not.toBe("Test Override Auth")
|
||||
}, 30000) // Increased timeout for plugin installation
|
||||
})
|
||||
|
||||
const file = path.join(import.meta.dir, "../../src/plugin/index.ts")
|
||||
|
||||
describe("plugin.config-hook-error-isolation", () => {
|
||||
test("config hooks are individually error-isolated in the layer factory", async () => {
|
||||
const src = await Bun.file(file).text()
|
||||
|
||||
// The config hook try/catch lives in the InstanceState factory (layer definition),
|
||||
// not in init() which now just delegates to the Effect service.
|
||||
expect(src).toContain("plugin config hook failed")
|
||||
|
||||
const pattern =
|
||||
/for\s*\(const hook of hooks\)\s*\{[\s\S]*?try\s*\{[\s\S]*?\.config\?\.\([\s\S]*?\}\s*catch\s*\(err\)\s*\{[\s\S]*?plugin config hook failed[\s\S]*?\}/
|
||||
expect(pattern.test(src)).toBe(true)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -117,3 +117,16 @@ describe("session messages endpoint", () => {
|
|||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe("session.prompt_async error handling", () => {
|
||||
test("prompt_async route has error handler for detached prompt call", async () => {
|
||||
const src = await Bun.file(path.join(import.meta.dir, "../../src/server/routes/session.ts")).text()
|
||||
const start = src.indexOf('"/:sessionID/prompt_async"')
|
||||
const end = src.indexOf('"/:sessionID/command"', start)
|
||||
expect(start).toBeGreaterThan(-1)
|
||||
expect(end).toBeGreaterThan(start)
|
||||
const route = src.slice(start, end)
|
||||
expect(route).toContain(".catch(")
|
||||
expect(route).toContain("Bus.publish(Session.Event.Error")
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import path from "path"
|
||||
import { describe, expect, test } from "bun:test"
|
||||
import { NamedError } from "@opencode-ai/util/error"
|
||||
import { fileURLToPath } from "url"
|
||||
import { Instance } from "../../src/project/instance"
|
||||
import { ModelID, ProviderID } from "../../src/provider/schema"
|
||||
|
|
@ -210,3 +211,78 @@ describe("session.prompt agent variant", () => {
|
|||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe("session.agent-resolution", () => {
|
||||
test("unknown agent throws typed error", async () => {
|
||||
await using tmp = await tmpdir({ git: true })
|
||||
await Instance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
const session = await Session.create({})
|
||||
const err = await SessionPrompt.prompt({
|
||||
sessionID: session.id,
|
||||
agent: "nonexistent-agent-xyz",
|
||||
noReply: true,
|
||||
parts: [{ type: "text", text: "hello" }],
|
||||
}).then(
|
||||
() => undefined,
|
||||
(e) => e,
|
||||
)
|
||||
expect(err).toBeDefined()
|
||||
expect(err).not.toBeInstanceOf(TypeError)
|
||||
expect(NamedError.Unknown.isInstance(err)).toBe(true)
|
||||
if (NamedError.Unknown.isInstance(err)) {
|
||||
expect(err.data.message).toContain('Agent not found: "nonexistent-agent-xyz"')
|
||||
}
|
||||
},
|
||||
})
|
||||
}, 30000)
|
||||
|
||||
test("unknown agent error includes available agent names", async () => {
|
||||
await using tmp = await tmpdir({ git: true })
|
||||
await Instance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
const session = await Session.create({})
|
||||
const err = await SessionPrompt.prompt({
|
||||
sessionID: session.id,
|
||||
agent: "nonexistent-agent-xyz",
|
||||
noReply: true,
|
||||
parts: [{ type: "text", text: "hello" }],
|
||||
}).then(
|
||||
() => undefined,
|
||||
(e) => e,
|
||||
)
|
||||
expect(NamedError.Unknown.isInstance(err)).toBe(true)
|
||||
if (NamedError.Unknown.isInstance(err)) {
|
||||
expect(err.data.message).toContain("build")
|
||||
}
|
||||
},
|
||||
})
|
||||
}, 30000)
|
||||
|
||||
test("unknown command throws typed error with available names", async () => {
|
||||
await using tmp = await tmpdir({ git: true })
|
||||
await Instance.provide({
|
||||
directory: tmp.path,
|
||||
fn: async () => {
|
||||
const session = await Session.create({})
|
||||
const err = await SessionPrompt.command({
|
||||
sessionID: session.id,
|
||||
command: "nonexistent-command-xyz",
|
||||
arguments: "",
|
||||
}).then(
|
||||
() => undefined,
|
||||
(e) => e,
|
||||
)
|
||||
expect(err).toBeDefined()
|
||||
expect(err).not.toBeInstanceOf(TypeError)
|
||||
expect(NamedError.Unknown.isInstance(err)).toBe(true)
|
||||
if (NamedError.Unknown.isInstance(err)) {
|
||||
expect(err.data.message).toContain('Command not found: "nonexistent-command-xyz"')
|
||||
expect(err.data.message).toContain("init")
|
||||
}
|
||||
},
|
||||
})
|
||||
}, 30000)
|
||||
})
|
||||
|
|
|
|||
Loading…
Reference in New Issue