diff --git a/packages/opencode/src/goal/loop.ts b/packages/opencode/src/goal/loop.ts index fac694df93..8349856ff4 100644 --- a/packages/opencode/src/goal/loop.ts +++ b/packages/opencode/src/goal/loop.ts @@ -474,24 +474,26 @@ const serviceLayer = Layer.effect( Effect.catchCause((cause) => Effect.gen(function* () { // F1: Only pause for non-interrupt causes. An interrupt (user - // pressed ESC during continuation) is safe to drop because the - // session ALWAYS re-emits idle afterwards, which re-drives this - // loop: SessionRunState.cancel (run-state.ts) and the runner's - // onIdle callback both call status.set(idle), and - // SessionStatus.set (status.ts) publishes the Status+Idle event - // pair unconditionally — even when the session was already idle. - // That fresh idle event forks a new afterIdle fiber whose - // shouldPreempt guard detects the user's newer message and pauses - // there if needed. Pausing HERE would race that replacement - // afterIdle fiber and emit a spurious pause. Real dispatch - // failures (provider fault, session write error) still get the - // recoverable pause below. + // pressed ESC during continuation) is safe to drop because + // SessionPrompt.cancel pauses goal-driven turns SYNCHRONOUSLY via + // goal.pauseForUserCancel (prompt.ts) BEFORE state.cancel lets the + // interrupt propagate — by the time this catchCause observes the + // cause, the goal is already paused, and pausing again HERE would + // double-publish. The session still ALWAYS re-emits idle + // afterwards, which re-drives this loop: SessionRunState.cancel + // (run-state.ts) and the runner's onIdle callback both call + // status.set(idle), and SessionStatus.set (status.ts) publishes + // the Status+Idle event pair unconditionally — even when the + // session was already idle. On that next cycle shouldPreempt is + // only the DB-failure fallback for a pauseForUserCancel that could + // not persist. Real dispatch failures (provider fault, session + // write error) still get the recoverable pause below. // F1: hasInterrupts is a structural check; Cause.interruptors only // collects DEFINED fiber ids and silently ignores interrupts // carrying none (e.g. Cause.interrupt()), which would otherwise be // misclassified as a dispatch failure and spuriously paused here. if (Cause.hasInterrupts(cause)) { - yield* Effect.logInfo("goal continuation interrupted (likely user ESC) — not pausing; shouldPreempt handles next cycle") + yield* Effect.logInfo("goal continuation interrupted (likely user ESC) — not pausing; cancel path already paused the goal") return Option.none() } const errMsg = `continuation dispatch failed: ${Cause.pretty(cause)}` diff --git a/packages/opencode/src/hook/settings.ts b/packages/opencode/src/hook/settings.ts index 9f61f6aafc..44b67c0983 100644 --- a/packages/opencode/src/hook/settings.ts +++ b/packages/opencode/src/hook/settings.ts @@ -1432,10 +1432,11 @@ const mcpHandler: HookHandler = { /** * `type: "http"` handler. Per CC protocol, `entry.command` is the endpoint URL; - * the envelope is POSTed as JSON. 2xx → body parsed via the same parseStdout path - * as command stdout. Non-2xx → synthetic `exitBlock` so the trigger aggregator - * surfaces it as a block. Network errors / timeouts → log.warn + silent allow, - * mirroring commandHandler's spawnError behavior (hooks must never crash the host). + * the envelope is POSTed as JSON with `entry.headers` applied verbatim (auth + * tokens etc.). 2xx → body parsed via the same parseStdout path as command + * stdout. Non-2xx → synthetic `exitBlock` so the trigger aggregator surfaces it + * as a block. Network errors / timeouts → log.warn + silent allow, mirroring + * commandHandler's spawnError behavior (hooks must never crash the host). * * Factory takes the resolved HttpClient so the HookHandler.run signature stays * `R = never` (the WP-4A interface contract). Captures `http` in closure scope — @@ -1454,6 +1455,7 @@ const httpHandler: HookHandler = { const url = httpUrl(entry) const exit = yield* HttpClientRequest.post(url).pipe( + HttpClientRequest.setHeaders(entry.headers ?? {}), HttpClientRequest.bodyJson(envelope), Effect.flatMap((req) => httpRead.execute(req)), Effect.flatMap((res) => diff --git a/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts b/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts index c950b632b0..1112688901 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts @@ -440,6 +440,16 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", // Non-empty hooks[] guard → 4xx. event/type membership is already enforced // by the payload's literal schemas; this completes the validation contract. if (ctx.payload.hooks.length === 0) return yield* new HttpApiError.BadRequest({}) + // Per-entry guards: a command hook must carry a runnable command line + // (blank-only strings would spawn nothing), and timeout is a positive + // seconds multiplier — negative values time the request out immediately, + // and 0 silently falls back to the default instead of meaning "no timeout". + const invalidEntry = ctx.payload.hooks.some( + (hook) => + (hook.type === "command" && !(hook.command ?? "").trim()) || + (hook.timeout !== undefined && hook.timeout <= 0), + ) + if (invalidEntry) return yield* new HttpApiError.BadRequest({}) const id = yield* sessionHooks.add(ctx.params.sessionID, { event: ctx.payload.event as HookEvent, matcher: ctx.payload.matcher, diff --git a/packages/opencode/test/hook/http-handler.test.ts b/packages/opencode/test/hook/http-handler.test.ts new file mode 100644 index 0000000000..ade67bdcba --- /dev/null +++ b/packages/opencode/test/hook/http-handler.test.ts @@ -0,0 +1,94 @@ +import { describe, expect } from "bun:test" +import { Effect, Layer } from "effect" +import { FetchHttpClient } from "effect/unstable/http" +import { SettingsHook } from "@/hook/settings" +import { SessionHooks } from "@/hook/session-hooks" +import { EventV2Bridge } from "@/event-v2-bridge" +import { Database } from "@opencode-ai/core/database/database" +import { SessionID } from "@/session/schema" +import { testEffect } from "../lib/effect" + +// httpHandler runtime contract against a real local server: configured +// entry.headers MUST reach the wire (auth tokens were silently dropped before +// this fix), and non-2xx responses MUST surface as the synthetic exitBlock so +// the trigger aggregator reports a block. + +const testLayer = SettingsHook.layer.pipe( + Layer.provide(EventV2Bridge.defaultLayer), + Layer.provide(Database.defaultLayer), + Layer.provideMerge(SessionHooks.defaultLayer), + Layer.provideMerge(FetchHttpClient.layer), +) +const it = testEffect(testLayer) + +const withFetch = ( + fetch: (req: Request) => Response | Promise, + fn: (url: string) => Effect.Effect, +) => + Effect.acquireUseRelease( + Effect.sync(() => Bun.serve({ port: 0, fetch })), + (server) => fn(server.url.toString()), + (server) => Effect.sync(() => server.stop(true)), + ) + +describe("SettingsHook http handler", () => { + it.instance("applies configured entry.headers to the outbound POST", () => + Effect.gen(function* () { + const sessionHooks = yield* SessionHooks.Service + const hook = yield* SettingsHook.Service + const sessionID = SessionID.descending() + let seen: Headers | undefined + yield* withFetch( + (req) => { + seen = req.headers + return new Response("{}", { status: 200, headers: { "content-type": "application/json" } }) + }, + (url) => + Effect.gen(function* () { + yield* sessionHooks.add(sessionID, { + event: "UserPromptSubmit", + hooks: [ + { + type: "http", + url, + headers: { authorization: "Bearer hook-secret", "x-hook-test": "present" }, + }, + ], + }) + const r = yield* hook.trigger( + { event: "UserPromptSubmit", prompt: "hi" }, + { sessionID, transcriptPath: "" }, + ) + expect(r.blocked).toBeUndefined() + expect(seen).toBeDefined() + expect(seen?.get("authorization")).toBe("Bearer hook-secret") + expect(seen?.get("x-hook-test")).toBe("present") + }), + ) + }), + ) + + it.instance("non-2xx response surfaces as exitBlock", () => + Effect.gen(function* () { + const sessionHooks = yield* SessionHooks.Service + const hook = yield* SettingsHook.Service + const sessionID = SessionID.descending() + yield* withFetch( + () => new Response("nope", { status: 500 }), + (url) => + Effect.gen(function* () { + yield* sessionHooks.add(sessionID, { + event: "UserPromptSubmit", + hooks: [{ type: "http", url }], + }) + const r = yield* hook.trigger( + { event: "UserPromptSubmit", prompt: "hi" }, + { sessionID, transcriptPath: "" }, + ) + expect(r.blocked).toBeDefined() + expect(r.blocked?.reason).toContain("500") + }), + ) + }), + ) +}) diff --git a/packages/opencode/test/server/httpapi-exercise/index.ts b/packages/opencode/test/server/httpapi-exercise/index.ts index 439e985a58..ad9f3a2080 100644 --- a/packages/opencode/test/server/httpapi-exercise/index.ts +++ b/packages/opencode/test/server/httpapi-exercise/index.ts @@ -1237,6 +1237,24 @@ const scenarios: Scenario[] = [ body: { event: "NotAnEvent", hooks: [{ type: "command", command: "printf '%s' 'x'" }] }, })) .status(400), + http.protected + .post("/session/{sessionID}/hook", "session.hook.add.empty_command") + .seeded((ctx) => ctx.session({ title: "Hook empty command session" })) + .at((ctx) => ({ + path: route("/session/{sessionID}/hook", { sessionID: ctx.state.id }), + headers: ctx.headers(), + body: { event: "UserPromptSubmit", hooks: [{ type: "command", command: "" }] }, + })) + .status(400), + http.protected + .post("/session/{sessionID}/hook", "session.hook.add.non_positive_timeout") + .seeded((ctx) => ctx.session({ title: "Hook bad timeout session" })) + .at((ctx) => ({ + path: route("/session/{sessionID}/hook", { sessionID: ctx.state.id }), + headers: ctx.headers(), + body: { event: "UserPromptSubmit", hooks: [{ type: "command", command: "true", timeout: 0 }] }, + })) + .status(400), http.protected .get("/session/{sessionID}/hook", "session.hook.list") .seeded((ctx) => ctx.session({ title: "Hook list session" })) diff --git a/packages/opencode/test/server/session-hooks-api.test.ts b/packages/opencode/test/server/session-hooks-api.test.ts new file mode 100644 index 0000000000..9aaf012f14 --- /dev/null +++ b/packages/opencode/test/server/session-hooks-api.test.ts @@ -0,0 +1,74 @@ +import { afterEach, describe, expect } from "bun:test" +import { Effect, Layer } from "effect" +import { Session } from "@/session/session" +import { disposeAllInstances, TestInstance } from "../fixture/fixture" +import { testEffect } from "../lib/effect" +import { httpApiLayer, requestInDirectory } from "./httpapi-layer" + +const it = testEffect(Layer.mergeAll(Session.defaultLayer, httpApiLayer)) + +afterEach(() => disposeAllInstances()) + +function addHook(directory: string, sessionID: string, hook: Record) { + return requestInDirectory(`/session/${sessionID}/hook`, directory, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ event: "UserPromptSubmit", hooks: [hook] }), + }) +} + +describe("session hook add validation", () => { + it.instance( + "rejects command-type hooks with a missing or blank command", + () => + Effect.gen(function* () { + const test = yield* TestInstance + const session = yield* Session.use.create({}) + + const missing = yield* addHook(test.directory, session.id, { type: "command" }) + expect(missing.status).toBe(400) + + const blank = yield* addHook(test.directory, session.id, { type: "command", command: " " }) + expect(blank.status).toBe(400) + }), + { git: true }, + ) + + it.instance( + "rejects non-positive timeout", + () => + Effect.gen(function* () { + const test = yield* TestInstance + const session = yield* Session.use.create({}) + + const zero = yield* addHook(test.directory, session.id, { type: "command", command: "true", timeout: 0 }) + expect(zero.status).toBe(400) + + const negative = yield* addHook(test.directory, session.id, { type: "command", command: "true", timeout: -5 }) + expect(negative.status).toBe(400) + }), + { git: true }, + ) + + it.instance( + "accepts valid command and http hooks", + () => + Effect.gen(function* () { + const test = yield* TestInstance + const session = yield* Session.use.create({}) + + const command = yield* addHook(test.directory, session.id, { type: "command", command: "true" }) + expect(command.status).toBe(200) + expect(typeof ((yield* command.json) as { id: string }).id).toBe("string") + + const http = yield* addHook(test.directory, session.id, { + type: "http", + url: "https://hooks.example.com/endpoint", + timeout: 30, + headers: { authorization: "Bearer token" }, + }) + expect(http.status).toBe(200) + }), + { git: true }, + ) +})