Skip to content

[finding] MCPServerPlugin's own docblock still teaches the deprecated stdio trigger — plugin.ts:112-122 disagrees with plugin.ts:234-239 twelve lines below it #14473

Description

@os-litant

Out-of-scope by-product of the skills optimization flight on #14305. It is the source of a defect that already shipped once: the flight's MCP section was written from this docblock and inherited its error, caught in contract review round 1 and fixed in PR #14463 at 5dabfd2a. Filed unassigned. A code comment disagreeing with its own code — no spec .describe() twin is owed.

The drift

packages/mcp/src/plugin.ts:112-122, the class docblock:

 * 2. **start** — … Starts the long-lived transport (stdio) only when
 *    `autoStart` is enabled or `OS_MCP_SERVER_ENABLED` is explicitly `true` — …
 *
 * Environment Variables:
 * - `OS_MCP_SERVER_ENABLED` — HTTP surface default-on; `false` disables it,
 *   explicit `true` additionally auto-starts the stdio transport

packages/mcp/src/plugin.ts:226-239, the code it documents:

    // ── stdio auto-start decision (opt-in, its OWN switch) ──
    // … it stays opt-in via a SEPARATE switch
    // (`OS_MCP_STDIO_ENABLED` / the `autoStart` option), never the HTTP var.
    const stdio = resolveMcpStdioAutoStart();
    const shouldStart = this.options.autoStart || stdio.enabled;
    if (stdio.viaDeprecatedAlias && !this.options.autoStart) {
      ctx.logger.warn(
        '[MCP] Starting the stdio transport via OS_MCP_SERVER_ENABLED=true is DEPRECATED — …
         Use OS_MCP_STDIO_ENABLED=true (or the plugin `autoStart` option)',
      );
    }

resolveMcpStdioAutoStart() (packages/types/src/env.ts:333-345) reads
OS_MCP_STDIO_ENABLED first and returns it clean; OS_MCP_SERVER_ENABLED=true falls through
to the legacy branch and returns viaDeprecatedAlias: true. So the docblock names only the
deprecated trigger, never the canonical one, and presents the deprecated behaviour as the
design. The env.ts docblock (:307-328) has the split right — this one was not updated with
it.

Why it is worth a card rather than a drive-by

The blast radius is measured, not hypothetical. This docblock is what a reader (human or AI)
lands on from MCPServerPlugin, and it is demonstrably what the published
skills/objectstack-ai package was written from: an author following it sets
OS_MCP_SERVER_ENABLED=true, gets a working transport plus a deprecation warning at every
boot
, and has no way from this file to learn the right spelling. One catch happened to be in
review; the next may not be.

Suggested fix

Four lines of comment in packages/mcp/src/plugin.ts:

  • Step 2: "…only when autoStart is enabled or OS_MCP_STDIO_ENABLED is truthy".
  • Environment Variables: keep OS_MCP_SERVER_ENABLED as the HTTP-surface gate only; add
    OS_MCP_STDIO_ENABLED as the stdio switch; mark the legacy OS_MCP_SERVER_ENABLED=true
    stdio trigger deprecated, matching the warning string 100 lines below.

Worth checking whether the same pre-split wording survives in packages/mcp/README.md or the
docs tree; this audit only measured the plugin docblock. Related, closed, not a duplicate:
#9579 (packages/mcp/README.md documenting runtime methods that do not exist) is the same
defect class on a neighbouring file, and its close-out did not cover this span.

Dedupe: one targeted search_issues over this repo (repo-scoped REST answers 403 for this
seat), returning a non-empty result set, so the session's search is not in the silent-zero
mode. Nothing open covers this.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions