History

Fix Google Photos uploads and MCP HEAD responses (#1428)

* [verified] Fix Google Photos upload endpoint * Preserve MCP HEAD response metadata ---------

saga-agent committed 3 months ago

Commit df01d911 parent 45d020c9 files · +124 −18Browse files

Unified · Split

9 files
  1. .changeset/fix-google-photos-upload-path.md +5 −0
  2. apps/host-selfhost/src/mcp/mcp.test.ts +1 −0
  3. apps/host-selfhost/src/mcp/org-path.test.ts +18 −1
  4. apps/host-selfhost/src/mcp/org-path.ts +12 −0
  5. apps/host-selfhost/src/serve.ts +15 −1
  6. packages/hosts/mcp/src/envelope.test.ts +7 −0
  7. packages/hosts/mcp/src/envelope.ts +29 −15
  8. packages/plugins/openapi/src/providers/google/discovery.test.ts +36 −0
  9. packages/plugins/openapi/src/providers/google/discovery.ts +1 −1
.changeset/fix-google-photos-upload-path.md+5−0
@@ -0,0 +1,5 @@
1+---
2+"@executor-js/plugin-openapi": patch
3+---
4+
5+Use the versioned Google Photos raw upload endpoint so generated upload tools send media to `/v1/uploads` instead of the invalid `/uploads` path.
apps/host-selfhost/src/mcp/mcp.test.ts+1−0
@@ -57,6 +57,7 @@ const initSession = async (token: string): Promise<string> => { · 56 unmodified lines
5757 },
5858 });
5959 expect(res.status).toBe(200);
60+ expect(res.headers.get("content-type")).toContain("application/json");
6061 const sessionId = res.headers.get("mcp-session-id") ?? "";
6162 expect(sessionId).not.toBe("");
6263 await res.text();
apps/host-selfhost/src/mcp/org-path.test.ts+18−1
@@ -1,6 +1,6 @@
11 import { describe, expect, it } from "@effect/vitest";
22
3−import { stripMcpOrgSegment } from "./org-path";
3+import { isMcpServingPath, stripMcpOrgSegment } from "./org-path";
44
55 describe("stripMcpOrgSegment", () => {
66 it("strips a single org segment before /mcp", () => {
@@ -32,3 +32,20 @@ describe("stripMcpOrgSegment", () => { · 25 unmodified lines
3232 expect(stripMcpOrgSegment("/a/b/mcp")).toBeNull(); // deeper than one segment
3333 });
3434 });
35+
36+describe("isMcpServingPath", () => {
37+ it("recognizes bare and org-scoped MCP serving routes", () => {
38+ expect(isMcpServingPath("/mcp")).toBe(true);
39+ expect(isMcpServingPath("/mcp/")).toBe(true);
40+ expect(isMcpServingPath("/mcp/toolkits/deploy")).toBe(true);
41+ expect(isMcpServingPath("/default/mcp")).toBe(true);
42+ expect(isMcpServingPath("/default/mcp/toolkits/deploy")).toBe(true);
43+ });
44+
45+ it("does not claim discovery, OAuth, or unrelated paths", () => {
46+ expect(isMcpServingPath("/.well-known/oauth-protected-resource")).toBe(false);
47+ expect(isMcpServingPath("/api/auth/mcp/authorize")).toBe(false);
48+ expect(isMcpServingPath("/mcp/unknown")).toBe(false);
49+ expect(isMcpServingPath("/")).toBe(false);
50+ });
51+});
apps/host-selfhost/src/mcp/org-path.ts+12−0
@@ -72,6 +72,18 @@ export const MCP_ORIGINAL_PATH_HEADER = "x-executor-mcp-original-path"; · 71 unmodified lines
7272 export const isRecognizedMcpOrgPath = (pathname: string): boolean =>
7373 stripMcpOrgSegment(pathname) !== null;
7474
75+/**
76+ * Whether `pathname` targets one of self-host's MCP serving endpoints, in
77+ * either bare or org-scoped form. Used by the production HTTP boundary to
78+ * reject HEAD explicitly before the SPA's GET/HEAD fallback can claim it.
79+ */
80+export const isMcpServingPath = (pathname: string): boolean => {
81+ const routed = stripMcpOrgSegment(pathname) ?? pathname;
82+ if (routed === "/mcp" || routed === "/mcp/") return true;
83+ const segments = routed.split("/").filter((segment) => segment.length > 0);
84+ return segments.length === 3 && segments[0] === "mcp" && segments[1] === "toolkits";
85+};
86+
7587 /**
7688 * Given a recognized original pathname (a `MCP_ORIGINAL_PATH_HEADER` value —
7789 * either the org-scoped MCP path itself, or its PRM-prefixed discovery-doc
apps/host-selfhost/src/serve.ts+15−1
@@ -25,6 +25,7 @@ import { · 24 unmodified lines
2525 } from "effect/unstable/http";
2626 import { BunFileSystem, BunHttpServer, BunPath, BunRuntime } from "@effect/platform-bun";
2727 import { Effect, Layer } from "effect";
28+import { jsonRpcErrorBody } from "@executor-js/host-mcp";
2829
2930 import { disposeAnalytics } from "./analytics";
3031 import { makeSelfHostApp } from "./app";
@@ -34,7 +35,7 @@ import { · 3 unmodified lines
3435 OAUTH_CALLBACK_PATH,
3536 oauthCallbackSignInRedirectLocation,
3637 } from "./auth/oauth-callback-login";
37−import { MCP_ORIGINAL_PATH_HEADER, stripMcpOrgSegment } from "./mcp/org-path";
38+import { isMcpServingPath, MCP_ORIGINAL_PATH_HEADER, stripMcpOrgSegment } from "./mcp/org-path";
3839
3940 const distDir = fileURLToPath(new URL("../dist/", import.meta.url));
4041 const assetsDir = fileURLToPath(new URL("../dist/assets/", import.meta.url));
@@ -53,6 +54,19 @@ const selfHostHttpMiddleware = (betterAuth: BetterAuthHandle) => · 12 unmodified lines
5354 Effect.gen(function* () {
5455 const request = yield* HttpServerRequest.HttpServerRequest;
5556 const url = new URL(request.url, "http://host.internal");
57+ // Streamable HTTP does not define HEAD. Reject it before the SPA's
58+ // GET/HEAD fallback can claim `/mcp` and return a misleading empty
59+ // `200 application/octet-stream` response. The shared MCP envelope
60+ // rejects unsupported methods the same way, but the static route wins
61+ // HEAD routing at the composed production-server boundary.
62+ if (request.method === "HEAD" && isMcpServingPath(url.pathname)) {
63+ const response = jsonRpcErrorBody(405, -32001, "Method not allowed");
64+ return HttpServerResponse.raw(response, {
65+ status: response.status,
66+ statusText: response.statusText,
67+ headers: response.headers,
68+ });
69+ }
5670 if (
5771 url.pathname === OAUTH_CALLBACK_PATH &&
5872 (request.method === "GET" || request.method === "HEAD")
packages/hosts/mcp/src/envelope.test.ts+7−0
@@ -79,6 +79,13 @@ const buildHandler = ( · 78 unmodified lines
7979 };
8080
8181 describe("McpServingRoutes envelope", () => {
82+ it("rejects HEAD with a JSON-RPC 405 before dispatch", async () => {
83+ const handler = buildHandler(OkStoreLive, McpErrorReporterNoop);
84+ const response = await handler(new Request("https://host.test/mcp", { method: "HEAD" }));
85+ expect(response.status).toBe(405);
86+ expect(response.headers.get("content-type")).toContain("application/json");
87+ });
88+
8289 it("rejects a non-GET/POST/DELETE/OPTIONS method with 405 -32001 before dispatch", async () => {
8390 const handler = buildHandler(OkStoreLive, McpErrorReporterNoop);
8491 for (const method of ["PUT", "PATCH"] as const) {
packages/hosts/mcp/src/envelope.ts+29−15
@@ -35,9 +35,9 @@ import { · 34 unmodified lines
3535 // Runtime-agnostic: built on `effect/unstable/http` (HttpRouter), NO
3636 // platform-bun. The `/mcp` flow is fully Effect; the streamable-HTTP transport
3737 // works on web `Request`/`Response`, so the envelope reconstructs the inbound
38−// web request once, hands it to the store, and wraps the store's `Response`
39−// with `HttpServerResponse.raw` (which passes a `Response` body through
40−// unchanged, preserving streaming SSE bodies).
38+// web request once, hands it to the store, and converts the store's `Response`
39+// into an Effect response while preserving both streaming bodies and outer
40+// metadata (the latter matters when the HTTP adapter strips a HEAD body).
4141 // ---------------------------------------------------------------------------
4242
4343 const MCP_PATH = "/mcp";
@@ -46,6 +46,22 @@ const TOOLKIT_MCP_PATH = "/mcp/toolkits/:toolkitSlug"; · 2 unmodified lines
4646 /** The methods the streamable-HTTP transport accepts on `/mcp`. */
4747 const ALLOWED_MCP_METHODS = new Set(["GET", "POST", "DELETE", "OPTIONS"]);
4848
49+/**
50+ * Preserve a WHATWG response's status and headers on the Effect wrapper.
51+ *
52+ * Passing only `response` to `HttpServerResponse.raw` works for ordinary
53+ * requests because `toWeb` returns the nested Response verbatim. For HEAD,
54+ * however, the adapter intentionally omits the nested body and serializes the
55+ * outer wrapper instead; without copied metadata that becomes an empty 200
56+ * with no content type.
57+ */
58+const fromWebResponse = (response: Response): HttpServerResponse.HttpServerResponse =>
59+ HttpServerResponse.raw(response, {
60+ status: response.status,
61+ statusText: response.statusText,
62+ headers: response.headers,
63+ });
64+
4965 /**
5066 * The canonical CORS preflight `Response` (204) answered for an `OPTIONS` on
5167 * `/mcp` AND on every provider-declared discovery path. A browser issues a
@@ -152,7 +168,7 @@ const discoveryRoute = (handler: (request: Request) => Effect.Effect<Response>) · 100 unmodified lines
152168 const httpRequest = yield* HttpServerRequest.HttpServerRequest;
153169 const request = yield* toWebRequest(httpRequest);
154170 const response = yield* handler(request);
155− return HttpServerResponse.raw(response);
171+ return fromWebResponse(response);
156172 });
157173
158174 /**
@@ -206,7 +222,7 @@ const mcpDispatch = (resource: McpResource) => · 47 unmodified lines
206222
207223 // CORS preflight: answer before auth so unauthenticated clients can probe.
208224 if (request.method === "OPTIONS") {
209− return HttpServerResponse.raw(corsPreflightResponse());
225+ return fromWebResponse(corsPreflightResponse());
210226 }
211227
212228 // Streamable-HTTP only defines GET/POST/DELETE on the endpoint. Any other
@@ -214,7 +230,7 @@ const mcpDispatch = (resource: McpResource) => · 1 unmodified line
214230 // otherwise it would fall through and spin up a session engine for a method
215231 // the transport can't serve.
216232 if (!ALLOWED_MCP_METHODS.has(request.method)) {
217− return HttpServerResponse.raw(jsonRpcResponse(405, -32001, "Method not allowed"));
233+ return fromWebResponse(jsonRpcResponse(405, -32001, "Method not allowed"));
218234 }
219235
220236 const sessionId = request.headers.get("mcp-session-id");
@@ -225,7 +241,7 @@ const mcpDispatch = (resource: McpResource) => · 4 unmodified lines
225241 // resource; an auth-level Forbidden may not carry either.
226242 const outcome = yield* auth.authenticate(request);
227243 if (!Predicate.isTagged(outcome, "Authenticated")) {
228− return HttpServerResponse.raw(renderAuthError(auth, request, outcome));
244+ return fromWebResponse(renderAuthError(auth, request, outcome));
229245 }
230246 const principal = outcome.principal;
231247
@@ -235,12 +251,12 @@ const mcpDispatch = (resource: McpResource) => · 3 unmodified lines
235251 // an engine for a bare GET/DELETE.
236252 if (!sessionId) {
237253 if (request.method === "GET") {
238− return HttpServerResponse.raw(
254+ return fromWebResponse(
239255 jsonRpcResponse(400, -32000, "mcp-session-id header required for SSE"),
240256 );
241257 }
242258 if (request.method === "DELETE") {
243− return HttpServerResponse.raw(
259+ return fromWebResponse(
244260 new Response(null, { status: 204, headers: { "access-control-allow-origin": "*" } }),
245261 );
246262 }
@@ -253,9 +269,7 @@ const mcpDispatch = (resource: McpResource) => · 6 unmodified lines
253269 sessionId,
254270 method: request.method,
255271 });
256− return HttpServerResponse.raw(
257− result instanceof Response ? result : renderDispatchError(result),
258− );
272+ return fromWebResponse(result instanceof Response ? result : renderDispatchError(result));
259273 });
260274
261275 /**
@@ -272,7 +286,7 @@ const mcpRoute = (resource: McpResource) => · 10 unmodified lines
272286 Effect.gen(function* () {
273287 const reporter = yield* McpErrorReporter;
274288 yield* reporter.report(cause);
275− return HttpServerResponse.raw(jsonRpcResponse(500, -32603, "Internal server error"));
289+ return fromWebResponse(jsonRpcResponse(500, -32603, "Internal server error"));
276290 }),
277291 ),
278292 );
@@ -301,7 +315,7 @@ export const McpServingRoutes = HttpRouter.use((router) => · 22 unmodified lines
301315 yield* router.add(
302316 "OPTIONS",
303317 route.path,
304− Effect.sync(() => HttpServerResponse.raw(corsPreflightResponse())),
318+ Effect.sync(() => fromWebResponse(corsPreflightResponse())),
305319 );
306320 }
307321 yield* router.add("*", MCP_PATH, mcpRoute(defaultMcpResource));
@@ -327,7 +341,7 @@ export const McpDiscoveryRoutes = HttpRouter.use((router) => · 19 unmodified lines
327341 yield* router.add(
328342 "OPTIONS",
329343 route.path,
330− Effect.sync(() => HttpServerResponse.raw(corsPreflightResponse())),
344+ Effect.sync(() => fromWebResponse(corsPreflightResponse())),
331345 );
332346 }
333347 }),
packages/plugins/openapi/src/providers/google/discovery.test.ts+36−0
@@ -491,6 +491,42 @@ it.effect("marks Google Discovery media-download methods as binary responses", ( · 490 unmodified lines
491491 }),
492492 );
493493
494+it.effect("uses the versioned Google Photos raw upload endpoint", () =>
495+ Effect.gen(function* () {
496+ const result = yield* convertGoogleDiscoveryToOpenApi({
497+ discoveryUrl: "https://www.googleapis.com/discovery/v1/apis/photoslibrary/v1/rest",
498+ // @effect-diagnostics-next-line preferSchemaOverJson:off
499+ documentText: JSON.stringify({
500+ name: "photoslibrary",
501+ version: "v1",
502+ title: "Google Photos Library API",
503+ rootUrl: "https://photoslibrary.googleapis.com/",
504+ servicePath: "",
505+ auth: {
506+ oauth2: {
507+ scopes: {
508+ "https://www.googleapis.com/auth/photoslibrary.appendonly": {
509+ description: "Upload to Google Photos",
510+ },
511+ },
512+ },
513+ },
514+ resources: {},
515+ schemas: {},
516+ }),
517+ });
518+
519+ const spec = decodeConvertedSpec(result.specText);
520+ const upload = spec.paths["/v1/uploads"]?.post;
521+ expect(upload).toMatchObject({
522+ operationId: "photoslibrary.mediaItems.upload",
523+ "x-executor-toolPath": "photoslibrary.mediaItems.upload",
524+ "x-executor-pathTemplate": "/v1/uploads",
525+ servers: [{ url: "https://photoslibrary.googleapis.com/" }],
526+ });
527+ }),
528+);
529+
494530 it.effect("supplies documented scopes when Picker Discovery omits auth metadata", () =>
495531 Effect.gen(function* () {
496532 const result = yield* convertGoogleDiscoveryToOpenApi({
packages/plugins/openapi/src/providers/google/discovery.ts+1−1
@@ -829,7 +829,7 @@ const GOOGLE_OAUTH_SECURITY_SCHEME = "googleOAuth2"; · 828 unmodified lines
829829 const GOOGLE_PHOTOS_LIBRARY_SERVICE = "photoslibrary";
830830 const GOOGLE_PHOTOS_APPENDONLY_SCOPE = "https://www.googleapis.com/auth/photoslibrary.appendonly";
831831 const GOOGLE_PHOTOS_UPLOAD_TOOL_PATH = "photoslibrary.mediaItems.upload";
832−const GOOGLE_PHOTOS_UPLOAD_PATH = "/uploads";
832+const GOOGLE_PHOTOS_UPLOAD_PATH = "/v1/uploads";
833833
834834 const isGooglePhotosService = (service: string): boolean =>
835835 service === GOOGLE_PHOTOS_LIBRARY_SERVICE || service === GOOGLE_PHOTOS_PICKER_SERVICE;