test: pin schema boundaries, error-code tables, package topology, and the stdio env safelist

docs/behavior-surface-pins.md

@@ -0,0 +1,49 @@
+# Behavior-surface pins
+
+Some tests in this repo are **pins**: they assert the exact current value of a
+wire- or consumer-visible behavior — an error code, a schema boundary, an
+export map, the stdio env safelist — rather than checking that a feature
+works. Their job is to distinguish a deliberate surface change from an
+accidental one: the regular suite stays green through either; a pin goes red
+through both.
+
+## When a pin goes red on your change
+
+A red pin does **not** mean the change is forbidden. It means the change is
+surface-visible and must be deliberate:
+
+1. Confirm the change is intended. If it isn't, the pin just caught an
+   accidental break.
+2. Update the pin in the same PR.
+3. Add a changeset if the surface is consumer-facing.
+4. Update `docs/migration.md` / `docs/migration-SKILL.md` where consumer-facing.
+
+Never weaken a pin (loosen an exact match, delete an assertion) just to make
+CI pass — that reopens the silent-drift hole the pin exists to close.
+
+## Where pins live
+
+| Surface | File |
+| --- | --- |
+| Wire error-code tables, error classes, version constants | `packages/core/test/types/errorSurfacePins.test.ts` |
+| Schema strict/strip/loose boundaries, key existence | `packages/core/test/types/schemaBoundaryPins.test.ts` |
+| Published package set, export maps, ESM-only topology | `packages/core/test/packageTopologyPins.test.ts` |
+| stdio environment-inheritance safelist | `packages/client/test/client/stdioEnvPins.test.ts` |
+
+## Writing a new pin
+
+- The expectation side must be a literal frozen in the test, never a value
+  imported from src. Comparing a source constant against itself pins nothing.
+- Mutation-check it once before landing: flip the source behavior locally and
+  confirm the pin actually goes red. A pin that stays green under the drift it
+  claims to guard is worse than no pin.
+- Pin behavior a deployed peer or consumer can observe. Internal details that
+  are invisible across the wire and the public API don't need pins.
+- Don't pin a known bug to make it load-bearing — file an issue instead.
+
+## History
+
+The original, much broader inventory was developed against v1.x in #2258 and
+#2262 (closed unmerged). This sweep ports only the boundary surfaces above;
+see those PRs for the fuller exploration and the reasoning behind what was
+left out.

packages/client/test/client/stdioEnvPins.test.ts

@@ -0,0 +1,69 @@
+/**
+ * Behavior-surface pins: the stdio environment-inheritance safelist.
+ *
+ * getDefaultEnvironment() decides which parent environment variables every
+ * spawned stdio server inherits. Widening the safelist leaks more of the
+ * parent environment into child processes, so both the list itself and the
+ * filtering behavior are pinned. A failing pin here means the change is
+ * deliberate: update the pin in the same change, together with a changeset
+ * and a migration-doc entry.
+ *
+ * See docs/behavior-surface-pins.md for the maintenance protocol.
+ */
+import { afterEach, describe, expect, test, vi } from 'vitest';
+
+import { DEFAULT_INHERITED_ENV_VARS, getDefaultEnvironment } from '../../src/client/stdio.js';
+
+// Frozen copy of the documented safelist. The expectation side is a literal,
+// not derived from src, so any edit to DEFAULT_INHERITED_ENV_VARS goes red
+// here regardless of which variables happen to be set in the runner's
+// environment. (The behavioral test below cannot catch a widened safelist on
+// its own: getDefaultEnvironment skips unset keys, and sensitive variables
+// are exactly the ones typically unset in CI.)
+const SAFELIST =
+    process.platform === 'win32'
+        ? [
+              'APPDATA',
+              'HOMEDRIVE',
+              'HOMEPATH',
+              'LOCALAPPDATA',
+              'PATH',
+              'PROCESSOR_ARCHITECTURE',
+              'SYSTEMDRIVE',
+              'SYSTEMROOT',
+              'TEMP',
+              'USERNAME',
+              'USERPROFILE',
+              'PROGRAMFILES'
+          ]
+        : ['HOME', 'LOGNAME', 'PATH', 'SHELL', 'TERM', 'USER'];
+
+describe('stdio environment safelist', () => {
+    afterEach(() => {
+        vi.unstubAllEnvs();
+    });
+
+    test('DEFAULT_INHERITED_ENV_VARS matches the frozen safelist exactly', () => {
+        expect([...DEFAULT_INHERITED_ENV_VARS].sort()).toEqual([...SAFELIST].sort());
+    });
+
+    test('getDefaultEnvironment inherits exactly the safelist keys that are set', () => {
+        for (const key of SAFELIST) {
+            vi.stubEnv(key, `safe-${key}`);
+        }
+        vi.stubEnv('STDIO_PIN_SECRET', 'must-not-be-inherited');
+
+        const env = getDefaultEnvironment();
+
+        expect(Object.keys(env).sort()).toEqual([...SAFELIST].sort());
+        for (const key of SAFELIST) {
+            expect(env[key]).toBe(`safe-${key}`);
+        }
+    });
+
+    test('skips values that look like exported shell functions', () => {
+        vi.stubEnv('PATH', '() { echo pwned; }');
+        const env = getDefaultEnvironment();
+        expect(env.PATH).toBeUndefined();
+    });
+});

packages/core/test/packageTopologyPins.test.ts

@@ -0,0 +1,146 @@
+/**
+ * Behavior-surface pins: workspace package topology and export maps.
+ *
+ * The published surface of the SDK is the set of public packages and their
+ * export-map entries. Consumers resolve deep subpaths through these maps, so
+ * adding, removing, or renaming an entry — or flipping a private flag — is a
+ * consumer-visible change. This pins the manifest-level topology: every change
+ * to it must be deliberate (update the pin, add a changeset, and document the
+ * migration). Runtime resolvability of the built entries is covered by the
+ * integration test workspace.
+ *
+ * See docs/behavior-surface-pins.md for the maintenance protocol.
+ */
+import { existsSync, readdirSync, readFileSync } from 'node:fs';
+import { dirname, join } from 'node:path';
+import { fileURLToPath } from 'node:url';
+
+import { describe, expect, test } from 'vitest';
+
+const packagesDir = join(dirname(fileURLToPath(import.meta.url)), '..', '..');
+
+interface PackageManifest {
+    name: string;
+    private?: boolean;
+    type?: string;
+    files?: string[];
+    bin?: Record<string, string>;
+    exports?: Record<string, unknown>;
+}
+
+function readManifest(relativeDir: string): PackageManifest {
+    return JSON.parse(readFileSync(join(packagesDir, relativeDir, 'package.json'), 'utf8')) as PackageManifest;
+}
+
+/** dir (relative to packages/) → expected manifest shape */
+const PUBLIC_PACKAGES: Record<string, { name: string; exportKeys: string[]; bin?: Record<string, string> }> = {
+    client: {
+        name: '@modelcontextprotocol/client',
+        exportKeys: ['.', './stdio', './validators/ajv', './validators/cf-worker', './_shims']
+    },
+    server: {
+        name: '@modelcontextprotocol/server',
+        exportKeys: ['.', './stdio', './validators/ajv', './validators/cf-worker', './_shims']
+    },
+    'server-legacy': {
+        name: '@modelcontextprotocol/server-legacy',
+        exportKeys: ['.', './sse', './auth']
+    },
+    'middleware/express': { name: '@modelcontextprotocol/express', exportKeys: ['.'] },
+    'middleware/fastify': { name: '@modelcontextprotocol/fastify', exportKeys: ['.'] },
+    'middleware/hono': { name: '@modelcontextprotocol/hono', exportKeys: ['.'] },
+    'middleware/node': { name: '@modelcontextprotocol/node', exportKeys: ['.'] },
+    codemod: {
+        name: '@modelcontextprotocol/codemod',
+        exportKeys: ['.'],
+        bin: { 'mcp-codemod': './dist/cli.mjs' }
+    }
+};
+
+describe('public package topology', () => {
+    for (const [dir, expected] of Object.entries(PUBLIC_PACKAGES)) {
+        describe(expected.name, () => {
+            const manifest = readManifest(dir);
+
+            test('is published under the pinned name', () => {
+                expect(manifest.name).toBe(expected.name);
+                expect(manifest.private).not.toBe(true);
+            });
+
+            test('export-map keys are pinned exactly', () => {
+                expect(Object.keys(manifest.exports ?? {})).toEqual(expected.exportKeys);
+            });
+
+            test('ships ESM only', () => {
+                expect(manifest.type).toBe('module');
+                // No entry may grow a 'require' condition: the v2 packages are
+                // ESM-only by design (a CJS build would be a new public surface).
+                const conditionsOf = (entry: unknown): string[] =>
+                    entry !== null && typeof entry === 'object'
+                        ? Object.entries(entry).flatMap(([key, value]) => [key, ...conditionsOf(value)])
+                        : [];
+                for (const entry of Object.values(manifest.exports ?? {})) {
+                    expect(conditionsOf(entry)).not.toContain('require');
+                }
+            });
+
+            test('publishes only dist', () => {
+                expect(manifest.files).toEqual(['dist']);
+            });
+
+            if (expected.bin) {
+                test('bin entries are pinned', () => {
+                    expect(manifest.bin).toEqual(expected.bin);
+                });
+            } else {
+                test('declares no bin entries', () => {
+                    expect(manifest.bin).toBeUndefined();
+                });
+            }
+        });
+    }
+});
+
+describe('the package set itself is pinned', () => {
+    /** Every directory under packages/ (one level, plus middleware/*) holding a package.json. */
+    function discoverManifestDirs(): string[] {
+        const dirs: string[] = [];
+        for (const entry of readdirSync(packagesDir, { withFileTypes: true })) {
+            if (!entry.isDirectory()) {
+                continue;
+            }
+            if (existsSync(join(packagesDir, entry.name, 'package.json'))) {
+                dirs.push(entry.name);
+                continue;
+            }
+            for (const nested of readdirSync(join(packagesDir, entry.name), { withFileTypes: true })) {
+                if (nested.isDirectory() && existsSync(join(packagesDir, entry.name, nested.name, 'package.json'))) {
+                    dirs.push(`${entry.name}/${nested.name}`);
+                }
+            }
+        }
+        return dirs.sort();
+    }
+
+    test('every manifest under packages/ is either a pinned public package or core', () => {
+        // The workspace glob (packages/**/*) auto-adopts any new directory and
+        // the changesets config publishes every non-private package, so the SET
+        // of packages is itself published surface. A new package must be added
+        // to PUBLIC_PACKAGES here deliberately (or pinned as private below) —
+        // otherwise it would ship to npm without any pin applying to it.
+        expect(discoverManifestDirs()).toEqual([...Object.keys(PUBLIC_PACKAGES), 'core'].sort());
+    });
+});
+
+describe('internal packages stay private', () => {
+    test('@modelcontextprotocol/core is private (bundled into client/server dists)', () => {
+        const manifest = readManifest('core');
+        expect(manifest.name).toBe('@modelcontextprotocol/core');
+        expect(manifest.private).toBe(true);
+    });
+
+    test('the workspace root is private', () => {
+        const manifest = JSON.parse(readFileSync(join(packagesDir, '..', 'package.json'), 'utf8')) as PackageManifest;
+        expect(manifest.private).toBe(true);
+    });
+});