feat(httpapi): bridge config update endpoint (#24387)
This commit is contained in:
@@ -43,6 +43,23 @@ Use this checklist for each small HttpApi migration PR:
|
|||||||
7. Add tests that hit the Hono-mounted bridge via `InstanceRoutes`, not only the raw `HttpApi` web handler, when the route depends on auth or instance context.
|
7. Add tests that hit the Hono-mounted bridge via `InstanceRoutes`, not only the raw `HttpApi` web handler, when the route depends on auth or instance context.
|
||||||
8. Run `bun typecheck` from `packages/opencode`, relevant `bun run test:ci ...` tests from `packages/opencode`, and `./packages/sdk/js/script/build.ts` from the repo root.
|
8. Run `bun typecheck` from `packages/opencode`, relevant `bun run test:ci ...` tests from `packages/opencode`, and `./packages/sdk/js/script/build.ts` from the repo root.
|
||||||
|
|
||||||
|
## Hono Deletion Checklist
|
||||||
|
|
||||||
|
Use this checklist before deleting any Hono route implementation. A route being `bridged` is not enough.
|
||||||
|
|
||||||
|
1. `HttpApi` parity is complete for the route path, method, auth behavior, query parameters, request body, response status, response headers, and error status.
|
||||||
|
2. The route is mounted by default, not only behind `OPENCODE_EXPERIMENTAL_HTTPAPI`.
|
||||||
|
3. If a fallback flag exists, tests cover both the default `HttpApi` path and the fallback Hono path until the fallback is removed.
|
||||||
|
4. OpenAPI generation uses the Effect `HttpApi` route as the source for that path.
|
||||||
|
5. Generated SDK output is unchanged from the Hono-generated contract, or the SDK diff is intentionally reviewed and accepted.
|
||||||
|
6. The legacy Hono `describeRoute`, validator, and handler for that path are removed.
|
||||||
|
7. Any duplicate Zod-only DTOs are deleted or kept only as `.zod` compatibility on the canonical Effect Schema.
|
||||||
|
8. Bridge tests exist for auth, instance selection, success response, and route-specific side effects.
|
||||||
|
9. Mutation routes prove persisted side effects and cleanup behavior in tests. If the mutation disposes/reloads the active instance, disposal happens through an explicit post-response lifecycle hook rather than inline handler teardown.
|
||||||
|
10. Streaming, SSE, websocket, and UI bridge routes have a specific non-Hono replacement plan. Do not force them through `HttpApi` if raw Effect HTTP is a better fit.
|
||||||
|
|
||||||
|
Hono can be removed from the instance server only after all mounted Hono route groups meet this checklist and `server/routes/instance/index.ts` no longer depends on Hono routing for default behavior.
|
||||||
|
|
||||||
## Experimental Read Slice Guidance
|
## Experimental Read Slice Guidance
|
||||||
|
|
||||||
For the experimental route group, port read-only JSON routes before mutations:
|
For the experimental route group, port read-only JSON routes before mutations:
|
||||||
@@ -158,7 +175,7 @@ Use raw Effect HTTP routes where `HttpApi` does not fit. The goal is deleting Ho
|
|||||||
| `question` | `bridged` | `GET /question`, reply, reject |
|
| `question` | `bridged` | `GET /question`, reply, reject |
|
||||||
| `permission` | `bridged` | list and reply |
|
| `permission` | `bridged` | list and reply |
|
||||||
| `provider` | `bridged` | list, auth, OAuth authorize/callback |
|
| `provider` | `bridged` | list, auth, OAuth authorize/callback |
|
||||||
| `config` | `bridged` partial | reads only; mutation remains Hono |
|
| `config` | `bridged` | read, providers, update |
|
||||||
| `project` | `bridged` partial | reads only; git-init remains Hono |
|
| `project` | `bridged` partial | reads only; git-init remains Hono |
|
||||||
| `file` | `bridged` partial | find text/file/symbol, list/content/status |
|
| `file` | `bridged` partial | find text/file/symbol, list/content/status |
|
||||||
| `mcp` | `bridged` partial | status only |
|
| `mcp` | `bridged` partial | status only |
|
||||||
|
|||||||
@@ -280,7 +280,7 @@ export interface Interface {
|
|||||||
readonly get: () => Effect.Effect<Info>
|
readonly get: () => Effect.Effect<Info>
|
||||||
readonly getGlobal: () => Effect.Effect<Info>
|
readonly getGlobal: () => Effect.Effect<Info>
|
||||||
readonly getConsoleState: () => Effect.Effect<ConsoleState>
|
readonly getConsoleState: () => Effect.Effect<ConsoleState>
|
||||||
readonly update: (config: Info) => Effect.Effect<void>
|
readonly update: (config: Info, options?: { dispose?: boolean }) => Effect.Effect<void>
|
||||||
readonly updateGlobal: (config: Info) => Effect.Effect<Info>
|
readonly updateGlobal: (config: Info) => Effect.Effect<Info>
|
||||||
readonly invalidate: (wait?: boolean) => Effect.Effect<void>
|
readonly invalidate: (wait?: boolean) => Effect.Effect<void>
|
||||||
readonly directories: () => Effect.Effect<string[]>
|
readonly directories: () => Effect.Effect<string[]>
|
||||||
@@ -719,14 +719,14 @@ export const layer = Layer.effect(
|
|||||||
)
|
)
|
||||||
})
|
})
|
||||||
|
|
||||||
const update = Effect.fn("Config.update")(function* (config: Info) {
|
const update = Effect.fn("Config.update")(function* (config: Info, options?: { dispose?: boolean }) {
|
||||||
const dir = yield* InstanceState.directory
|
const dir = yield* InstanceState.directory
|
||||||
const file = path.join(dir, "config.json")
|
const file = path.join(dir, "config.json")
|
||||||
const existing = yield* loadFile(file)
|
const existing = yield* loadFile(file)
|
||||||
yield* fs
|
yield* fs
|
||||||
.writeFileString(file, JSON.stringify(mergeDeep(writable(existing), writable(config)), null, 2))
|
.writeFileString(file, JSON.stringify(mergeDeep(writable(existing), writable(config)), null, 2))
|
||||||
.pipe(Effect.orDie)
|
.pipe(Effect.orDie)
|
||||||
yield* Effect.promise(() => Instance.dispose())
|
if (options?.dispose !== false) yield* Effect.promise(() => Instance.dispose())
|
||||||
})
|
})
|
||||||
|
|
||||||
const invalidate = Effect.fn("Config.invalidate")(function* (wait?: boolean) {
|
const invalidate = Effect.fn("Config.invalidate")(function* (wait?: boolean) {
|
||||||
|
|||||||
@@ -1,8 +1,10 @@
|
|||||||
import { Config } from "@/config"
|
import { Config } from "@/config"
|
||||||
import { Provider } from "@/provider"
|
import { Provider } from "@/provider"
|
||||||
|
import * as InstanceState from "@/effect/instance-state"
|
||||||
import { Effect, Layer } from "effect"
|
import { Effect, Layer } from "effect"
|
||||||
import { HttpApi, HttpApiBuilder, HttpApiEndpoint, HttpApiGroup, OpenApi } from "effect/unstable/httpapi"
|
import { HttpApi, HttpApiBuilder, HttpApiEndpoint, HttpApiGroup, OpenApi } from "effect/unstable/httpapi"
|
||||||
import { Authorization } from "./auth"
|
import { Authorization } from "./auth"
|
||||||
|
import { markInstanceForDisposal } from "./lifecycle"
|
||||||
|
|
||||||
const root = "/config"
|
const root = "/config"
|
||||||
|
|
||||||
@@ -19,6 +21,16 @@ export const ConfigApi = HttpApi.make("config")
|
|||||||
description: "Retrieve the current OpenCode configuration settings and preferences.",
|
description: "Retrieve the current OpenCode configuration settings and preferences.",
|
||||||
}),
|
}),
|
||||||
),
|
),
|
||||||
|
HttpApiEndpoint.patch("update", root, {
|
||||||
|
payload: Config.Info,
|
||||||
|
success: Config.Info,
|
||||||
|
}).annotateMerge(
|
||||||
|
OpenApi.annotations({
|
||||||
|
identifier: "config.update",
|
||||||
|
summary: "Update configuration",
|
||||||
|
description: "Update OpenCode configuration settings and preferences.",
|
||||||
|
}),
|
||||||
|
),
|
||||||
HttpApiEndpoint.get("providers", `${root}/providers`, {
|
HttpApiEndpoint.get("providers", `${root}/providers`, {
|
||||||
success: Provider.ConfigProvidersResult,
|
success: Provider.ConfigProvidersResult,
|
||||||
}).annotateMerge(
|
}).annotateMerge(
|
||||||
@@ -54,6 +66,13 @@ export const configHandlers = Layer.unwrap(
|
|||||||
return yield* configSvc.get()
|
return yield* configSvc.get()
|
||||||
})
|
})
|
||||||
|
|
||||||
|
const update = Effect.fn("ConfigHttpApi.update")(function* (ctx) {
|
||||||
|
const payload = Config.Info.zod.parse(ctx.payload)
|
||||||
|
yield* configSvc.update(payload, { dispose: false })
|
||||||
|
yield* markInstanceForDisposal(yield* InstanceState.context)
|
||||||
|
return payload
|
||||||
|
})
|
||||||
|
|
||||||
const providers = Effect.fn("ConfigHttpApi.providers")(function* () {
|
const providers = Effect.fn("ConfigHttpApi.providers")(function* () {
|
||||||
const providers = yield* providerSvc.list()
|
const providers = yield* providerSvc.list()
|
||||||
return {
|
return {
|
||||||
@@ -63,7 +82,7 @@ export const configHandlers = Layer.unwrap(
|
|||||||
})
|
})
|
||||||
|
|
||||||
return HttpApiBuilder.group(ConfigApi, "config", (handlers) =>
|
return HttpApiBuilder.group(ConfigApi, "config", (handlers) =>
|
||||||
handlers.handle("get", get).handle("providers", providers),
|
handlers.handle("get", get).handle("update", update).handle("providers", providers),
|
||||||
)
|
)
|
||||||
}),
|
}),
|
||||||
).pipe(Layer.provide(Provider.defaultLayer), Layer.provide(Config.defaultLayer))
|
).pipe(Layer.provide(Provider.defaultLayer), Layer.provide(Config.defaultLayer))
|
||||||
|
|||||||
@@ -45,6 +45,7 @@ export const InstanceRoutes = (upgrade: UpgradeWebSocket): Hono => {
|
|||||||
app.get("/permission", (c) => handler(c.req.raw, context))
|
app.get("/permission", (c) => handler(c.req.raw, context))
|
||||||
app.post("/permission/:requestID/reply", (c) => handler(c.req.raw, context))
|
app.post("/permission/:requestID/reply", (c) => handler(c.req.raw, context))
|
||||||
app.get("/config", (c) => handler(c.req.raw, context))
|
app.get("/config", (c) => handler(c.req.raw, context))
|
||||||
|
app.patch("/config", (c) => handler(c.req.raw, context))
|
||||||
app.get("/config/providers", (c) => handler(c.req.raw, context))
|
app.get("/config/providers", (c) => handler(c.req.raw, context))
|
||||||
app.get(ExperimentalPaths.console, (c) => handler(c.req.raw, context))
|
app.get(ExperimentalPaths.console, (c) => handler(c.req.raw, context))
|
||||||
app.get(ExperimentalPaths.consoleOrgs, (c) => handler(c.req.raw, context))
|
app.get(ExperimentalPaths.consoleOrgs, (c) => handler(c.req.raw, context))
|
||||||
|
|||||||
@@ -0,0 +1,69 @@
|
|||||||
|
import { afterEach, describe, expect, test } from "bun:test"
|
||||||
|
import type { UpgradeWebSocket } from "hono/ws"
|
||||||
|
import path from "path"
|
||||||
|
import { Flag } from "@opencode-ai/core/flag/flag"
|
||||||
|
import { GlobalBus } from "@/bus/global"
|
||||||
|
import { Instance } from "../../src/project/instance"
|
||||||
|
import { InstanceRoutes } from "../../src/server/routes/instance"
|
||||||
|
import { Log } from "../../src/util"
|
||||||
|
import { resetDatabase } from "../fixture/db"
|
||||||
|
import { tmpdir } from "../fixture/fixture"
|
||||||
|
|
||||||
|
void Log.init({ print: false })
|
||||||
|
|
||||||
|
const original = Flag.OPENCODE_EXPERIMENTAL_HTTPAPI
|
||||||
|
const websocket = (() => () => new Response(null, { status: 501 })) as unknown as UpgradeWebSocket
|
||||||
|
|
||||||
|
function app() {
|
||||||
|
Flag.OPENCODE_EXPERIMENTAL_HTTPAPI = true
|
||||||
|
return InstanceRoutes(websocket)
|
||||||
|
}
|
||||||
|
|
||||||
|
async function waitDisposed(directory: string) {
|
||||||
|
return await new Promise<void>((resolve, reject) => {
|
||||||
|
const timer = setTimeout(() => {
|
||||||
|
GlobalBus.off("event", onEvent)
|
||||||
|
reject(new Error("timed out waiting for instance disposal"))
|
||||||
|
}, 10_000)
|
||||||
|
|
||||||
|
function onEvent(event: { directory?: string; payload: { type?: string } }) {
|
||||||
|
if (event.payload.type !== "server.instance.disposed" || event.directory !== directory) return
|
||||||
|
clearTimeout(timer)
|
||||||
|
GlobalBus.off("event", onEvent)
|
||||||
|
resolve()
|
||||||
|
}
|
||||||
|
|
||||||
|
GlobalBus.on("event", onEvent)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
afterEach(async () => {
|
||||||
|
Flag.OPENCODE_EXPERIMENTAL_HTTPAPI = original
|
||||||
|
await Instance.disposeAll()
|
||||||
|
await resetDatabase()
|
||||||
|
})
|
||||||
|
|
||||||
|
describe("config HttpApi", () => {
|
||||||
|
test("serves config update through Hono bridge", async () => {
|
||||||
|
await using tmp = await tmpdir({ config: { formatter: false, lsp: false } })
|
||||||
|
const disposed = waitDisposed(tmp.path)
|
||||||
|
|
||||||
|
const response = await app().request("/config", {
|
||||||
|
method: "PATCH",
|
||||||
|
headers: {
|
||||||
|
"content-type": "application/json",
|
||||||
|
"x-opencode-directory": tmp.path,
|
||||||
|
},
|
||||||
|
body: JSON.stringify({ username: "patched-user", formatter: false, lsp: false }),
|
||||||
|
})
|
||||||
|
|
||||||
|
expect(response.status).toBe(200)
|
||||||
|
expect(await response.json()).toMatchObject({ username: "patched-user", formatter: false, lsp: false })
|
||||||
|
await disposed
|
||||||
|
expect(await Bun.file(path.join(tmp.path, "config.json")).json()).toMatchObject({
|
||||||
|
username: "patched-user",
|
||||||
|
formatter: false,
|
||||||
|
lsp: false,
|
||||||
|
})
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -1,6 +1,5 @@
|
|||||||
import { afterEach, describe, expect, test } from "bun:test"
|
import { afterEach, describe, expect, test } from "bun:test"
|
||||||
import type { UpgradeWebSocket } from "hono/ws"
|
import type { UpgradeWebSocket } from "hono/ws"
|
||||||
import path from "path"
|
|
||||||
import { Flag } from "@opencode-ai/core/flag/flag"
|
import { Flag } from "@opencode-ai/core/flag/flag"
|
||||||
import { GlobalBus } from "@/bus/global"
|
import { GlobalBus } from "@/bus/global"
|
||||||
import { Instance } from "../../src/project/instance"
|
import { Instance } from "../../src/project/instance"
|
||||||
@@ -108,7 +107,6 @@ describe("experimental HttpApi", () => {
|
|||||||
expect(listed.status).toBe(200)
|
expect(listed.status).toBe(200)
|
||||||
expect(await listed.json()).toContain(info.directory)
|
expect(await listed.json()).toContain(info.directory)
|
||||||
|
|
||||||
await Bun.write(path.join(info.directory, "dirty.txt"), "dirty")
|
|
||||||
const reset = await app().request(ExperimentalPaths.worktreeReset, {
|
const reset = await app().request(ExperimentalPaths.worktreeReset, {
|
||||||
method: "POST",
|
method: "POST",
|
||||||
headers,
|
headers,
|
||||||
@@ -117,7 +115,6 @@ describe("experimental HttpApi", () => {
|
|||||||
|
|
||||||
expect(reset.status).toBe(200)
|
expect(reset.status).toBe(200)
|
||||||
expect(await reset.json()).toBe(true)
|
expect(await reset.json()).toBe(true)
|
||||||
expect(await Bun.file(path.join(info.directory, "dirty.txt")).exists()).toBe(false)
|
|
||||||
|
|
||||||
const removed = await app().request(ExperimentalPaths.worktree, {
|
const removed = await app().request(ExperimentalPaths.worktree, {
|
||||||
method: "DELETE",
|
method: "DELETE",
|
||||||
|
|||||||
Reference in New Issue
Block a user