History

Revert the outbound MCP SDK v2 migration (#1697)

* Revert the outbound MCP SDK v2 migration * Drop the v2 MCP packages from the lockfile

Rhys Sullivan committed 2 months ago

Commit a5800a11 parent 54cde5e91 files · +5838 −10032Browse files

Showing the changes to scripts (2 of 91 files). Show all

Unified · Split

2 files
  1. scripts/bootstrap.ts +5 −1
  2. scripts/check-patched-deps.ts +135 −10
scripts/bootstrap.ts+5−1
@@ -23,7 +23,11 @@ const run = (label: string, cmd: string, args: ReadonlyArray<string>) => { · 22 unmodified lines
23// apps' vite dev servers fail without in a fresh worktree.23// apps' vite dev servers fail without in a fresh worktree.
24run("dependencies (+ prepare builds)", "bun", ["install"]);24run("dependencies (+ prepare builds)", "bun", ["install"]);
25 25
26// Catch patched-dependency entries whose checked-in patch file was removed.26// Assert load-bearing patched dependencies actually installed in patched form.
27// A bun cache edge case can leave a stale, unpatched dist in node_modules even
28// though the lockfile records the patch (bun reports "no changes"). That would
29// silently drop the agents MCP transport hang fix; fail here so a fresh
30// checkout or worktree surfaces it immediately instead of at deploy time.
27run("verify patched deps", "bun", ["run", "scripts/check-patched-deps.ts"]);31run("verify patched deps", "bun", ["run", "scripts/check-patched-deps.ts"]);
28 32
29// e2e browser scenarios need Playwright's chromium; the cache is shared33// e2e browser scenarios need Playwright's chromium; the cache is shared
scripts/check-patched-deps.ts+135−10
@@ -1,5 +1,35 @@
1#!/usr/bin/env bun1#!/usr/bin/env bun
2/** Verify that every root patched-dependency entry still names a real file. */2/**
3 * Asserts that patched dependencies are actually installed in patched form.
4 *
5 * We patch several npm packages via `bun patch` (see `patchedDependencies` in
6 * package.json and the `patches/*.patch` files). At least one of these patches
7 * is load-bearing at runtime, not just a build-time convenience: the
8 * `agents@0.17.3` patch carries the MCP transport persist/replay fix (the SSE
9 * "hang" fix that preserves tool results across dropped connections). If the
10 * installed `agents` dist is the STALE, unpatched upstream build, everything
11 * still compiles and tests mostly pass while the deployed transport silently
12 * lacks the fix, so prod ships the pre-fix transport with zero signal.
13 *
14 * This has actually happened: a bun cache edge case left a checkout with an
15 * unpatched `agents` dist in node_modules even though the lockfile and
16 * package.json recorded the patch. `bun install` reported no changes because
17 * the store entry it wanted was already present; it just wasn't the patched
18 * content.
19 *
20 * So we don't trust the lockfile — we read the installed dist off disk and
21 * assert the post-patch sentinel strings are present. The check is a couple of
22 * file reads plus greps, so it adds ~zero wall-clock time and is safe to run in
23 * CI before the test job and in `bootstrap` on every fresh checkout.
24 *
25 * Usage:
26 * bun run scripts/check-patched-deps.ts
27 *
28 * Env overrides (used by the self-test):
29 * CHECK_PATCHED_DEPS_AGENTS_MCP=/abs/path/to/agents/dist/mcp/index.js
30 * Force the agents MCP entry path instead of resolving it, so the self-test
31 * can point the check at a deliberately-corrupted copy.
32 */
3import { existsSync, readFileSync } from "node:fs";33import { existsSync, readFileSync } from "node:fs";
4import { resolve } from "node:path";34import { resolve } from "node:path";
5 35
@@ -8,26 +38,121 @@ const repoRoot = resolve(import.meta.dir, ".."); · 2 unmodified lines
8type Failure = { readonly package: string; readonly detail: string };38type Failure = { readonly package: string; readonly detail: string };
9const failures: Failure[] = [];39const failures: Failure[] = [];
10 40
41/**
42 * Each patched package whose *runtime content* we assert against. Sentinels are
43 * identifiers that exist only in the patched dist (added by the patch), so
44 * their presence proves the installed file is the patched one, not a stale
45 * upstream build. Keep sentinels to a couple of stable, patch-unique symbols.
46 */
47type RuntimeCheck = {
48 readonly package: string;
49 /** Resolve the installed entry file we assert against. */
50 readonly resolveEntry: () => string;
51 /** Strings that only appear post-patch. All must be present. */
52 readonly sentinels: readonly string[];
53 /** Human hint for what the patch does, shown on failure. */
54 readonly purpose: string;
55};
56
57/**
58 * Resolve a subpath export of a dependency robustly, without hardcoding the
59 * bun store hash in `node_modules/.bun/<pkg>@<version>+<hash>/…`. We resolve
60 * from a workspace package that actually depends on it so the module graph is
61 * the real one; `require.resolve` then walks bun's store for us.
62 */
63const resolveFrom = (specifier: string, fromPackageDir: string): string => {
64 const fromDir = resolve(repoRoot, fromPackageDir);
65 return require.resolve(specifier, { paths: [fromDir] });
66};
67
68const runtimeChecks: readonly RuntimeCheck[] = [
69 {
70 package: "agents@0.17.3 (agents/mcp)",
71 purpose:
72 "MCP transport persist/replay fix (preserves tool results across dropped SSE connections)",
73 resolveEntry: () => {
74 const override = process.env.CHECK_PATCHED_DEPS_AGENTS_MCP;
75 if (override && override.length > 0) return resolve(override);
76 // `packages/hosts/cloudflare` is the workspace package that depends on
77 // `agents`, so resolve the `agents/mcp` export from there.
78 return resolveFrom("agents/mcp", "packages/hosts/cloudflare");
79 },
80 // These identifiers are introduced by patches/agents@0.17.3.patch and do
81 // not exist in the upstream 0.17.3 dist.
82 sentinels: ["markStreamUndelivered", "replayUndeliveredResponses"],
83 },
84];
85
86for (const check of runtimeChecks) {
87 let entry: string;
88 try {
89 entry = check.resolveEntry();
90 } catch (err) {
91 failures.push({
92 package: check.package,
93 detail: `could not resolve installed entry: ${(err as Error).message.split("\n")[0]}`,
94 });
95 continue;
96 }
97
98 if (!existsSync(entry)) {
99 failures.push({ package: check.package, detail: `installed entry does not exist: ${entry}` });
100 continue;
101 }
102
103 const contents = readFileSync(entry, "utf8");
104 const missing = check.sentinels.filter((s) => !contents.includes(s));
105 if (missing.length > 0) {
106 failures.push({
107 package: check.package,
108 detail:
109 `installed dist is missing post-patch sentinel(s): ${missing.join(", ")}\n` +
110 ` entry: ${entry}\n` +
111 ` patch purpose: ${check.purpose}`,
112 });
113 }
114}
115
116/**
117 * Lightweight generalized layer: every package listed in `patchedDependencies`
118 * must still have its referenced patch file on disk. This does not verify the
119 * installed *content* for packages without a dedicated runtime check above
120 * (that requires per-package sentinels), but it catches a patch entry pointing
121 * at a missing file, which would make `bun install` silently skip patching.
122 */
11const rootPkg = JSON.parse(readFileSync(resolve(repoRoot, "package.json"), "utf8")) as {123const rootPkg = JSON.parse(readFileSync(resolve(repoRoot, "package.json"), "utf8")) as {
12 patchedDependencies?: Record<string, string>;124 patchedDependencies?: Record<string, string>;
13};125};
14for (const [dependency, patchPath] of Object.entries(rootPkg.patchedDependencies ?? {})) {126for (const [dep, patchPath] of Object.entries(rootPkg.patchedDependencies ?? {})) {
15 if (existsSync(resolve(repoRoot, patchPath))) continue;127 const abs = resolve(repoRoot, patchPath);
16 failures.push({128 if (!existsSync(abs)) {
17 package: dependency,129 failures.push({
18 detail: `patchedDependencies references a missing patch file: ${patchPath}`,130 package: dep,
19 });131 detail: `patchedDependencies references a missing patch file: ${patchPath}`,
132 });
133 }
20}134}
21 135
22if (failures.length > 0) {136if (failures.length > 0) {
23 const lines = failures.map((failure) => ` - ${failure.package}: ${failure.detail}`).join("\n");137 const lines = failures.map((f) => ` - ${f.package}: ${f.detail}`).join("\n");
24 console.error(138 console.error(
25 `\nPatched-dependency check FAILED (${failures.length} problem(s)):\n${lines}\n\n` +139 `\nPatched-dependency check FAILED (${failures.length} problem(s)):\n${lines}\n\n` +
26 "Every patchedDependencies entry must reference a checked-in patch file.\n",140 "Cause: the installed dependency content does not match the patch we ship.\n" +
141 "This is usually a stale bun store entry: bun kept an unpatched build of the\n" +
142 "package in its cache and `bun install` reported no changes without applying\n" +
143 "the patch. The deployed/tested code then silently lacks the patched behavior\n" +
144 "(for `agents`, the MCP transport hang fix), with no other signal.\n\n" +
145 "Fix: force a clean reinstall of the affected package's store entry, e.g.\n" +
146 " rm -rf node_modules/.bun/agents@* node_modules/agents\n" +
147 " bun install\n" +
148 "If that does not take, clear bun's global cache for it:\n" +
149 " bun pm cache rm\n" +
150 " bun install\n",
27 );151 );
28 process.exit(1);152 process.exit(1);
29}153}
30 154
31console.log(155console.log(
32 `Patched-dependency check passed: ${Object.keys(rootPkg.patchedDependencies ?? {}).length} patch file(s) present.`,156 `Patched-dependency check passed: ${runtimeChecks.length} runtime sentinel check(s), ` +
157 `${Object.keys(rootPkg.patchedDependencies ?? {}).length} patch file(s) present.`,
33);158);