fix(mcp): do not enable production mode at module import time
The top-level enableProductionMode() call added in #537 fixed a real issue (MCP server resolving the wrong database path at startup) but introduced test-isolation breakage as a side effect: merely importing src/mcp/server.ts flipped the global _productionMode flag, which then broke unrelated tests that depend on the default (development) database path resolution. This shows up concretely as "Store Creation > createStore throws without explicit path in test mode" failing on Bun (ubuntu-latest) in CI, because test/mcp.test.ts imports startMcpHttpServer from this module. Move the enableProductionMode() call from module scope into the two server entry points (startMcpServer and startMcpHttpServer). The fix originally intended by #537 — ensuring production mode is active before getDefaultDbPath runs — is preserved because both call sites still flip the flag before createStore / getDefaultDbPath. Importing the module for its exports no longer mutates global state. Verified locally against current main: CI failure on Bun (ubuntu-latest) reproduces on unmodified upstream main (14+ consecutive failed runs since #537 merged on 2026-04-09) and is resolved by this change. Refs: #537
This commit is contained in:
parent
e8de7cab02
commit
54262f566c
@ -32,8 +32,6 @@ import {
|
||||
import { getConfigPath } from "../collections.js";
|
||||
import { enableProductionMode } from "../store.js";
|
||||
|
||||
enableProductionMode();
|
||||
|
||||
// =============================================================================
|
||||
// Types for structured content
|
||||
// =============================================================================
|
||||
@ -541,6 +539,12 @@ Intent-aware lex (C++ performance, not sports):
|
||||
// =============================================================================
|
||||
|
||||
export async function startMcpServer(): Promise<void> {
|
||||
// Opt into production mode when the MCP server is actually started, not
|
||||
// when this module is merely imported for its exports. Importing the module
|
||||
// at the top level flipped the global production flag and broke test
|
||||
// isolation for downstream suites that expect the default (development)
|
||||
// database path behaviour.
|
||||
enableProductionMode();
|
||||
const configPath = getConfigPath();
|
||||
const store = await createStore({
|
||||
dbPath: getDefaultDbPath(),
|
||||
@ -566,6 +570,10 @@ export type HttpServerHandle = {
|
||||
* Binds to localhost only. Returns a handle for shutdown and port discovery.
|
||||
*/
|
||||
export async function startMcpHttpServer(port: number, options?: { quiet?: boolean }): Promise<HttpServerHandle> {
|
||||
// See startMcpServer() for the rationale — flip production mode here so the
|
||||
// HTTP transport resolves the real database path, without leaking state into
|
||||
// callers that only import this module for its exports (e.g. tests).
|
||||
enableProductionMode();
|
||||
const configPath = getConfigPath();
|
||||
const store = await createStore({
|
||||
dbPath: getDefaultDbPath(),
|
||||
|
||||
Loading…
Reference in New Issue
Block a user