From 0694f60bdb2d85c0131c667cfff28446322d3aac Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Wed, 3 Jun 2026 13:36:22 +0800 Subject: [PATCH] fix(server): surface the declared 404 when deleting a missing message part The DELETE /session/:sessionID/message/:messageID/part/:partID route (part.delete) declares a 404, but Session.removePart was a silent no-op: it fired MessageV2.Event.PartRemoved and returned the id without checking existence, so deleting a part that does not exist returned 200. The declared 404 was a phantom. Add the existence check in the ROUTE handler (via Session.getPart, which returns the part or undefined) and throw NotFoundError when the part is missing (ErrorMiddleware maps it to 404). Unlike deleteMessage, the check cannot live in the service: Session.removePart is also called internally by the message processor (src/session/processor.ts), which must stay a tolerant no-op for an already-gone part. Keeping the check route-local makes the public contract real without changing the processor's behavior. No frontend impact: the desktop app does not call this endpoint directly (it reacts to the message.part.removed SSE event). The only behavior change is that deleting a missing/already-removed part now returns the already-declared 404. Refs #936 --- packages/opencode/src/server/instance/session.ts | 11 +++++++++++ .../opencode/test/server/session-messages.test.ts | 8 ++++++++ 2 files changed, 19 insertions(+) diff --git a/packages/opencode/src/server/instance/session.ts b/packages/opencode/src/server/instance/session.ts index 0b54dd240..af3898c6b 100644 --- a/packages/opencode/src/server/instance/session.ts +++ b/packages/opencode/src/server/instance/session.ts @@ -1331,6 +1331,17 @@ export const SessionRoutes = lazy(() => await AppRuntime.runPromise( Effect.gen(function* () { const sessions = yield* Session.Service + // Surface the route's declared 404 instead of silently succeeding: + // deleting a part that does not exist is a not-found, not a no-op. + // The check lives in the route, not Session.removePart, because + // removePart is also called internally by the message processor, + // which must stay a tolerant no-op for an already-gone part. + const part = yield* sessions.getPart({ + sessionID: params.sessionID, + messageID: params.messageID, + partID: params.partID, + }) + if (!part) throw new NotFoundError({ message: `Part not found: ${params.partID}` }) yield* sessions.removePart({ sessionID: params.sessionID, messageID: params.messageID, diff --git a/packages/opencode/test/server/session-messages.test.ts b/packages/opencode/test/server/session-messages.test.ts index f7fdfd2a8..c05ee6f4b 100644 --- a/packages/opencode/test/server/session-messages.test.ts +++ b/packages/opencode/test/server/session-messages.test.ts @@ -416,6 +416,14 @@ describe("session messages endpoint", () => { expect(await remove.json()).toBe(true) expect((await svc.messages({ sessionID: session.id }))[0].parts).toHaveLength(0) + // The route's declared 404 is now real: deleting an already-removed + // part surfaces NotFoundError instead of silently succeeding. + const missing = await app.request(`/session/${session.id}/message/${messageID}/part/${partID}`, { + method: "DELETE", + }) + expect(missing.status).toBe(404) + expect((await missing.json()).name).toBe("NotFoundError") + await svc.remove(session.id) }, }),