fix: support HTTPS servers and handle named pipe ports

[rajan-chari]

Summary

Fixes #511. Two bugs that affected HTTPS and Azure App Service deployments:

1. ExpressAdapter silently falls back to HTTP when given an HTTPS server

ExpressAdapter constructor uses instanceof http.Server to detect when a server is passed. However, https.Server extends tls.Server (not http.Server), so https.createServer() instanceof http.Server returns false. The adapter silently falls through to create its own http.Server, ignoring the user's HTTPS server entirely — a silent HTTP downgrade with no error.

Fix: Import https and check serverOrApp instanceof http.Server || serverOrApp instanceof https.Server. Widen the constructor parameter and server field types to http.Server | https.Server.

2. DevtoolsPlugin crashes on Azure App Service named pipe ports

On Azure App Service (IIS + iisnode), the PORT environment variable is a Windows named pipe path like \.\pipe\507cb72a-.... DevtoolsPlugin calls parseInt(port, 10) + 1 to derive its port, which returns NaN + 1 = NaN, crashing the devtools HTTP server. Same root cause as #487 (fixed in #488 for HttpServer.start(), but missed in DevtoolsPlugin.onStart()).

Fix: Explicit branching — if port is a named pipe (non-numeric string), fall back to port 3979 with a warning suggesting customPort option.

Test plan

• 12/12 unit tests pass (10 existing + 2 new HTTPS tests)
• New test: https.createServer({} ) instanceof https.Server === true and instanceof http.Server === false (documents root cause)
• New test: ExpressAdapter(httpsServer) wires up correctly (stop rejects with "not running", not "managed externally")
• New test: route registration works on https.Server
• E2E: HTTPS server scenario — bot receives activities over HTTPS
• E2E: Named pipe scenario — DevtoolsPlugin starts on fallback port with warning
• E2E: HTTP regression — existing http.Server path unchanged

Files changed

File	Change
packages/apps/src/http/express-adapter.ts	Accept https.Server, widen types
packages/dev/src/plugin.ts	Handle named pipe port with fallback
packages/apps/src/http/express-adapter.spec.ts	2 new HTTPS server tests

Co-Authored-By: Claude Opus 4.6 (1M context) noreply@anthropic.com

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Fixes two deployment/runtime issues: (1) ExpressAdapter now correctly accepts and uses user-supplied HTTPS servers (preventing a silent HTTP downgrade), and (2) DevtoolsPlugin no longer crashes when PORT is a Windows named pipe (Azure App Service), falling back to a default devtools port with a warning.

Changes:

• Accept https.Server in ExpressAdapter by widening types and updating server detection logic.
• Add named-pipe-safe port derivation logic to DevtoolsPlugin.onStart().
• Add unit tests documenting the HTTPS instanceof behavior and basic HTTPS server wiring.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
packages/dev/src/plugin.ts	Avoids NaN port math on named pipes by using a fallback port and warning.
packages/apps/src/http/express-adapter.ts	Treats https.Server as a valid externally-managed server and stores it without downgrading to HTTP.
packages/apps/src/http/express-adapter.spec.ts	Adds tests covering HTTPS instanceof root cause and basic adapter acceptance of https.Server.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

customPort is checked via truthiness, so a valid value like 0 will be ignored and the code will fall back to deriving a port. Use an explicit null/undefined check instead (e.g., this.options.customPort != null) or check typeof this.options.customPort === 'number'.

[heyitsaamir]

IMO, this is not worth fixing. Devtools is supposed to be for local development.

[Copilot]

parseInt() will accept partially-numeric strings (e.g. "3000abc" => 3000), which can incorrectly treat an invalid PORT as valid. Consider using a stricter parse (e.g., Number(port) with Number.isFinite(...) plus a full-string numeric check) so only purely numeric ports are incremented; otherwise treat it as a named pipe/invalid string and use the fallback.

[Copilot]

This assertion is tightly coupled to a specific error message string, making the test brittle to future wording changes. A more robust assertion would directly verify the adapter is using the passed-in server instance (e.g., by checking internal state via a safe test-only accessor, or by asserting behavior that cannot depend on the exact wording, such as checking the error type/shape if available).

Suggested change

	// Verify the adapter stored the server: stop() should reject with
	// "Server is not running" (not "managed externally" which would mean
	// the adapter didn't recognize the https.Server and created its own)
	await expect(adapter.stop()).rejects.toThrow('Server is not running');
	// Verify the adapter stored the server by asserting stop() rejects in
	// this unstarted state, without coupling the test to exact wording.
	expect.assertions(2);
	try {
	await adapter.stop();
	} catch (error) {
	expect(error).toBeInstanceOf(Error);
	expect((error as Error).message).toEqual(expect.any(String));
	}