From 0b3816956cf2fbf4219d117ceaf80cfcab473a8e Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Wed, 7 Oct 2026 15:47:18 +0200 Subject: [PATCH 1/2] feat(appkit): ship pnpm-patched dependencies inside published tarballs A pnpm patch (patchedDependencies) is applied only at this monorepo's install; it does not travel through a consumer's npm or pnpm install, so an app built on a published appkit would resolve the unpatched registry copy. tools/bundle-patched-deps.ts plans, per tarball, which patched packages the package uses (directly or transitively, including the CLI deps from shared), and dist-appkit.ts copies those patched copies into the tarball and lists them in bundledDependencies. Each bundled package's own dependencies are declared at their installed versions, since package managers do not install those for a bundled package. The build fails instead of shipping an unpatched copy when a patch entry is stale, the patch is not applied, or a bundled package needs a different dependency version than the tarball declares. CI now installs the built tarballs with npm and pnpm (tools/verify-bundled-patches.ts) and checks each bundled package resolves to the patched copy. With no patches defined (as on main today) nothing is bundled and the published tarballs are unchanged. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- .github/workflows/ci.yml | 6 + tools/bundle-patched-deps.test.ts | 182 ++++++++++++++++++++++++ tools/bundle-patched-deps.ts | 223 ++++++++++++++++++++++++++++++ tools/dist-appkit.ts | 41 ++++++ tools/verify-bundled-patches.ts | 96 +++++++++++++ 5 files changed, 548 insertions(+) create mode 100644 tools/bundle-patched-deps.test.ts create mode 100644 tools/bundle-patched-deps.ts create mode 100644 tools/verify-bundled-patches.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0edf4050c..ee90332f7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -259,6 +259,12 @@ jobs: - name: Build SDK tarballs run: pnpm pack:prerelease + # pnpm-patched dependencies are shipped inside the tarballs via + # bundledDependencies (tools/bundle-patched-deps.ts). Install each tarball + # with npm and pnpm and fail if a consumer would resolve an unpatched copy. + - name: Verify bundled patched dependencies + run: pnpm exec tsx tools/verify-bundled-patches.ts packages/appkit packages/appkit-ui + - name: Prepare template artifact run: pnpm exec tsx tools/prepare-template-artifact.ts diff --git a/tools/bundle-patched-deps.test.ts b/tools/bundle-patched-deps.test.ts new file mode 100644 index 000000000..4dc2ef792 --- /dev/null +++ b/tools/bundle-patched-deps.test.ts @@ -0,0 +1,182 @@ +import { + mkdirSync, + mkdtempSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; + +import { afterEach, expect, test } from "vitest"; + +import { + collectDependencyClosure, + findInstalledPackage, + parsePatchKey, + planBundledPatches, + readPatchedDependencies, +} from "./bundle-patched-deps"; + +const temporaryDirectories: string[] = []; +afterEach(() => { + for (const directory of temporaryDirectories.splice(0)) + rmSync(directory, { recursive: true, force: true }); +}); + +/** + * A pnpm-shaped tree: real packages live in `.pnpm//node_modules/` + * (patched ones under a `_patch_hash=` id) with their deps symlinked as + * siblings; `consumer/node_modules/` symlinks to the store. + */ +function pnpmTree( + packages: Array<{ + name: string; + version: string; + patched?: boolean; + deps?: Record; + }>, + consumerDeps: string[], +) { + const root = mkdtempSync(join(tmpdir(), "bundle-patched-")); + temporaryDirectories.push(root); + const store = join(root, "node_modules/.pnpm"); + const realDir = (name: string) => { + const p = packages.find((x) => x.name === name); + if (!p) throw new Error(`fixture is missing package ${name}`); + const id = `${name.replace("/", "+")}@${p.version}${p.patched ? "_patch_hash=abc" : ""}`; + return join(store, id, "node_modules", name); + }; + for (const p of packages) { + const dir = realDir(p.name); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, "package.json"), + JSON.stringify({ + name: p.name, + version: p.version, + dependencies: p.deps, + }), + ); + for (const dep of Object.keys(p.deps ?? {})) { + const link = join(dirname(dir), dep); + mkdirSync(dirname(link), { recursive: true }); + symlinkSync(realDir(dep), link); + } + } + const consumer = join(root, "consumer"); + for (const name of consumerDeps) { + const link = join(consumer, "node_modules", name); + mkdirSync(dirname(link), { recursive: true }); + symlinkSync(realDir(name), link); + } + return consumer; +} + +function plan( + consumer: string, + roots: string[], + patches: string[], + declared: Record, +) { + return planBundledPatches({ + patches: patches.map(parsePatchKey), + closure: collectDependencyClosure([{ fromDir: consumer, names: roots }]), + declared, + resolveDeclaredVersion: (name) => + findInstalledPackage(consumer, name)?.version, + }); +} + +test("parsePatchKey splits scoped and unscoped keys", () => { + expect(parsePatchKey("@scope/pkg@1.2.3")).toEqual({ + key: "@scope/pkg@1.2.3", + name: "@scope/pkg", + version: "1.2.3", + }); + expect(parsePatchKey("pkg@1.0.0").name).toBe("pkg"); + expect(() => parsePatchKey("@scope/pkg")).toThrow(/exact version/); +}); + +test("readPatchedDependencies reads pnpm-workspace.yaml, falling back to package.json", () => { + const root = mkdtempSync(join(tmpdir(), "bundle-patched-")); + temporaryDirectories.push(root); + writeFileSync( + join(root, "package.json"), + JSON.stringify({ pnpm: { patchedDependencies: { "old@1.0.0": "p" } } }), + ); + expect(readPatchedDependencies(root).map((p) => p.key)).toEqual([ + "old@1.0.0", + ]); + writeFileSync( + join(root, "pnpm-workspace.yaml"), + "patchedDependencies:\n '@s/new@2.0.0': patches/x.patch\n", + ); + expect(readPatchedDependencies(root).map((p) => p.key)).toEqual([ + "@s/new@2.0.0", + ]); +}); + +test("bundles a directly used patch and declares its undeclared deps", () => { + const consumer = pnpmTree( + [ + { name: "a", version: "1.0.0", patched: true, deps: { b: "^2" } }, + { name: "b", version: "2.1.0" }, + ], + ["a"], + ); + const result = plan(consumer, ["a"], ["a@1.0.0"], { a: "1.0.0" }); + expect(result.bundle.map((p) => p.name)).toEqual(["a"]); + expect(result.addDependencies).toEqual({ b: "2.1.0" }); +}); + +test("bundles a transitively used patch and declares it", () => { + const consumer = pnpmTree( + [ + { name: "top", version: "1.0.0", deps: { "@s/deep": "1.0.0" } }, + { name: "@s/deep", version: "1.0.0", patched: true }, + ], + ["top"], + ); + const result = plan(consumer, ["top"], ["@s/deep@1.0.0"], { top: "1.0.0" }); + expect(result.bundle.map((p) => p.name)).toEqual(["@s/deep"]); + expect(result.addDependencies).toEqual({ "@s/deep": "1.0.0" }); +}); + +test("ignores patches on packages the tarball does not use", () => { + const consumer = pnpmTree([{ name: "a", version: "1.0.0" }], ["a"]); + const result = plan(consumer, ["a"], ["unused@1.0.0"], { a: "1.0.0" }); + expect(result).toEqual({ bundle: [], addDependencies: {} }); +}); + +test("fails when the used version no longer matches the patch (stale entry)", () => { + const consumer = pnpmTree([{ name: "a", version: "1.1.0" }], ["a"]); + expect(() => plan(consumer, ["a"], ["a@1.0.0"], { a: "1.1.0" })).toThrow( + /installed at 1.1.0 but the patch targets 1.0.0/, + ); +}); + +test("fails when the patched version is installed without the patch", () => { + const consumer = pnpmTree([{ name: "a", version: "1.0.0" }], ["a"]); + expect(() => plan(consumer, ["a"], ["a@1.0.0"], { a: "1.0.0" })).toThrow( + /without the patch applied/, + ); +}); + +test("fails when a bundled package needs a different version than declared", () => { + const consumer = pnpmTree( + [ + { name: "a", version: "1.0.0", patched: true, deps: { b: "2.0.0" } }, + { name: "b", version: "2.0.0" }, + ], + ["a"], + ); + expect(() => + planBundledPatches({ + patches: [parsePatchKey("a@1.0.0")], + closure: collectDependencyClosure([{ fromDir: consumer, names: ["a"] }]), + declared: { a: "1.0.0", b: "3.0.0" }, + resolveDeclaredVersion: () => "3.0.0", + }), + ).toThrow(/needs b@2.0.0, but the package declares b@3.0.0/); +}); diff --git a/tools/bundle-patched-deps.ts b/tools/bundle-patched-deps.ts new file mode 100644 index 000000000..62a14c243 --- /dev/null +++ b/tools/bundle-patched-deps.ts @@ -0,0 +1,223 @@ +import { createHash } from "node:crypto"; +import fs from "node:fs"; +import path from "node:path"; + +import { parse } from "yaml"; + +/** + * Ship pnpm-patched dependencies inside a published tarball. + * + * A pnpm patch (`patchedDependencies`) is applied only at this monorepo's + * install; it does not travel through a consumer's `npm install` / + * `pnpm install`, so a deployed app would resolve the unpatched registry copy. + * Instead, every patched package the tarball's package actually uses (directly + * or transitively) is copied into the tarball's `node_modules` and listed in + * `bundledDependencies`, so the consumer resolves the patched copy. + * + * A bundled package's own dependencies are NOT installed by the consumer's + * package manager on its behalf, and under pnpm a bundled package can only + * see what the tarball's package declares. So each of them must be a declared + * dependency of the tarball's package, at the version the patched copy was + * installed with; the plan adds missing ones and fails on conflicts. + */ + +export interface PatchedDependency { + /** The `patchedDependencies` key, e.g. `@scope/pkg@1.2.3`. */ + key: string; + name: string; + version: string; +} + +export interface InstalledPackage { + name: string; + version: string; + /** Real (symlink-resolved) package directory. */ + dir: string; + dependencies: Record; +} + +export interface BundlePlan { + /** Patched packages to copy into the tarball's node_modules. */ + bundle: InstalledPackage[]; + /** Dependencies to add to the tarball's package.json (name → exact version). */ + addDependencies: Record; +} + +/** Split a `patchedDependencies` key (`name@version`, name may be scoped). */ +export function parsePatchKey(key: string): PatchedDependency { + const at = key.lastIndexOf("@"); + if (at <= 0) { + throw new Error( + `patchedDependencies: "${key}" must pin an exact version (name@version)`, + ); + } + return { key, name: key.slice(0, at), version: key.slice(at + 1) }; +} + +/** Read `patchedDependencies` from pnpm-workspace.yaml (pnpm 11) or package.json. */ +export function readPatchedDependencies(rootDir: string): PatchedDependency[] { + const workspaceFile = path.join(rootDir, "pnpm-workspace.yaml"); + const fromWorkspace = fs.existsSync(workspaceFile) + ? parse(fs.readFileSync(workspaceFile, "utf-8"))?.patchedDependencies + : undefined; + const rootPkg = JSON.parse( + fs.readFileSync(path.join(rootDir, "package.json"), "utf-8"), + ); + const entries = fromWorkspace ?? rootPkg.pnpm?.patchedDependencies ?? {}; + return Object.keys(entries).map(parsePatchKey); +} + +/** + * Find the installed package `name` as Node would resolve it from `fromDir` + * (walking up `node_modules` directories), returning its real directory. + */ +export function findInstalledPackage( + fromDir: string, + name: string, +): InstalledPackage | undefined { + let dir = fromDir; + while (true) { + const manifest = path.join(dir, "node_modules", name, "package.json"); + if (fs.existsSync(manifest)) { + const realDir = fs.realpathSync(path.dirname(manifest)); + const json = JSON.parse(fs.readFileSync(manifest, "utf-8")); + return { + name, + version: json.version, + dir: realDir, + dependencies: { + ...json.dependencies, + ...json.optionalDependencies, + }, + }; + } + const parent = path.dirname(dir); + if (parent === dir) return undefined; + dir = parent; + } +} + +/** + * Every installed package reachable from `roots` through dependencies, + * keyed by name (a name may resolve to several versions in the tree). + * Unresolvable names (e.g. uninstalled optional deps) are skipped. + */ +export function collectDependencyClosure( + roots: Array<{ fromDir: string; names: string[] }>, +): Map { + const byName = new Map(); + const seen = new Set(); + const queue = roots.flatMap(({ fromDir, names }) => + names.map((name) => ({ fromDir, name })), + ); + // Index loop rather than shift(): the queue grows while we walk it. + for (let i = 0; i < queue.length; i++) { + const { fromDir, name } = queue[i]; + const found = findInstalledPackage(fromDir, name); + if (!found || seen.has(found.dir)) continue; + seen.add(found.dir); + byName.set(name, [...(byName.get(name) ?? []), found]); + for (const dep of Object.keys(found.dependencies)) { + queue.push({ fromDir: found.dir, name: dep }); + } + } + return byName; +} + +/** + * Decide which patched packages to bundle and which dependencies they need + * declared. Throws when a patch can't be shipped correctly: + * - the package is used but not at the patched version (stale patch entry), + * - the patched version is installed without the patch applied, + * - a bundled package needs a dependency at a different version than the one + * the tarball's package already declares. + */ +export function planBundledPatches(input: { + patches: PatchedDependency[]; + closure: Map; + /** Final dependencies of the tarball's package.json. */ + declared: Record; + /** Installed version of a declared dependency, as the package resolves it. */ + resolveDeclaredVersion: (name: string) => string | undefined; +}): BundlePlan { + const { patches, closure, declared, resolveDeclaredVersion } = input; + const bundle: InstalledPackage[] = []; + + for (const patch of patches) { + const installed = closure.get(patch.name); + if (!installed) continue; // patch targets something this package doesn't use + const match = installed.find((p) => p.version === patch.version); + if (!match) { + throw new Error( + `bundled patches: ${patch.name} is installed at ${installed + .map((p) => p.version) + .join(", ")} but the patch targets ${patch.version}. ` + + `Update the "${patch.key}" entry in patchedDependencies.`, + ); + } + // pnpm stores patched packages under a "_patch_hash=" directory. + if (!match.dir.includes("_patch_hash=")) { + throw new Error( + `bundled patches: ${patch.key} is installed without the patch applied ` + + `(${match.dir}). Run pnpm install.`, + ); + } + bundle.push(match); + } + + const bundledNames = new Set(bundle.map((p) => p.name)); + const addDependencies: Record = {}; + for (const pkg of bundle) { + for (const dep of Object.keys(pkg.dependencies)) { + if (bundledNames.has(dep)) continue; + const needed = findInstalledPackage(pkg.dir, dep)?.version; + if (!needed) continue; // uninstalled optional dependency + if (declared[dep] !== undefined) { + const have = resolveDeclaredVersion(dep); + if (have !== undefined && have !== needed) { + throw new Error( + `bundled patches: ${pkg.name} needs ${dep}@${needed}, but the ` + + `package declares ${dep}@${declared[dep]} (installed ${have}).`, + ); + } + continue; + } + const prior = addDependencies[dep]; + if (prior !== undefined && prior !== needed) { + throw new Error( + `bundled patches: conflicting versions of ${dep} needed (${prior}, ${needed}).`, + ); + } + addDependencies[dep] = needed; + } + } + + // Bundled packages must be listed in dependencies to be packed + installed. + for (const pkg of bundle) { + if (declared[pkg.name] === undefined) { + addDependencies[pkg.name] = pkg.version; + } + } + + return { bundle, addDependencies }; +} + +/** Content hash of a package directory (relative paths + file bytes). */ +export function hashPackageDir(dir: string): string { + const hash = createHash("sha256"); + const walk = (current: string) => { + for (const entry of fs + .readdirSync(current, { withFileTypes: true }) + .sort((a, b) => a.name.localeCompare(b.name))) { + if (entry.name === "node_modules") continue; + const full = path.join(current, entry.name); + if (entry.isDirectory()) walk(full); + else { + hash.update(path.relative(dir, full)); + hash.update(fs.readFileSync(full)); + } + } + }; + walk(dir); + return hash.digest("hex"); +} diff --git a/tools/dist-appkit.ts b/tools/dist-appkit.ts index fee75f96a..076e9ef19 100644 --- a/tools/dist-appkit.ts +++ b/tools/dist-appkit.ts @@ -2,6 +2,13 @@ import fs from "node:fs"; import path from "node:path"; import { parseArgs } from "node:util"; +import { + collectDependencyClosure, + findInstalledPackage, + planBundledPatches, + readPatchedDependencies, +} from "./bundle-patched-deps"; + const __dirname = path.dirname(new URL(import.meta.url).pathname); const { values } = parseArgs({ options: { @@ -23,6 +30,14 @@ const pkg = JSON.parse(fs.readFileSync("package.json", "utf-8")); // "shared" is intentionally excluded: it is bundled directly into appkit/appkit-ui via noExternal. const WORKSPACE_PACKAGE_REPLACEMENTS = ["@databricks/lakebase"]; +// Snapshot the package's own dependency names before they are rewritten below: +// they (plus the CLI dependencies from shared) are where the search for +// pnpm-patched packages to bundle starts. Workspace packages are published as +// their own tarballs, so their dependency trees are not walked here. +const ownDependencyNames = Object.keys(pkg.dependencies ?? {}).filter( + (name) => name !== "shared" && !WORKSPACE_PACKAGE_REPLACEMENTS.includes(name), +); + if (prerelease) { pkg.version = `${pkg.version}-pr.${prerelease}`; } @@ -64,6 +79,32 @@ if (fs.existsSync(sharedPostinstall)) { pkg.dependencies = pkg.dependencies || {}; Object.assign(pkg.dependencies, CLI_DEPENDENCIES); +// Ship every pnpm-patched dependency this package uses inside the tarball via +// `bundledDependencies` (see tools/bundle-patched-deps.ts for why and how). +const sharedDir = path.dirname(sharedPkgPath); +const plan = planBundledPatches({ + patches: readPatchedDependencies(path.join(__dirname, "..")), + closure: collectDependencyClosure([ + { fromDir: process.cwd(), names: ownDependencyNames }, + { fromDir: sharedDir, names: Object.keys(CLI_DEPENDENCIES ?? {}) }, + ]), + declared: pkg.dependencies, + resolveDeclaredVersion: (name) => + findInstalledPackage(process.cwd(), name)?.version ?? + findInstalledPackage(sharedDir, name)?.version, +}); +Object.assign(pkg.dependencies, plan.addDependencies); +for (const patched of plan.bundle) { + // `dereference` copies the real files, not pnpm's store symlink. + const dest = path.join("tmp/node_modules", patched.name); + fs.rmSync(dest, { recursive: true, force: true }); + fs.mkdirSync(path.dirname(dest), { recursive: true }); + fs.cpSync(patched.dir, dest, { recursive: true, dereference: true }); +} +if (plan.bundle.length > 0) { + pkg.bundledDependencies = plan.bundle.map((p) => p.name); +} + fs.writeFileSync("tmp/package.json", JSON.stringify(pkg, null, 2)); fs.cpSync("dist", "tmp/dist", { recursive: true }); diff --git a/tools/verify-bundled-patches.ts b/tools/verify-bundled-patches.ts new file mode 100644 index 000000000..3ded92a3a --- /dev/null +++ b/tools/verify-bundled-patches.ts @@ -0,0 +1,96 @@ +/** + * End-to-end check that a built tarball's bundled patched dependencies survive + * a real consumer install, with both npm and pnpm. + * + * Usage (after `pnpm tarball` / `pnpm tarball:prerelease`): + * tsx tools/verify-bundled-patches.ts packages/appkit [packages/appkit-ui ...] + * + * For each package it installs `/tmp/*.tgz` into a scratch project and, + * from the installed package's real directory, asserts every + * `bundledDependencies` entry resolves to the bundled (patched) copy and that + * each of that copy's dependencies resolves too. + */ +import { execFileSync } from "node:child_process"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { findInstalledPackage, hashPackageDir } from "./bundle-patched-deps"; + +const packageDirs = process.argv.slice(2); +if (packageDirs.length === 0) { + console.error("usage: verify-bundled-patches.ts [...]"); + process.exit(2); +} + +let failures = 0; +const fail = (message: string) => { + failures++; + console.error(` ✗ ${message}`); +}; + +for (const packageDir of packageDirs) { + const tmpDir = path.resolve(packageDir, "tmp"); + const manifest = JSON.parse( + fs.readFileSync(path.join(tmpDir, "package.json"), "utf-8"), + ); + const bundled: string[] = manifest.bundledDependencies ?? []; + const tarball = fs.readdirSync(tmpDir).find((f) => f.endsWith(".tgz")); + if (!tarball) throw new Error(`no tarball in ${tmpDir}; build it first`); + console.log( + `${manifest.name} (${tarball}): bundled ${bundled.join(", ") || "none"}`, + ); + if (bundled.length === 0) continue; + + for (const pm of ["npm", "pnpm"] as const) { + const scratch = fs.mkdtempSync(path.join(os.tmpdir(), `verify-${pm}-`)); + try { + fs.writeFileSync( + path.join(scratch, "package.json"), + JSON.stringify({ + name: "verify-consumer", + private: true, + dependencies: { + [manifest.name]: `file:${path.join(tmpDir, tarball)}`, + }, + }), + ); + execFileSync(pm, ["install", "--ignore-scripts"], { + cwd: scratch, + stdio: "ignore", + }); + const installedDir = fs.realpathSync( + path.join(scratch, "node_modules", manifest.name), + ); + for (const name of bundled) { + const resolved = findInstalledPackage(installedDir, name); + if (!resolved) { + fail(`${pm}: ${name} does not resolve from ${manifest.name}`); + continue; + } + const expected = hashPackageDir( + path.join(tmpDir, "node_modules", name), + ); + if (hashPackageDir(resolved.dir) !== expected) { + fail( + `${pm}: ${name} resolves to an unpatched copy (${resolved.dir})`, + ); + continue; + } + for (const dep of Object.keys(resolved.dependencies)) { + if (!findInstalledPackage(resolved.dir, dep)) { + fail(`${pm}: ${name}'s dependency ${dep} does not resolve`); + } + } + console.log(` ✓ ${pm}: ${name} resolves to the bundled patched copy`); + } + } finally { + fs.rmSync(scratch, { recursive: true, force: true }); + } + } +} + +if (failures > 0) { + console.error(`${failures} bundled-patch check(s) failed`); + process.exit(1); +} From 0ed973867be78fe0e72871c8b17bf90fe7342724 Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Thu, 8 Oct 2026 15:53:11 +0200 Subject: [PATCH 2/2] ci: run CI on stacked pull requests Drop the base-branch filter on the pull_request trigger so PRs whose base is another PR branch (a stack) get CI, not only PRs into main. Co-authored-by: Isaac Signed-off-by: MarioCadenas --- .github/workflows/ci.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ee90332f7..b361bdc91 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,9 +1,8 @@ name: CI on: + # No base-branch filter: stacked PRs (base = the PR below them) get CI too. pull_request: - branches: - - main concurrency: group: ${{ github.workflow }}-${{ github.ref }}