History

Stop SDK chunks from importing private @executor-js/api (#755)

* Stop SDK chunks from importing private @executor-js/api Each plugin's `src/sdk/plugin.ts` was reaching into `../api/group` and `../api/handlers` to fill the optional `routes`/`handlers`/ `extensionService` fields. tsup followed those imports into the published SDK chunk, which carried `import { InternalError } from "@executor-js/api"` and `import { addGroup, capture } from "@executor-js/api"`. Because `@executor-js/api` is `private: true`, plain ESM consumers hit `Cannot find package '@executor-js/api'` on `import("@executor-js/plugin-mcp/core")` (and graphql / openapi). Changes: - Move `InternalError` (the wire-level 500 schema) to `@executor-js/sdk/core`. `@executor-js/api/observability.ts` re-exports it for back-compat — server code is unaffected. - Each plugin's `src/api/group.ts` now imports `InternalError` from `@executor-js/sdk/core` instead of `@executor-js/api`. - Each plugin's `src/sdk/plugin.ts` no longer imports `../api/*` and no longer sets `routes`/`handlers`/`extensionService` on the returned plugin. - New `*HttpPlugin` factories on the `/api` subpath wrap the SDK factory and add the HTTP fields. Hosts that mount HTTP routes switch to the `/api` subpath and the new factory name (`mcpHttpPlugin`, `graphqlHttpPlugin`, `openApiHttpPlugin`, `onepasswordHttpPlugin`, `googleDiscoveryHttpPlugin`). - Apps cloud / local / marketing updated to use the `*HttpPlugin` factories from `/api`. - `@executor-js/api` demoted from `dependencies` to devDep + optional peerDep on `plugin-onepassword` and `plugin-google-discovery`, matching the other plugins. * Add pack-and-import smoke test for public library packages Catches the regression class where a published `@executor-js/*` bundle transitively imports a private workspace package (e.g. the `@executor-js/api` leak this branch fixes). Runs as a release-gate step before `release:publish:packages`, so a broken bundle blocks publish. How it works: - Reuses `publish-packages.ts --dry-run` to pack every public package with `publishConfig.exports` and `workspace:*` rewrites applied. - For each tarball, installs into a fresh temp dir with npm `overrides` pointing every other `@executor-js/*` transitive dep at its local tarball — without that, npm pulls the currently-published registry version of `@executor-js/sdk` etc. and masks any unreleased-symbol drift. - Reads the installed manifest (post-`publishConfig`) and dynamically `import()`s every subpath in its `exports` map. - Hard-fails on any `Cannot find package '@executor-js/*'`. Other resolution failures (missing `react`, `effect`, `@effect/vitest`, etc.) are logged as `skip` since they reflect a missing peer in the smoke env, not a bug in the bundle. `publish-packages.ts` reordered so pack always happens (was skipped when the version was already on npm). The "already published" check now only short-circuits the `npm publish` step, so smoke tests and pkg-pr-new previews always have a fresh tarball to work with. Wired up: - `bun run release:smoke:packages` — invoke locally. - `release.yml` runs it before `release:publish:packages`. Reverse-verified by temporarily restoring the pre-fix mcp files — script reports `FAIL @executor-js/plugin-mcp — references private '@executor-js/api'` and exits 1.

Rhys Sullivan committed 6 months ago

Commit d0cf70c1 parent 93ef46e29 files · +477 −91Browse files

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

Unified · Split

2 files
  1. scripts/publish-packages.ts +10 −6
  2. scripts/smoke-test-packed.ts +234 −0
scripts/publish-packages.ts+10−6
@@ -287,12 +287,7 @@ const publishPackage = async ( · 286 unmodified lines
287287 throw new Error(`Missing dist/ in ${pkgDir}. Did you run 'bun run build:packages'?`);
288288 }
289289
290− if (await packageAlreadyPublished(name, version)) {
291− console.log(`[skip] ${name}@${version} already on npm`);
292− return;
293− }
294−
295− console.log(`[publish] ${name}@${version} (${channel})${dryRun ? " [dry-run]" : ""}`);
290+ console.log(`[pack] ${name}@${version} (${channel})${dryRun ? " [dry-run]" : ""}`);
296291
297292 // Clean any stale tarballs from previous runs so our readdir finds exactly
298293 // the archive produced by the pack below.
@@ -326,6 +321,15 @@ const publishPackage = async ( · 27 unmodified lines
326321 return;
327322 }
328323
324+ // Skip publishing already-shipped versions. The pack still ran above so
325+ // smoke tests / pkg-pr-new previews always have a fresh tarball.
326+ if (await packageAlreadyPublished(name, version)) {
327+ console.log(`[skip] ${name}@${version} already on npm`);
328+ return;
329+ }
330+
331+ console.log(`[publish] ${name}@${version} (${channel})`);
332+
329333 const args = ["publish", tarball, "--access", "public", "--tag", channel];
330334 if (process.env.GITHUB_ACTIONS === "true") {
331335 args.push("--provenance");
scripts/smoke-test-packed.ts+234−0
@@ -0,0 +1,234 @@
1+#!/usr/bin/env bun
2+/**
3+ * Pack-and-import smoke test for the public `@executor-js/*` packages.
4+ *
5+ * Reproduces what an external npm consumer experiences:
6+ *
7+ * 1. Pack each publishable workspace via `publish-packages.ts
8+ * --dry-run`, so `publishConfig.exports` and `workspace:*` rewrites
9+ * are applied to the tarball.
10+ * 2. For each package, install the tarball into a fresh temp dir
11+ * with npm `overrides` pointing every other `@executor-js/*`
12+ * transitive dep at its local tarball — otherwise npm pulls the
13+ * currently-published version from the registry, which masks any
14+ * internal-API mismatch this branch introduced.
15+ * 3. Read the installed package.json (the post-`publishConfig` view)
16+ * and dynamically `import()` every subpath in its `exports` map.
17+ *
18+ * Failures for `@executor-js/*` package not-found are hard failures —
19+ * that's the regression class where private workspace packages leak
20+ * into a public bundle. Failures for other peers (`react`, `effect`,
21+ * `@tanstack/*`, etc.) are downgraded to warnings, since they reflect
22+ * a missing peer in the smoke environment, not a bug in the bundle.
23+ *
24+ * Invoke via `bun run release:smoke:packages`.
25+ */
26+import { $ } from "bun";
27+import { existsSync, readdirSync } from "node:fs";
28+import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises";
29+import { tmpdir } from "node:os";
30+import { dirname, join, resolve } from "node:path";
31+import { fileURLToPath } from "node:url";
32+
33+const repoRoot = resolve(dirname(fileURLToPath(import.meta.url)), "..");
34+
35+const PUBLIC_PACKAGE_DIRS = [
36+ "packages/core/storage-core",
37+ "packages/kernel/core",
38+ "packages/kernel/runtime-quickjs",
39+ "packages/core/sdk",
40+ "packages/core/config",
41+ "packages/core/execution",
42+ "packages/core/cli",
43+ "packages/plugins/example",
44+ "packages/plugins/file-secrets",
45+ "packages/plugins/google-discovery",
46+ "packages/plugins/graphql",
47+ "packages/plugins/keychain",
48+ "packages/plugins/mcp",
49+ "packages/plugins/onepassword",
50+ "packages/plugins/openapi",
51+] as const;
52+
53+type PackageJson = {
54+ name: string;
55+ version: string;
56+ exports?: Record<string, unknown>;
57+};
58+
59+const readPackageJson = async (pkgDir: string): Promise<PackageJson> => {
60+ const raw = await readFile(join(pkgDir, "package.json"), "utf8");
61+ return JSON.parse(raw) as PackageJson;
62+};
63+
64+const findTarball = (pkgDir: string): string | null => {
65+ const tgz = readdirSync(pkgDir).find((entry) => entry.endsWith(".tgz"));
66+ return tgz ? join(pkgDir, tgz) : null;
67+};
68+
69+const subpathsToTest = (pkg: PackageJson): readonly string[] => Object.keys(pkg.exports ?? {});
70+
71+const importSpecifier = (pkgName: string, subpath: string): string =>
72+ subpath === "." ? pkgName : `${pkgName}${subpath.slice(1)}`;
73+
74+type SmokeFailure = {
75+ readonly pkg: string;
76+ readonly subpath: string;
77+ readonly reason: string;
78+};
79+
80+const PRIVATE_PACKAGE_RE = /Cannot find package '(@executor-js\/[^']+)'/;
81+
82+const firstMeaningfulLine = (stderr: string): string => {
83+ const lines = stderr
84+ .split("\n")
85+ .map((line) => line.trim())
86+ .filter(Boolean);
87+ const errorLine = lines.find((line) => /^(\w*Error|Cannot find)/i.test(line));
88+ if (errorLine) return errorLine;
89+ for (const line of lines) {
90+ if (line.startsWith("node:")) continue;
91+ if (line.startsWith("file://")) continue;
92+ if (line.startsWith("at ")) continue;
93+ if (line.startsWith("import ")) continue;
94+ if (line.startsWith("throw ")) continue;
95+ if (line === "^" || /^\^+$/.test(line)) continue;
96+ return line;
97+ }
98+ return lines[0] ?? "(no stderr)";
99+};
100+
101+type Tarballs = ReadonlyMap<string, string>;
102+
103+const smokeTestPackage = async (
104+ pkgDir: string,
105+ tarballs: Tarballs,
106+ failures: SmokeFailure[],
107+): Promise<void> => {
108+ const pkg = await readPackageJson(pkgDir);
109+ const tarballPath = tarballs.get(pkg.name);
110+ if (!tarballPath) {
111+ failures.push({ pkg: pkg.name, subpath: "", reason: "no tarball produced" });
112+ return;
113+ }
114+
115+ const tmp = await mkdtemp(join(tmpdir(), "executor-smoke-"));
116+ try {
117+ // npm `overrides` forces transitive `@executor-js/*` deps to resolve
118+ // to their local tarball instead of whatever's published on npm.
119+ // Without this, a plugin built against an unreleased symbol in
120+ // `@executor-js/sdk` would silently install the published `sdk`
121+ // and fail at import — a real bug, but not the bundle bug we're
122+ // here to catch.
123+ const overrides: Record<string, string> = {};
124+ for (const [name, path] of tarballs) {
125+ overrides[name] = `file:${path}`;
126+ }
127+ const fixture = {
128+ name: "executor-smoke-fixture",
129+ version: "0.0.0",
130+ private: true,
131+ type: "module",
132+ dependencies: { [pkg.name]: `file:${tarballPath}` },
133+ overrides,
134+ };
135+ await writeFile(join(tmp, "package.json"), `${JSON.stringify(fixture, null, 2)}\n`);
136+
137+ const install = await $`npm install --no-audit --no-fund --legacy-peer-deps`
138+ .cwd(tmp)
139+ .quiet()
140+ .nothrow();
141+ if (install.exitCode !== 0) {
142+ failures.push({
143+ pkg: pkg.name,
144+ subpath: "<install>",
145+ reason: install.stderr.toString().trim().split("\n").slice(-3).join("\n"),
146+ });
147+ return;
148+ }
149+
150+ // Read the installed manifest — that's the real published view
151+ // (publishConfig.exports applied, workspace specifiers resolved).
152+ const installedPkg = await readPackageJson(join(tmp, "node_modules", ...pkg.name.split("/")));
153+ const subpaths = subpathsToTest(installedPkg);
154+ if (subpaths.length === 0) {
155+ failures.push({
156+ pkg: pkg.name,
157+ subpath: "",
158+ reason: "no exports declared in published manifest",
159+ });
160+ return;
161+ }
162+
163+ for (const subpath of subpaths) {
164+ const spec = importSpecifier(pkg.name, subpath);
165+ const probe =
166+ await $`node --input-type=module --eval ${`await import(${JSON.stringify(spec)});`}`
167+ .cwd(tmp)
168+ .quiet()
169+ .nothrow();
170+ if (probe.exitCode === 0) {
171+ console.log(` ok ${spec}`);
172+ continue;
173+ }
174+ const stderr = probe.stderr.toString();
175+ const privateMatch = stderr.match(PRIVATE_PACKAGE_RE);
176+ if (privateMatch) {
177+ const offending = privateMatch[1];
178+ failures.push({
179+ pkg: pkg.name,
180+ subpath,
181+ reason: `published bundle imports private workspace package '${offending}'`,
182+ });
183+ console.log(` FAIL ${spec} — references private '${offending}'`);
184+ continue;
185+ }
186+ const peerMatch =
187+ stderr.match(/Cannot find package '([^']+)'/) ??
188+ stderr.match(/Cannot find module '([^']+)'/);
189+ const detail = peerMatch ? `missing peer '${peerMatch[1]}'` : firstMeaningfulLine(stderr);
190+ console.log(` skip ${spec} — ${detail}`);
191+ }
192+ } finally {
193+ await rm(tmp, { recursive: true, force: true });
194+ }
195+};
196+
197+const main = async () => {
198+ console.log("[smoke] packing public packages via publish-packages.ts --dry-run");
199+ await $`bun run scripts/publish-packages.ts --dry-run`.cwd(repoRoot);
200+
201+ const tarballs = new Map<string, string>();
202+ for (const relDir of PUBLIC_PACKAGE_DIRS) {
203+ const pkgDir = join(repoRoot, relDir);
204+ if (!existsSync(pkgDir)) continue;
205+ const pkg = await readPackageJson(pkgDir);
206+ const tarball = findTarball(pkgDir);
207+ if (tarball) tarballs.set(pkg.name, tarball);
208+ }
209+
210+ const failures: SmokeFailure[] = [];
211+ for (const relDir of PUBLIC_PACKAGE_DIRS) {
212+ const pkgDir = join(repoRoot, relDir);
213+ if (!existsSync(pkgDir)) {
214+ failures.push({ pkg: relDir, subpath: "", reason: "missing dir" });
215+ continue;
216+ }
217+ const pkg = await readPackageJson(pkgDir);
218+ console.log(`[smoke] ${pkg.name}`);
219+ await smokeTestPackage(pkgDir, tarballs, failures);
220+ }
221+
222+ if (failures.length === 0) {
223+ console.log("[smoke] all packages OK");
224+ return;
225+ }
226+
227+ console.error(`\n[smoke] ${failures.length} failure(s):`);
228+ for (const f of failures) {
229+ console.error(` - ${f.pkg}${f.subpath ? ` (${f.subpath})` : ""}: ${f.reason}`);
230+ }
231+ process.exit(1);
232+};
233+
234+await main();

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t