Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 15 additions & 13 deletions packages/opencode/src/goal/loop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)}`
Expand Down
10 changes: 6 additions & 4 deletions packages/opencode/src/hook/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 —
Expand All @@ -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) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
94 changes: 94 additions & 0 deletions packages/opencode/test/hook/http-handler.test.ts
Original file line number Diff line number Diff line change
@@ -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 = <A, E, R>(
fetch: (req: Request) => Response | Promise<Response>,
fn: (url: string) => Effect.Effect<A, E, R>,
) =>
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")
}),
)
}),
)
})
18 changes: 18 additions & 0 deletions packages/opencode/test/server/httpapi-exercise/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" }))
Expand Down
74 changes: 74 additions & 0 deletions packages/opencode/test/server/session-hooks-api.test.ts
Original file line number Diff line number Diff line change
@@ -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<string, unknown>) {
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 },
)
})
Loading