From 5b2b30060228eaff29c4b3d576dccaf02051deeb Mon Sep 17 00:00:00 2001 From: Shoubhit Dash Date: Wed, 13 May 2026 16:48:18 +0530 Subject: [PATCH] fix(session): tighten http error contracts (#27308) --- .../routes/instance/httpapi/groups/session.ts | 28 +++++------ .../instance/httpapi/handlers/session.ts | 39 ++++++++++----- .../test/server/httpapi-session.test.ts | 49 +++++++++++++++++++ packages/sdk/js/src/v2/gen/types.gen.ts | 30 +++++------- 4 files changed, 104 insertions(+), 42 deletions(-) diff --git a/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts b/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts index 2053aba3b4..b8c8a142be 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/groups/session.ts @@ -141,7 +141,7 @@ export const SessionApi = HttpApi.make("session") params: { sessionID: SessionID }, query: WorkspaceRoutingQuery, success: described(Schema.Array(Session.Info), "List of children"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.children", @@ -153,7 +153,7 @@ export const SessionApi = HttpApi.make("session") params: { sessionID: SessionID }, query: WorkspaceRoutingQuery, success: described(Schema.Array(Todo.Info), "Todo list"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.todo", @@ -250,7 +250,7 @@ export const SessionApi = HttpApi.make("session") params: { sessionID: SessionID }, query: WorkspaceRoutingQuery, success: described(Schema.Boolean, "Aborted session"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: HttpApiError.BadRequest, }).annotateMerge( OpenApi.annotations({ identifier: "session.abort", @@ -263,7 +263,7 @@ export const SessionApi = HttpApi.make("session") query: WorkspaceRoutingQuery, payload: InitPayload, success: described(Schema.Boolean, "200"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.init", @@ -314,7 +314,7 @@ export const SessionApi = HttpApi.make("session") query: WorkspaceRoutingQuery, payload: PromptPayload, success: described(MessageV2.WithParts, "Created message"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.prompt", @@ -327,7 +327,7 @@ export const SessionApi = HttpApi.make("session") query: WorkspaceRoutingQuery, payload: PromptPayload, success: described(HttpApiSchema.NoContent, "Prompt accepted"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.prompt_async", @@ -341,7 +341,7 @@ export const SessionApi = HttpApi.make("session") query: WorkspaceRoutingQuery, payload: CommandPayload, success: described(MessageV2.WithParts, "Created message"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.command", @@ -354,7 +354,7 @@ export const SessionApi = HttpApi.make("session") query: WorkspaceRoutingQuery, payload: ShellPayload, success: described(MessageV2.WithParts, "Created message"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.shell", @@ -367,7 +367,7 @@ export const SessionApi = HttpApi.make("session") query: WorkspaceRoutingQuery, payload: RevertPayload, success: described(Session.Info, "Updated session"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.revert", @@ -380,7 +380,7 @@ export const SessionApi = HttpApi.make("session") params: { sessionID: SessionID }, query: WorkspaceRoutingQuery, success: described(Session.Info, "Updated session"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.unrevert", @@ -393,7 +393,7 @@ export const SessionApi = HttpApi.make("session") query: WorkspaceRoutingQuery, payload: PermissionResponsePayload, success: described(Schema.Boolean, "Permission processed successfully"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "permission.respond", @@ -406,7 +406,7 @@ export const SessionApi = HttpApi.make("session") params: { sessionID: SessionID, messageID: MessageID }, query: WorkspaceRoutingQuery, success: described(Schema.Boolean, "Successfully deleted message"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "session.deleteMessage", @@ -419,7 +419,7 @@ export const SessionApi = HttpApi.make("session") params: { sessionID: SessionID, messageID: MessageID, partID: PartID }, query: WorkspaceRoutingQuery, success: described(Schema.Boolean, "Successfully deleted part"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "part.delete", @@ -431,7 +431,7 @@ export const SessionApi = HttpApi.make("session") query: WorkspaceRoutingQuery, payload: MessageV2.Part, success: described(MessageV2.Part, "Successfully updated part"), - error: [HttpApiError.BadRequest, HttpApiError.NotFound], + error: [HttpApiError.BadRequest, ApiNotFoundError], }).annotateMerge( OpenApi.annotations({ identifier: "part.update", 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 40444385f5..b12be2cfc2 100644 --- a/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts +++ b/packages/opencode/src/server/routes/instance/httpapi/handlers/session.ts @@ -68,15 +68,21 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", return Object.fromEntries(yield* statusSvc.list()) }) + const requireSession = Effect.fn("SessionHttpApi.requireSession")(function* (sessionID: SessionID) { + return yield* SessionError.mapStorageNotFound(session.get(sessionID)) + }) + const get = Effect.fn("SessionHttpApi.get")(function* (ctx: { params: { sessionID: SessionID } }) { - return yield* SessionError.mapStorageNotFound(session.get(ctx.params.sessionID)) + return yield* requireSession(ctx.params.sessionID) }) const children = Effect.fn("SessionHttpApi.children")(function* (ctx: { params: { sessionID: SessionID } }) { + yield* requireSession(ctx.params.sessionID) return yield* session.children(ctx.params.sessionID) }) const todo = Effect.fn("SessionHttpApi.todo")(function* (ctx: { params: { sessionID: SessionID } }) { + yield* requireSession(ctx.params.sessionID) return yield* todoSvc.get(ctx.params.sessionID) }) @@ -99,7 +105,7 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", catch: () => new HttpApiError.BadRequest({}), }) } - yield* SessionError.mapStorageNotFound(session.get(ctx.params.sessionID)) + yield* requireSession(ctx.params.sessionID) if (ctx.query.limit === undefined || ctx.query.limit === 0) { return yield* SessionError.mapStorageNotFound(session.messages({ sessionID: ctx.params.sessionID })) } @@ -165,7 +171,7 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", params: { sessionID: SessionID } payload: typeof UpdatePayload.Type }) { - const current = yield* SessionError.mapStorageNotFound(session.get(ctx.params.sessionID)) + const current = yield* requireSession(ctx.params.sessionID) if (ctx.payload.title !== undefined) { yield* session.setTitle({ sessionID: ctx.params.sessionID, title: ctx.payload.title }) } @@ -178,7 +184,7 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", if (ctx.payload.time?.archived !== undefined) { yield* session.setArchived({ sessionID: ctx.params.sessionID, time: ctx.payload.time.archived }) } - return yield* SessionError.mapStorageNotFound(session.get(ctx.params.sessionID)) + return yield* requireSession(ctx.params.sessionID) }) const fork = Effect.fn("SessionHttpApi.fork")(function* (ctx: { @@ -216,6 +222,7 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", params: { sessionID: SessionID } payload: typeof InitPayload.Type }) { + yield* requireSession(ctx.params.sessionID) yield* promptSvc .command({ sessionID: ctx.params.sessionID, @@ -234,22 +241,24 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", // ErrorMiddleware → NamedError.Unknown 500) instead of blanket-mapping // every failure to a 400 BadRequest. const share = Effect.fn("SessionHttpApi.share")(function* (ctx: { params: { sessionID: SessionID } }) { + yield* requireSession(ctx.params.sessionID) yield* shareSvc.share(ctx.params.sessionID).pipe(Effect.mapError(() => new HttpApiError.InternalServerError({}))) - return yield* SessionError.mapStorageNotFound(session.get(ctx.params.sessionID)) + return yield* requireSession(ctx.params.sessionID) }) const unshare = Effect.fn("SessionHttpApi.unshare")(function* (ctx: { params: { sessionID: SessionID } }) { + yield* requireSession(ctx.params.sessionID) yield* shareSvc .unshare(ctx.params.sessionID) .pipe(Effect.mapError(() => new HttpApiError.InternalServerError({}))) - return yield* SessionError.mapStorageNotFound(session.get(ctx.params.sessionID)) + return yield* requireSession(ctx.params.sessionID) }) const summarize = Effect.fn("SessionHttpApi.summarize")(function* (ctx: { params: { sessionID: SessionID } payload: typeof SummarizePayload.Type }) { - yield* revertSvc.cleanup(yield* SessionError.mapStorageNotFound(session.get(ctx.params.sessionID))) + yield* revertSvc.cleanup(yield* requireSession(ctx.params.sessionID)) const messages = yield* SessionError.mapStorageNotFound(session.messages({ sessionID: ctx.params.sessionID })) const defaultAgent = yield* agentSvc.defaultAgent() const currentAgent = messages.findLast((message) => message.info.role === "user")?.info.agent ?? defaultAgent @@ -271,6 +280,7 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", params: { sessionID: SessionID } payload: typeof PromptPayload.Type }) { + yield* requireSession(ctx.params.sessionID) const message = yield* promptSvc .prompt({ ...ctx.payload, @@ -286,6 +296,7 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", params: { sessionID: SessionID } payload: typeof PromptPayload.Type }) { + yield* requireSession(ctx.params.sessionID) yield* promptSvc.prompt({ ...ctx.payload, sessionID: ctx.params.sessionID }).pipe( Effect.catchCause((cause) => Effect.gen(function* () { @@ -307,6 +318,7 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", params: { sessionID: SessionID } payload: typeof CommandPayload.Type }) { + yield* requireSession(ctx.params.sessionID) return yield* promptSvc .command({ ...ctx.payload, sessionID: ctx.params.sessionID }) .pipe(Effect.mapError(() => new HttpApiError.BadRequest({}))) @@ -316,6 +328,7 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", params: { sessionID: SessionID } payload: typeof ShellPayload.Type }) { + yield* requireSession(ctx.params.sessionID) return yield* promptSvc.shell({ ...ctx.payload, sessionID: ctx.params.sessionID }) }) @@ -323,17 +336,20 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", params: { sessionID: SessionID } payload: typeof RevertPayload.Type }) { + yield* requireSession(ctx.params.sessionID) return yield* revertSvc.revert({ sessionID: ctx.params.sessionID, ...ctx.payload }) }) const unrevert = Effect.fn("SessionHttpApi.unrevert")(function* (ctx: { params: { sessionID: SessionID } }) { + yield* requireSession(ctx.params.sessionID) return yield* revertSvc.unrevert({ sessionID: ctx.params.sessionID }) }) const permissionRespond = Effect.fn("SessionHttpApi.permissionRespond")(function* (ctx: { - params: { permissionID: PermissionID } + params: { sessionID: SessionID; permissionID: PermissionID } payload: typeof PermissionResponsePayload.Type }) { + yield* requireSession(ctx.params.sessionID) yield* permissionSvc.reply({ requestID: ctx.params.permissionID, reply: ctx.payload.response }) return true }) @@ -341,6 +357,7 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", const deleteMessage = Effect.fn("SessionHttpApi.deleteMessage")(function* (ctx: { params: { sessionID: SessionID; messageID: MessageID } }) { + yield* requireSession(ctx.params.sessionID) yield* runState.assertNotBusy(ctx.params.sessionID) yield* session.removeMessage(ctx.params) return true @@ -349,6 +366,7 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", const deletePart = Effect.fn("SessionHttpApi.deletePart")(function* (ctx: { params: { sessionID: SessionID; messageID: MessageID; partID: PartID } }) { + yield* requireSession(ctx.params.sessionID) yield* session.removePart(ctx.params) return true }) @@ -357,15 +375,14 @@ export const sessionHandlers = HttpApiBuilder.group(InstanceHttpApi, "session", params: { sessionID: SessionID; messageID: MessageID; partID: PartID } payload: typeof MessageV2.Part.Type }) { + yield* requireSession(ctx.params.sessionID) const payload = ctx.payload as MessageV2.Part if ( payload.id !== ctx.params.partID || payload.messageID !== ctx.params.messageID || payload.sessionID !== ctx.params.sessionID ) { - throw new Error( - `Part mismatch: body.id='${payload.id}' vs partID='${ctx.params.partID}', body.messageID='${payload.messageID}' vs messageID='${ctx.params.messageID}', body.sessionID='${payload.sessionID}' vs sessionID='${ctx.params.sessionID}'`, - ) + return yield* new HttpApiError.BadRequest({}) } return yield* session.updatePart(payload) }) diff --git a/packages/opencode/test/server/httpapi-session.test.ts b/packages/opencode/test/server/httpapi-session.test.ts index 8cffb6e825..8b686739da 100644 --- a/packages/opencode/test/server/httpapi-session.test.ts +++ b/packages/opencode/test/server/httpapi-session.test.ts @@ -212,6 +212,14 @@ describe("session HttpApi", () => { expect(get.status).toBe(404) expect(yield* responseJson(get)).toEqual(missingSessionBody) + const children = yield* request(pathFor(SessionPaths.children, { sessionID: missingSession }), { headers }) + expect(children.status).toBe(404) + expect(yield* responseJson(children)).toEqual(missingSessionBody) + + const todo = yield* request(pathFor(SessionPaths.todo, { sessionID: missingSession }), { headers }) + expect(todo.status).toBe(404) + expect(yield* responseJson(todo)).toEqual(missingSessionBody) + const messages = yield* request(pathFor(SessionPaths.messages, { sessionID: missingSession }), { headers }) expect(messages.status).toBe(404) expect(yield* responseJson(messages)).toEqual(missingSessionBody) @@ -223,6 +231,21 @@ describe("session HttpApi", () => { expect(remove.status).toBe(404) expect(yield* responseJson(remove)).toEqual(missingSessionBody) + const prompt = yield* request(pathFor(SessionPaths.prompt, { sessionID: missingSession }), { + headers: { ...headers, "content-type": "application/json" }, + method: "POST", + body: JSON.stringify({ agent: "build", noReply: true, parts: [{ type: "text", text: "hello" }] }), + }) + expect(prompt.status).toBe(404) + expect(yield* responseJson(prompt)).toEqual(missingSessionBody) + + const abort = yield* request(pathFor(SessionPaths.abort, { sessionID: missingSession }), { + headers, + method: "POST", + }) + expect(abort.status).toBe(200) + expect(yield* responseJson(abort)).toBe(true) + const session = yield* createSession({ title: "missing message" }) const missingMessage = MessageID.ascending() const message = yield* request( @@ -530,6 +553,32 @@ describe("session HttpApi", () => { { git: true, config: { formatter: false, lsp: false } }, ) + it.instance( + "rejects part updates whose path and body ids disagree", + () => + Effect.gen(function* () { + const test = yield* TestInstance + const headers = { "x-opencode-directory": test.directory, "content-type": "application/json" } + const session = yield* createSession({ title: "part mismatch" }) + const message = yield* createTextMessage(session.id, "first") + const response = yield* request( + pathFor(SessionPaths.updatePart, { + sessionID: session.id, + messageID: message.info.id, + partID: message.part.id, + }), + { + method: "PATCH", + headers, + body: JSON.stringify({ ...message.part, id: PartID.ascending() }), + }, + ) + + expect(response.status).toBe(400) + }), + { git: true, config: { formatter: false, lsp: false } }, + ) + it.instance( "serves remaining non-LLM session mutation routes", () => diff --git a/packages/sdk/js/src/v2/gen/types.gen.ts b/packages/sdk/js/src/v2/gen/types.gen.ts index a1591aa2c3..35fb6bf812 100644 --- a/packages/sdk/js/src/v2/gen/types.gen.ts +++ b/packages/sdk/js/src/v2/gen/types.gen.ts @@ -5442,7 +5442,7 @@ export type SessionChildrenErrors = { */ 400: BadRequestError /** - * Not found + * NotFoundError */ 404: NotFoundError } @@ -5476,7 +5476,7 @@ export type SessionTodoErrors = { */ 400: BadRequestError /** - * Not found + * NotFoundError */ 404: NotFoundError } @@ -5586,7 +5586,7 @@ export type SessionPromptErrors = { */ 400: BadRequestError /** - * Not found + * NotFoundError */ 404: NotFoundError } @@ -5624,7 +5624,7 @@ export type SessionDeleteMessageErrors = { */ 400: BadRequestError /** - * Not found + * NotFoundError */ 404: NotFoundError } @@ -5731,10 +5731,6 @@ export type SessionAbortErrors = { * Bad request */ 400: BadRequestError - /** - * Not found - */ - 404: NotFoundError } export type SessionAbortError = SessionAbortErrors[keyof SessionAbortErrors] @@ -5770,7 +5766,7 @@ export type SessionInitErrors = { */ 400: BadRequestError /** - * Not found + * NotFoundError */ 404: NotFoundError } @@ -5925,7 +5921,7 @@ export type SessionPromptAsyncErrors = { */ 400: BadRequestError /** - * Not found + * NotFoundError */ 404: NotFoundError } @@ -5974,7 +5970,7 @@ export type SessionCommandErrors = { */ 400: BadRequestError /** - * Not found + * NotFoundError */ 404: NotFoundError } @@ -6019,7 +6015,7 @@ export type SessionShellErrors = { */ 400: BadRequestError /** - * Not found + * NotFoundError */ 404: NotFoundError } @@ -6059,7 +6055,7 @@ export type SessionRevertErrors = { */ 400: BadRequestError /** - * Not found + * NotFoundError */ 404: NotFoundError } @@ -6093,7 +6089,7 @@ export type SessionUnrevertErrors = { */ 400: BadRequestError /** - * Not found + * NotFoundError */ 404: NotFoundError } @@ -6130,7 +6126,7 @@ export type PermissionRespondErrors = { */ 400: BadRequestError /** - * Not found + * NotFoundError */ 404: NotFoundError } @@ -6166,7 +6162,7 @@ export type PartDeleteErrors = { */ 400: BadRequestError /** - * Not found + * NotFoundError */ 404: NotFoundError } @@ -6202,7 +6198,7 @@ export type PartUpdateErrors = { */ 400: BadRequestError /** - * Not found + * NotFoundError */ 404: NotFoundError }