Uh oh!
There was an error while loading. Please reload this page.
docs(mcp): correct MCPServerPlugin's docblock to the canonical stdio switch - #15067
Merged
Conversation
…switch The class docblock still taught the pre-split trigger: step 2 said the long-lived stdio transport starts "only when `autoStart` is enabled or `OS_MCP_SERVER_ENABLED` is explicitly `true`", and the Environment Variables block said explicit `true` "additionally auto-starts the stdio transport". Neither named `OS_MCP_STDIO_ENABLED` — the canonical switch `resolveMcpStdioAutoStart()` reads first — about 100 lines above a runtime warning telling the operator that exact trigger is DEPRECATED. Comments only; no behaviour change. The deprecation sentence is copied verbatim from the runtime warning below it so the two cannot drift into two phrasings of the same rule. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
The docblock is a published surface, measured rather than assumed: `MCPServerPlugin` is exported from the package entry, root tsup config has `dts` emit on, and packages/mcp's `files` ships `dist` — so the comment reaches consumers as `dist/index.d.ts` and renders in editor IntelliSense. That makes it user-visible, so a changeset is owed and `skip-changeset` does not apply. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
Contributor
📓 Docs Drift Check
What this run could not see
Coarse fallback — 12 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): |
os-trump
marked this pull request as ready for review
September 3, 2026 19:57
This was referenced Sep 3, 2026
os-trump
enabled auto-merge
September 3, 2026 20:00
Uh oh!
There was an error while loading. Please reload this page.
os-trump pushed a commit
that referenced
this pull request
Sep 3, 2026
…arity Brings in PR #15062 (`os i18n check --help`), which lands in packages/cli while this branch is open — the dispatch named it as the one to merge and re-verify against. Also #15065 (rest), #15063/#15067 (docs) and #15055 (spec). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#14473
MCPServerPlugin's own class docblock still taught the pre-split stdiotrigger, about 100 lines above a runtime warning saying the opposite.
Step 2 said the long-lived transport starts "only when
autoStartis enabled orOS_MCP_SERVER_ENABLEDis explicitlytrue", and the Environment Variablesblock said explicit
true"additionally auto-starts the stdio transport".Neither named
OS_MCP_STDIO_ENABLEDanywhere.Read against the authority —
resolveMcpStdioAutoStart()inpackages/types/src/env.ts— that is backwards. The resolver readsOS_MCP_STDIO_ENABLEDfirst and returns it clean;OS_MCP_SERVER_ENABLED=truefalls through to a legacy branch that returns
viaDeprecatedAlias: true, andstart()then warns the operator that this exact trigger is DEPRECATED. So thedocblock presented the deprecated trigger as the design and never named the
canonical one. An author following it got a working transport plus a
deprecation warning at every boot, with no way from this file to learn the
right spelling.
A comment card with a demonstrated downstream
This is not tidiness. The published
skills/objectstack-aiMCP section waswritten from this docblock and inherited the same error — caught in contract
review and corrected by PR #14463 (
5dabfd2a). This file is the upstream of thatdefect and was still uncorrected. One catch happened to be in review; the next
may not be.
The change
packages/mcp/src/plugin.ts, comments only, no behaviour change:autoStartis enabled orOS_MCP_STDIO_ENABLEDis truthy";OS_MCP_SERVER_ENABLEDis described as thedefault-on HTTP gate only;
OS_MCP_STDIO_ENABLEDis added as the stdiotransport's own switch (default OFF); and the legacy trigger is marked
deprecated in the runtime warning's own words, copied verbatim from the
ctx.logger.warnbelow it rather than paraphrased, so the two cannot driftinto two phrasings of one rule.
Both blocks now point at the two decision points by name (
isMcpServerEnabled,resolveMcpStdioAutoStart) so the next reader lands on the authority instead ofon prose about it.
The blast radius stayed at one file, re-measured
Triage measured that every neighbour was already updated when the split landed.
I re-ran that independently rather than taking it on trust, with a positive
control first:
Positive control: the same pathspec and flags for
OS_MCP_STDIO_ENABLEDreturnsmany hits and exit 0, so the pathspec and cwd are sound and a zero would have
meant something. The result confirms triage —
packages/mcp/README.md:79,:86-88scopes the var to HTTP and carries the deprecation,
content/docs/deployment/environment-variables.mdx:257,:264-268gates it to HTTPwith the deprecation spelled out, and
skills/objectstack-ai/SKILL.md:188-190isthe corrected text that PR #14463 landed. No neighbour still teaches the deprecated
trigger, so nothing rode along with this fix.
Changeset: owed, and measured rather than assumed
A comment-only diff can qualify for
skip-changeset, but only if it publishesnothing. Measured from
packages/mcp's own manifest, it publishes:filesis["dist", "README.md", "CHANGELOG.md"]— sodistships;tsup.config.tssetsdts: !process.env.OS_SKIP_DTS, so declarationemit is on;
packages/mcp/src/index.ts:13exportsMCPServerPlugin, so this docblock sitson the package entry;
fixture through this repo's own toolchain, where a sentinel comment on an
exported class came through into the emitted
index.d.tsunchanged.So the corrected text reaches consumers as
dist/index.d.tsand renders ineditor IntelliSense. That is user-visible, so a changeset is included and
skip-changesetdoes not apply here.Could a gate have caught this drift?
Asked deliberately, and not built in this PR. None of the 204 discovered
check families does this today: the nearest neighbours grade docs pages against
source paths or the mechanics of comment masking, none reads a docblock's claims
against the code it documents.
One could, and fairly narrowly, because this repo already gives it an anchor: env
switches have a single decision point per switch in
packages/types/src/env.ts,and the deprecated-alias relationship is structural rather than a wordlist. A
gate could key on the resolver's own shape — the var read in the branch returning
viaDeprecatedAlias: trueis the legacy spelling, the var read in the cleanbranch is the canonical one — and then require that any JSDoc block naming the
legacy var also names the canonical var in the same block. That is exactly the
condition this docblock violated, it needs no hand-maintained list of variable
pairs, and it would go red the next time a split lands and one comment is missed.
Verification
Gate union derived mechanically at the final HEAD
9544e659bbon a clean treevia
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, notfrom a hand-written path list: 33 families (25 by path plus the 8 that only
apply once a changeset exists). Re-run in full at that same commit:
a pass and never as a red:
check:dual-build-cjs-loadsandcheck-test-completenessboth require a whole-repopnpm build/ a real testrun log, and
scripts/pm/check-half-states.mjsneeds GitHub API access thisseat's session gate refuses.
Repo-wide ESLint was run in full rather than narrowed —
pnpm lint(
eslint . --no-inline-config), exit 0 in 2m13s over the whole tree.Declared narrowing:
pnpm buildandpnpm --filter @objectstack/mcp typecheckwere not measured. Three attempts on the shared verification lockeach returned exit 99 (queue-timeout, never acquired, 9 minutes each, about 27
minutes total) behind sibling seats' long-running builds. The derived union for
this card names no build and no test family, and the diff is comments only, so
nothing here is claimed green on that basis — CI runs the farm regardless.
🤖 Generated with Claude Code
https://claude.ai/code/session_016yfqQh2dBgPAymYd7xipza
Generated by Claude Code