feat: add Streamable HTTP transport support by keykbd · Pull Request #326 · kintone/mcp-server

keykbd · 2026-03-04T03:47:42Z

Why

Addresses the HTTP transport portion of #304.

Currently the server only supports stdio transport, which requires a 1:1 process
per client. Adding Streamable HTTP transport enables deployment on remote
environments such as AWS AgentCore, or running as a shared MCP server for a team.

What

Add --transport http to start in Streamable HTTP mode (default remains stdio)
No new runtime dependencies (uses SDK built-in StreamableHTTPServerTransport + node:http)
Stateless mode (each request creates an independent session)
Tests added (14 for HTTP server, 9 for config parsing)

Out of scope (planned for a follow-up PR):

Dynamic authentication (the second half of [Feature Request] Support for Streamable HTTP Transport (with Dynamic Authentication) #304)

How to test

pnpm build && pnpm test && pnpm lint

To manually verify HTTP mode:

KINTONE_BASE_URL=https://example.cybozu.com \
KINTONE_USERNAME=user KINTONE_PASSWORD=pass \
node dist/index.js --transport http
# -> HTTP server listening on http://127.0.0.1:3000/mcp

Checklist

Updated documentation if it is required.
Added tests if it is required.
Passed pnpm lint and pnpm test on the root directory.

cc @nameless-mc @shabaraba @neos-nozaki

Add --transport http option to start in Streamable HTTP mode (default remains stdio). Uses SDK built-in StreamableHTTPServerTransport + node:http with no new runtime dependencies. - Stateless mode (each request creates an independent session) - Origin header validation, Content-Type check, 1MB body limit - Default 127.0.0.1 bind, --port and --hostname options - Graceful shutdown with SIGTERM/SIGINT handling - Tests added (14 for HTTP server, 9 for config parsing)

shabaraba · 2026-03-11T05:53:37Z

@keykbd
Thank you for creating the PR. We'll take a look later!

shabaraba

@keykbd
Thank you for this PR. We apologize for the very delayed review 🙇

We've gone through the changes and left some review comments. Please take a look when you get a chance.

Thank you!

- Remove "localhost" from LOOPBACK_HOSTS to prevent DNS rebinding attacks - Make cleanup idempotent to avoid double-closing transport/server - Remove no-op assertion from oversized body test - Add test verifying cleanup is called on successful request completion - Update localhost origin test to reflect new DNS rebinding protection behavior - Update README/README_en TOC via pnpm doc:update-toc

nameless-mc · 2026-07-13T03:01:25Z

+**注意事項:**
+
+- デフォルトでは `127.0.0.1` にバインドされるため、ローカルからのみアクセス可能です
+- 外部からのアクセスを許可するには `--hostname 0.0.0.0` を指定してください
+- HTTPモードはステートレスで動作します（リクエストごとにセッションが独立）


[SHOULD / security] The notes section mentions that --hostname 0.0.0.0 allows external access, but doesn't warn that the HTTP endpoint has no authentication and no encryption. The Docker example also uses --hostname 0.0.0.0 by default, making it easy to copy-paste into an insecure configuration.

Suggestion — add a warning like:

**注意事項:** - デフォルトでは `127.0.0.1` にバインドされるため、ローカルからのみアクセス可能です - 外部からのアクセスを許可するには `--hostname 0.0.0.0` を指定してください - **`0.0.0.0` で公開する場合、HTTPエンドポイントには認証がありません。信頼できないネットワークではリバースプロキシ（TLS + 認証）の背後に配置してください** - HTTPモードはステートレスで動作します（リクエストごとにセッションが独立）

(Same for README_en.md)

Added the auth/TLS warning to the 0.0.0.0 notes in both README.md and README_en.md.

nameless-mc · 2026-07-13T03:01:57Z

+        limitExceeded = true;
+        res.writeHead(413, { "Content-Type": "application/json" });
+        res.end(JSON.stringify({ error: "Payload Too Large" }));


[SHOULD / maintainability] The res.writeHead(status, {"Content-Type":"application/json"}); res.end(JSON.stringify({error:...})) pattern appears 7 times in this file (413/415/400/500/400/404/403). A small helper would reduce duplication and make future changes (e.g., adding an error code field) easier to apply consistently.

const sendJsonError = (res: ServerResponse, status: number, message: string) => { res.writeHead(status, { "Content-Type": "application/json" }); res.end(JSON.stringify({ error: message })); };

Extracted sendJsonError as suggested and replaced all 7 call sites.

nameless-mc · 2026-07-13T03:02:25Z

  };
 };
+
+export const getTransportConfig = (): TransportConfig => parseTransportConfig();


[SHOULD / maintainability] The other config getters in this file cache their parsed result at module load time (line 10: const config = parseKintoneMcpServerConfig()), but getTransportConfig re-parses process.argv on every call. No practical issue today since it's called once, but it breaks the established pattern.

Suggested change

export const getTransportConfig = (): TransportConfig => parseTransportConfig();

const transportConfig = parseTransportConfig();

export const getTransportConfig = (): TransportConfig => transportConfig;

Cached transportConfig at module load time, matching the other config getters.

nameless-mc · 2026-07-13T03:02:51Z

+  if (transportConfig.transport === "http") {
+    console.error("Starting HTTP server...");
+    const httpServer = await startHttpServer(
+      serverConfig,
+      transportConfig.port,
+      transportConfig.hostname,
+    );
+
+    let shuttingDown = false;
+    const shutdown = () => {
+      if (shuttingDown) return;
+      shuttingDown = true;
+      console.error("Shutting down HTTP server...");
+      httpServer.close();
+
+      // Force exit if connections linger
+      setTimeout(() => {
+        console.error("Forcing shutdown after timeout");
+        httpServer.closeAllConnections();
+      }, SHUTDOWN_TIMEOUT_MS).unref();
+    };
+    process.on("SIGTERM", shutdown);
+    process.on("SIGINT", shutdown);


[SHOULD / test-coverage] The new logic here — transport branching (http vs stdio), SIGTERM/SIGINT handler registration, the shuttingDown idempotency flag, and the SHUTDOWN_TIMEOUT_MS forced exit — has no test coverage. Consider extracting the shutdown setup into a testable function, or at minimum adding a test with vi.useFakeTimers() to verify the timeout behavior.

Extracted the shutdown setup into src/shutdown.ts (setupGracefulShutdown) and added src/__tests__/shutdown.test.ts covering SIGTERM/SIGINT, idempotency, and the timeout behavior via vi.useFakeTimers().

nameless-mc · 2026-07-13T03:05:44Z

+    expect(response.status).toBe(200);
+    expect(mockHandleRequest).toHaveBeenCalledOnce();
+  });
+});


[SHOULD / test-coverage] isAllowedOrigin has three branches: loopback, all-interfaces (0.0.0.0/::), and specific hostname. The tests only exercise the first two (127.0.0.1 and 0.0.0.0). The third branch (normalizeHostname(originHost) === normalizedBind) is never tested.

Consider adding cases like:

it("should allow same hostname origin when binding to specific hostname", async () => { server = await startHttpServer(mockServerConfig, 0, "192.168.1.100"); const response = await makeRequest(server, { body: "{}", headers: { "Content-Type": "application/json", Origin: "http://192.168.1.100:3000" }, }); expect(response.status).toBe(200); }); it("should reject different hostname origin when binding to specific hostname", async () => { server = await startHttpServer(mockServerConfig, 0, "192.168.1.100"); const response = await makeRequest(server, { body: "{}", headers: { "Content-Type": "application/json", Origin: "http://192.168.1.200:3000" }, }); expect(response.status).toBe(403); });

Added coverage for the specific-hostname branch. Note: I exported isAllowedOrigin and unit-tested it directly rather than via startHttpServer, since binding to 192.168.1.100 fails with EADDRNOTAVAIL in this environment.

nameless-mc · 2026-07-13T03:06:50Z

+    if (req.method === "POST") {
+      await handleMcpPost(req, res, serverConfig);
+    } else {
+      // Delegate GET/DELETE to SDK transport (stateless mode returns 405)
+      await handleMcpTransport(req, res, serverConfig);
+    }


[CONSIDER / correctness] Origin header validation is implemented, but CORS response headers (Access-Control-Allow-Origin, etc.) and OPTIONS preflight handling are not. Browser-based MCP clients would fail with CORS errors. Not a problem if browser clients are out of scope, but worth noting for future consideration.

Agreed this is out of scope for now as noted — leaving as-is.

nameless-mc · 2026-07-13T03:07:30Z

+      return;
+    }
+
+    if (url.pathname !== "/mcp") {


[CONSIDER / maintainability] The "/mcp" path literal appears here and again in the log message at line 195. A small constant (const MCP_PATH = "/mcp") would prevent them from diverging in a future change.

Extracted as a MCP_PATH constant.

nameless-mc · 2026-07-13T03:08:00Z

+      }
+    });
+    req.on("error", () => {
+      if (!limitExceeded) {
+        resolve(null);
+      }


[CONSIDER / correctness] When req.on("error") fires and limitExceeded is false, resolve(null) is returned. The caller (handleMcpPost) then returns without sending any response, leaving res open. In practice, a req error usually means the client disconnected (so sending a response wouldn't help), but the res object stays open until the HTTP server's internal timeout cleans it up.

One option: destroy res here so resources are freed immediately.

req.on("error", () => { if (!limitExceeded) { if (!res.headersSent) res.destroy(); resolve(null); } });

Added res.destroy() in the error path as suggested.

nameless-mc · 2026-07-13T04:12:28Z

[CONSIDER / security] The HTTP server is plaintext only. For localhost usage this is fine. If exposed on a network (0.0.0.0), the MCP client ↔ server communication (including JSON-RPC payloads) is unencrypted. Recommending a reverse proxy with TLS termination in the README (as part of the 0.0.0.0 warning) would be sufficient for now.

Folded this into the 0.0.0.0 warning update (comment above) in both README files.

nameless-mc · 2026-07-13T04:13:05Z

+export const parseTransportConfig = (): TransportConfig => {
+  const cmdArgs = parse(process.argv);
+  const result = transportConfigSchema.safeParse({
+    TRANSPORT: cmdArgs.transport,
+    PORT: cmdArgs.port,
+    HOSTNAME: cmdArgs.hostname,
+  });


[SHOULD] parseTransportConfig reads only CLI args and not environment variables, while the existing parseKintoneMcpServerConfig uses merge(process.env, cmdArgs) to support both. This inconsistency means docker run -e TRANSPORT=http won't work — users must pass --transport http as a container argument instead.

At minimum PORT should have an env var fallback, as PORT is a common convention in Docker / Cloud Run / Heroku environments. Aligning with the existing merge() pattern would keep the config layer consistent.

Added env var fallback for TRANSPORT/PORT/HOSTNAME, aligned with the existing merge() pattern.

nameless-mc · 2026-07-13T05:01:41Z

[SHOULD / correctness] The kintone-download-file tool writes files to the server's local filesystem (KINTONE_ATTACHMENTS_DIR) and returns the absolute file path. In stdio mode this works because the server and client share the same machine, but in HTTP transport mode the server may be remote — the returned path is on the server's filesystem and inaccessible to the MCP client.

Options to consider:

Disable kintone-download-file when running in HTTP mode (or warn in the tool's output)

Document this limitation in the HTTP transport section of the README

In the future, return file contents inline (e.g., base64-encoded) instead of a path

Documented this limitation in the README's HTTP transport section (both languages).

- Extract sendJsonError helper to remove duplicated error-response code in http.ts - Constantize the "/mcp" path literal as MCP_PATH - Destroy the response on request error to free resources early - Cache getTransportConfig at module load time, matching other config getters - Add env var fallback (TRANSPORT/PORT/HOSTNAME) to parseTransportConfig - Extract graceful shutdown logic into a testable setupGracefulShutdown function - Add test coverage for shutdown behavior, isAllowedOrigin's specific-hostname branch, and config env var fallback - Document the 0.0.0.0 auth/TLS caveat and kintone-download-file's HTTP-mode file path limitation in README/README_en

- Remove HOSTNAME from parseTransportConfig's env fallback since it collides with the reserved HOSTNAME variable auto-set by Docker/shells, which could break HTTP mode startup when --hostname is omitted - Update README/README_en config tables to reflect that only TRANSPORT and PORT support env vars - Add tests for parseRequestUrl's invalid-URL (400) path and readBody's req.on("error") path, both previously flagged as untested in the original review but missed across two rounds of fixes

shabaraba · 2026-07-23T08:18:16Z

@keykbd
Thank you for creating this PR, and sorry again for the delayed review 🙇

We've gone ahead and addressed the remaining review comments ourselves. No further action needed on your side — we'll take it from here.

Thank you for your patience!

keykbd · 2026-07-24T06:29:18Z

@shabaraba
Thank you for handling the rest! Happy to see this moving forward 🙏

keykbd requested a review from a team as a code owner March 4, 2026 03:47

keykbd requested review from chihiro-adachi and nameless-mc and removed request for a team March 4, 2026 03:47

josh-vo mentioned this pull request Jun 5, 2026

feat: add Streamable HTTP transport support josh-vo/mcp-server#1

Merged

3 tasks

shabaraba self-assigned this Jun 29, 2026

shabaraba reviewed Jun 30, 2026

View reviewed changes

Comment thread src/transport/http.ts Outdated

Comment thread src/transport/http.ts

Comment thread README.md

Comment thread src/transport/__tests__/http.test.ts Outdated

Comment thread src/transport/__tests__/http.test.ts

neos-nozaki mentioned this pull request Jun 30, 2026

[Feature Request] Support for Streamable HTTP Transport (with Dynamic Authentication) #304

Open

nameless-mc reviewed Jul 13, 2026

View reviewed changes

shabaraba added 2 commits July 13, 2026 16:36

keykbd closed this Jul 24, 2026

keykbd reopened this Jul 24, 2026

-export const getTransportConfig = (): TransportConfig => parseTransportConfig();
+const transportConfig = parseTransportConfig();
+export const getTransportConfig = (): TransportConfig => transportConfig;

Uh oh!

Conversation

keykbd commented Mar 4, 2026

Why

What

How to test

Checklist

Uh oh!

shabaraba commented Mar 11, 2026

Uh oh!

shabaraba left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shabaraba commented Jul 23, 2026

Uh oh!

keykbd commented Jul 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants