feat(plugins): return a Response from a route handler to serve non-JSON content#2162
Conversation
Plugin route handlers could only return JSON — whatever they returned
was wrapped in the standard { success, data } envelope, so there was no
way to serve an image, a file, or any custom content type from a plugin
route (emdash-cms#2110).
A handler that returns a Response now has it passed through verbatim:
the route registry carries it on RouteResult.response with the
Response's own status, and the plugin API endpoint returns it directly
instead of JSON-wrapping it. Ordinary return values keep the envelope.
Passthrough is trusted-plugin only; sandboxed results cross a
serialization boundary and stay JSON.
Fixes emdash-cms#2110
🦋 Changeset detectedLatest commit: 8854d3e The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
github-actions
Bot
added
area/core
needs-rebase
size/M
review/awaiting-author
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
The approach is sound: this is a real, additive bug fix that lets trusted plugin route handlers serve non-JSON content by returning a Response, without changing the envelope for ordinary return values. The implementation is minimal and the new unit tests in routes.test.ts cover the handler-level behavior well.
I found a few gaps to tighten before merge:
- Comment discipline. Both new inline comments reference issue
#2110, which violates the AGENTS.md convention that comments should not point to issues/PRs/review threads. The JSDoc onRouteResultandPluginRoutealready explains the behavior for future readers, so these inline comments can be removed. - Missing API-route test. The unit tests verify
PluginRouteHandler.invoke()returns the rawResponseinresult.response, but they don't cover the actual HTTP path in[...path].tswhere thatResponseis returned to the client. Adding a focused test inplugin-api-route-auth.test.ts(mockinghandlePluginApiRouteto return{ success: true, response: ... }and asserting the route returns it verbatim) would close the loop on the reported bug. - Stale
HandlerResponsecontract.locals.emdash.handlePluginApiRouteandhandlePublicPluginApiRoutenow return a result that may containresponse: Response, but theHandlerResponsetype inastro/types.tsdoesn't expose it. Updating the interface keeps the locals contract honest for SSR/server callers.
Findings
-
[needs fixing]
packages/core/src/plugins/routes.ts:158-160The inline comments reference issue
#2110. AGENTS.md says comments should be evergreen for future readers and must not reference issues/PRs/review threads. The JSDoc onRouteResultalready documents the passthrough behavior, so this block can be removed entirely.const result = await route.handler(routeContext); if (result instanceof Response) { return { -
[needs fixing]
packages/core/src/astro/routes/api/plugins/[pluginId]/[...path].ts:86-88Same comment discipline issue: the inline comment references
#2110. The surrounding code (passthrough instanceof Response) is self-explanating, and the JSDoc onRouteResult/PluginRoutecovers the rationale.const passthrough = (result as { response?: unknown }).response; if (passthrough instanceof Response) { return passthrough; } -
[needs fixing]
packages/core/src/astro/routes/api/plugins/[pluginId]/[...path].ts:89This passthrough path is not exercised by any test.
routes.test.tsverifies the lower-levelPluginRouteHandler.invoke()shape, but the reported bug is about the HTTP endpoint returning the raw response. Add a case inpackages/core/tests/unit/astro/plugin-api-route-auth.test.ts(or a sibling file) that mockshandlePluginApiRouteto return{ success: true, response: new Response(...), status: 200 }and asserts the Astro route returns that exactResponseto the client.it("returns a handler Response verbatim instead of wrapping it", async () => { const { locals, handlePluginApiRoute } = createLocals(null, true); const png = new Response(new Uint8Array([0x89, 0x50, 0x4e, 0x47]), { headers: { "Content-Type": "image/png" }, }); handlePluginApiRoute.mockResolvedValue({ success: true, response: png, status: 200 }); const res = await invoke(GET, "GET", locals, { csrf: false }); expect(res).toBe(png); expect(res.headers.get("Content-Type")).toBe("image/png"); }); -
[suggestion]
packages/core/src/astro/types.ts:208handlePluginApiRouteandhandlePublicPluginApiRoutecan now return a result carryingresponse: Responsefor trusted plugin routes. TheHandlerResponseinterface should reflect that so callers oflocals.emdash.handlePluginApiRoute/handlePublicPluginApiRoutesee the full shape in TypeScript rather than discovering an untyped runtime field.export interface HandlerResponse<T = unknown> { success: boolean; data?: T; response?: Response; error?: { code: string; message: string; details?: Record<string, unknown>; }; }
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
…ponse-passthrough # Conflicts: # packages/core/src/astro/routes/api/plugins/[pluginId]/[...path].ts # packages/core/src/plugins/types.ts
github-actions
Bot
added
review/needs-rereview
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
There was a problem hiding this comment.
The approach remains sound: this is a minimal, additive fix that lets trusted plugin route handlers serve non-JSON content by returning a Response, without changing the envelope for ordinary return values. The JSDoc on RouteResult and PluginRoute documents the sandbox limitation well, and the lower-level unit tests in routes.test.ts correctly cover the handler-level behavior.
However, my prior review pointed out three gaps, and none of them have been addressed in the current head:
- Comment discipline. Inline comments in
packages/core/src/plugins/routes.tsandpackages/core/src/astro/routes/api/plugins/[pluginId]/[...path].tsstill reference issue#2110, which violates the AGENTS.md rule that comments must never point to issues/PRs/review threads. - Missing HTTP-path test.
routes.test.tsverifiesPluginRouteHandler.invoke()returns the rawResponse, but the actual Astro API route ([...path].ts) — where the passthrough is returned to the client — is not exercised by any test. A focused regression test there is still needed to close the loop on the reported bug. - Stale
HandlerResponsecontract.locals.emdash.handlePluginApiRouteandhandlePublicPluginApiRoutenow return aRouteResultcarryingresponse: Response(and the existingstatus), but theHandlerResponseinterface inastro/types.tsstill only exposessuccess,data, anderror. Updating it keeps the locals contract honest for callers and lets the route file drop its unsafe casts.
No new security, data-integrity, or logged-out query concerns were introduced by this change. Once the comments are cleaned up, the HTTP path is tested, and the type contract is updated, this should be good to merge.
Findings
-
[needs fixing]
packages/core/src/plugins/routes.ts:184-185The inline comment block references issue
#2110. AGENTS.md explicitly forbids issue/PR/review-thread references in comments — that context belongs in the commit message and PR description. TheRouteResult.responseJSDoc already documents the passthrough behavior for future readers, so this comment can be removed.const result = await route.handler(routeContext); if (result instanceof Response) { return { -
[needs fixing]
packages/core/src/astro/routes/api/plugins/[pluginId]/[...path].ts:92-93Same comment-discipline violation: the inline comment references
#2110. The surrounding code (passthrough instanceof Response) is self-explanatory, and the JSDoc onRouteResult/PluginRoutecovers the rationale and the sandbox limitation. Remove the issue reference.const passthrough = (result as { response?: unknown }).response; if (passthrough instanceof Response) { return passthrough; } -
[needs fixing]
packages/core/src/astro/routes/api/plugins/[pluginId]/[...path].ts:97This passthrough path is not exercised by any test.
routes.test.tsverifies the lower-levelPluginRouteHandler.invoke()shape, but the reported bug is about the HTTP endpoint returning the raw response. Add a focused test inpackages/core/tests/unit/astro/plugin-api-route-auth.test.tsthat mockshandlePluginApiRouteto return{ success: true, response: ... }and asserts the Astro route returns that exactResponseverbatim.it("returns a handler Response verbatim instead of wrapping it", async () => { const { locals, handlePluginApiRoute } = createLocals(null, true); const png = new Response(new Uint8Array([0x89, 0x50, 0x4e, 0x47]), { headers: { "Content-Type": "image/png" }, }); handlePluginApiRoute.mockResolvedValue({ success: true, response: png, status: 200 }); const res = await invoke(GET, "GET", locals, { csrf: false }); expect(res).toBe(png); expect(res.headers.get("Content-Type")).toBe("image/png"); }); -
[suggestion]
packages/core/src/astro/types.ts:210handlePluginApiRouteandhandlePublicPluginApiRoutenow return a result that may carryresponse: Responsefor trusted plugin routes, and the route file already readsstatusfrom that result via an unsafe cast. Reflecting both fields in the publicHandlerResponseinterface keeps thelocals.emdashcontract honest and lets callers (and the route file) use typed fields instead of casting.export interface HandlerResponse<T = unknown> { success: boolean; data?: T; response?: Response; status?: number; error?: { code: string; message: string; details?: Record<string, unknown>; }; }
github-actions
Bot
added
review/awaiting-author
What does this PR do?
Plugin API route handlers could only return JSON — whatever a handler returned was wrapped in the standard
{ success, data }envelope, so there was no way to serve an image, a file, or anything with a custom content type from a plugin route. Returning aResponsedidn't help either; it was JSON-serialized.A handler that returns a
Responsenow has it passed through verbatim:PluginRouteHandler.invokedetectsresult instanceof Responseand carries it onRouteResult.responsewith the Response's own status/_emdash/api/plugins/{pluginId}/{...path}) returns that Response directly instead ofapiSuccess()-wrapping it{ success, data }envelopehandleSandboxedRoute, so they stay JSON-only (documented on the type and in the changeset)The exact repro from the issue now works:
Closes #2110
Type of change
(Filed as a bug — an image route currently has no way to return its image. The change is additive and backwards-compatible; happy to re-file under a Discussion if you'd rather treat it as a feature.)
Checklist
pnpm typecheckpasses (packages/core)pnpm lintpasses for the changed files (pnpm lintoverall currently fails on a pre-existing error inpackages/registry-verification/src/records.tsthat reproduces on a cleanmain)pnpm testpasses (targeted:packages/core tests/unit/plugins/routes.test.ts, 39/39)pnpm formathas been runemdashpatch)AI-generated code disclosure
Tests
Three new cases in
packages/core/tests/unit/plugins/routes.test.ts:image/pngResponse is returned verbatim (result.responseis the Response,result.dataundefined, body bytes and content-type intact)Location) carries its status through to the resultresult.dataset,result.responseundefined)