Skip to content

Commit 868d549

Browse files
authored
fix(httpapi): pagination Link header echoes request host (anomalyco#25527)
1 parent ecf34e7 commit 868d549

2 files changed

Lines changed: 132 additions & 2 deletions

File tree

packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { Todo } from "@/session/todo"
1818
import { MessageID, PartID, SessionID } from "@/session/schema"
1919
import { NotFoundError } from "@/storage/storage"
2020
import { NamedError } from "@opencode-ai/core/util/error"
21-
import { Cause, Effect, Schema, Scope } from "effect"
21+
import { Cause, Effect, Option, Schema, Scope } from "effect"
2222
import * as Stream from "effect/Stream"
2323
import { HttpServerRequest, HttpServerResponse } from "effect/unstable/http"
2424
import { HttpApiBuilder, HttpApiError, HttpApiSchema } from "effect/unstable/httpapi"
@@ -125,7 +125,9 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session",
125125
if (!page.cursor) return page.items
126126

127127
const request = yield* HttpServerRequest.HttpServerRequest
128-
const url = new URL(request.url, "http://localhost")
128+
// toURL() honors the Host + x-forwarded-proto headers, so the Link
129+
// header echoes the real origin instead of a hard-coded localhost.
130+
const url = Option.getOrElse(HttpServerRequest.toURL(request), () => new URL(request.url, "http://localhost"))
129131
url.searchParams.set("limit", ctx.query.limit.toString())
130132
url.searchParams.set("before", page.cursor)
131133
return HttpServerResponse.jsonUnsafe(page.items, {
Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
import { afterEach, describe, expect, test } from "bun:test"
2+
import { Effect } from "effect"
3+
import { Flag } from "@opencode-ai/core/flag/flag"
4+
import * as Log from "@opencode-ai/core/util/log"
5+
import { WithInstance } from "../../src/project/with-instance"
6+
import { Server } from "../../src/server/server"
7+
import { Session } from "@/session/session"
8+
import { MessageID } from "../../src/session/schema"
9+
import { ModelID, ProviderID } from "../../src/provider/schema"
10+
import { resetDatabase } from "../fixture/db"
11+
import { disposeAllInstances, tmpdir } from "../fixture/fixture"
12+
13+
void Log.init({ print: false })
14+
15+
const original = Flag.OPENCODE_EXPERIMENTAL_HTTPAPI
16+
17+
afterEach(async () => {
18+
Flag.OPENCODE_EXPERIMENTAL_HTTPAPI = original
19+
await disposeAllInstances()
20+
await resetDatabase()
21+
})
22+
23+
function app(experimental: boolean) {
24+
Flag.OPENCODE_EXPERIMENTAL_HTTPAPI = experimental
25+
return experimental ? Server.Default().app : Server.Legacy().app
26+
}
27+
28+
function runSession<A, E>(fx: Effect.Effect<A, E, Session.Service>) {
29+
return Effect.runPromise(fx.pipe(Effect.provide(Session.defaultLayer)))
30+
}
31+
32+
function createSessionWithMessages(directory: string, count: number) {
33+
return WithInstance.provide({
34+
directory,
35+
fn: async () => {
36+
const session = await runSession(Session.Service.use((svc) => svc.create({})))
37+
for (let i = 0; i < count; i++) {
38+
await runSession(
39+
Effect.gen(function* () {
40+
const svc = yield* Session.Service
41+
yield* svc.updateMessage({
42+
id: MessageID.ascending(),
43+
role: "user",
44+
sessionID: session.id,
45+
agent: "build",
46+
model: { providerID: ProviderID.make("test"), modelID: ModelID.make("test") },
47+
time: { created: Date.now() },
48+
})
49+
}),
50+
)
51+
}
52+
return session.id
53+
},
54+
})
55+
}
56+
57+
// ──────────────────────────────────────────────────────────────────────────────
58+
// Reproducer 1: Link header should reflect the request's actual Host header,
59+
// not "localhost". HttpApi uses `new URL(request.url, "http://localhost")`
60+
// which embeds localhost because request.url is path-only. Fix: use
61+
// `HttpServerRequest.toURL(request)` which honors the Host header.
62+
// ──────────────────────────────────────────────────────────────────────────────
63+
describe("Link header host", () => {
64+
test("HttpApi pagination Link header echoes request host", async () => {
65+
await using tmp = await tmpdir({ config: { formatter: false, lsp: false } })
66+
const sessionID = await createSessionWithMessages(tmp.path, 3)
67+
68+
const response = await app(true).request(`/session/${sessionID}/message?limit=2`, {
69+
headers: {
70+
host: "opencode.test:4096",
71+
"x-opencode-directory": tmp.path,
72+
},
73+
})
74+
75+
expect(response.status).toBe(200)
76+
const link = response.headers.get("link")
77+
expect(link).not.toBeNull()
78+
// Link should contain the request's Host, not "localhost".
79+
expect(link).toContain("opencode.test")
80+
expect(link).not.toContain("localhost")
81+
})
82+
})
83+
84+
// ──────────────────────────────────────────────────────────────────────────────
85+
// Reproducer 2: GET /session/{missing-id}/todo should return 404, not 500.
86+
// The session.todo handler in HttpApi doesn't wrap with `mapNotFound`, so a
87+
// `NotFoundError` from the service surfaces as a defect → 500. Hono's
88+
// equivalent maps to 404 via `errors.notFound`.
89+
//
90+
// Affected endpoints (handlers without mapNotFound): todo, diff, summarize,
91+
// fork, abort, init, deleteMessage, command, shell, revert, unrevert.
92+
//
93+
// FIXME: unskip when mapNotFound coverage is added (next PR).
94+
// ──────────────────────────────────────────────────────────────────────────────
95+
describe("404 mapping for missing session", () => {
96+
test.todo("HttpApi /session/{missing}/todo returns 404 not 500", async () => {
97+
await using tmp = await tmpdir({ config: { formatter: false, lsp: false } })
98+
99+
const response = await app(true).request("/session/ses_does_not_exist/todo", {
100+
headers: { "x-opencode-directory": tmp.path },
101+
})
102+
103+
expect(response.status).toBe(404)
104+
})
105+
})
106+
107+
// ──────────────────────────────────────────────────────────────────────────────
108+
// Reproducer 3: 404 response body shape should match Hono's NamedError
109+
// envelope `{ name, data: { message } }`. HttpApi returns the typed-error
110+
// shape `{ _tag }` instead. SDK consumers reading `error.data.message`
111+
// see undefined.
112+
//
113+
// FIXME: unskip when error JSON shape policy is decided + applied (separate PR).
114+
// ──────────────────────────────────────────────────────────────────────────────
115+
describe("Error JSON shape parity", () => {
116+
test.todo("HttpApi 404 body matches NamedError shape", async () => {
117+
await using tmp = await tmpdir({ config: { formatter: false, lsp: false } })
118+
119+
const response = await app(true).request("/session/ses_does_not_exist", {
120+
headers: { "x-opencode-directory": tmp.path },
121+
})
122+
123+
expect(response.status).toBe(404)
124+
const body = (await response.json()) as { name?: string; data?: { message?: string } }
125+
expect(body.name).toBe("NotFoundError")
126+
expect(typeof body.data?.message).toBe("string")
127+
})
128+
})

0 commit comments

Comments
 (0)