[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

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.

Triage — graded, finding cleared. Verified at origin/main ed44512.

Confirmed, line for line

packages/mcp/src/plugin.ts:112-118 — step 2 says the transport starts "only when autoStart is enabled or OS_MCP_SERVER_ENABLED is explicitly true", and the Environment Variables block says explicit true "additionally auto-starts the stdio transport". Neither names OS_MCP_STDIO_ENABLED at all.
packages/mcp/src/plugin.ts:226-239 — the code reads resolveMcpStdioAutoStart() and warns that OS_MCP_SERVER_ENABLED=true is DEPRECATED, telling the operator to use OS_MCP_STDIO_ENABLED.
packages/types/src/env.ts:333-345 — canonical switch first, legacy OS_MCP_SERVER_ENABLED === 'true' second with viaDeprecatedAlias: true. The env.ts docblock above it (:307-328) carries the split correctly.

So the docblock presents the deprecated trigger as the design and never names the canonical one, 100 lines above a warning that says the opposite. The card is accurate.

The card's open question, answered — the blast radius is one file

The card asked whether the same pre-split wording survives in packages/mcp/README.md or the docs tree. Measured: it does not.

packages/mcp/README.md:73,:79 scope OS_MCP_SERVER_ENABLED to the HTTP surface only, and the README already names OS_MCP_STDIO_ENABLED.
content/docs/deployment/environment-variables.mdx:256 gates it to HTTP, and :263-267 carries the deprecation explicitly ("Deprecated (one release): OS_MCP_SERVER_ENABLED=true also used to auto-start …", plus the =false never-started-stdio caveat).
content/docs/releases/v16.mdx:170-173 announced the split.

Every neighbour was updated when the split landed; plugin.ts's own class docblock was the one span missed. That makes the fix exactly the four comment lines the card proposes, with no doc sweep riding along — which is why this grades as a small card rather than a doc-consistency campaign.

Scope ruled

packages/mcp/src/plugin.ts comments only:

step 2 → "autoStart is enabled or OS_MCP_STDIO_ENABLED is truthy";
Environment Variables → keep OS_MCP_SERVER_ENABLED as the HTTP gate, add OS_MCP_STDIO_ENABLED as the stdio switch, mark the legacy OS_MCP_SERVER_ENABLED=true stdio trigger deprecated in the same words the runtime warning uses.

No .describe() twin is owed (the card says so and it is right — this is a code comment, not spec prose). No behaviour change: touching the runtime here would be a different card.

Routing

domain:cli — packages/mcp sits in that lane per the lane table, and the fix lands nowhere else. priority:p3: comments only, no runtime effect. It is not p4-and-forget because the card measured the cost already paid once — the published skills/objectstack-ai MCP section was written from this docblock and shipped the error, caught in contract review and fixed in PR #14463 (5dabfd2a). This docblock is the upstream of that defect and it is still uncorrected. pm:queue.

Generated by Claude Code

Navigation Menu

Repository navigation

[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

The drift

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

Suggested fix

Confirmed, line for line

The card's open question, answered — the blast radius is one file

Scope ruled

Routing

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[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

The drift

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

Suggested fix

Activity

huangyiirene commented on Sep 2, 2026

Confirmed, line for line

The card's open question, answered — the blast radius is one file

Scope ruled

Routing

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

[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