From b078028794d0ec35c03aa5bb63168341757e2e62 Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi Date: Sun, 2 Aug 2026 21:42:48 -0400 Subject: [PATCH 1/3] perf(ci): narrow merge-queue lanes for products with nested JS workspaces products/desktop vendors its own pnpm workspace, so its manifests, configs, and assets are neither .py nor .tsx nor under backend/. They fell into the "could be either domain" case and claimed every backend lane, serializing TypeScript-only PRs against all of Python. Files inside a subtree the product's own pnpm-workspace.yaml declares as a package now claim only the product's frontend lane. Everything else, including the product root manifests and any .py under a package, keeps widening. Co-Authored-By: Claude Opus 5 (1M context) --- .github/scripts/trunk-impacted-targets.js | 116 +++++++++++++++++- .../scripts/trunk-impacted-targets.test.js | 71 +++++++++++ 2 files changed, 185 insertions(+), 2 deletions(-) diff --git a/.github/scripts/trunk-impacted-targets.js b/.github/scripts/trunk-impacted-targets.js index 7502f0a861bb..239a32c96127 100644 --- a/.github/scripts/trunk-impacted-targets.js +++ b/.github/scripts/trunk-impacted-targets.js @@ -284,6 +284,103 @@ function listIsolatedProducts(repoRoot, products) { return isolated } +// --- Nested JS workspaces --- + +// A product that vendors its own pnpm workspace (products/desktop is the one +// today) has an apps/ + packages/ layout instead of the backend/ + frontend/ +// split the product rules assume. Its manifests, configs, and assets are none +// of .py, backend/, or .tsx, so they land in the "could be either" bucket and +// widen to every backend lane — a package.json under packages/ serializing a +// TypeScript-only PR against all of Python. +// +// The workspace file is the product's own declaration of which subtrees are JS +// packages, so it is a safer signal than an extension allowlist: a path only +// narrows when the product itself says a JS package lives there. Anything +// outside those subtrees (the product's root manifests, scripts/, backend/) +// keeps the old widening behavior. +const WORKSPACE_DECLARATION = 'pnpm-workspace.yaml' + +// Minimal reader for the `packages:` block of a pnpm workspace file. Only the +// list-of-globs form is understood; anything else yields no globs, which leaves +// the product on the old behavior rather than guessing. +function parseWorkspacePackageGlobs(text) { + const globs = [] + let inPackages = false + for (const rawLine of text.split('\n')) { + const line = rawLine.replace(/#.*$/, '').trimEnd() + if (!line.trim()) { + continue + } + if (/^packages:\s*$/.test(line)) { + inPackages = true + continue + } + if (!inPackages) { + continue + } + const item = line.match(/^\s+-\s+(.+)$/) + if (!item) { + break + } + globs.push(item[1].trim().replace(/^['"]|['"]$/g, '')) + } + return globs +} + +function compileWorkspaceMatcher(globs) { + const include = [] + const exclude = [] + for (const glob of globs) { + const negated = glob.startsWith('!') + const matcher = globToRegExp(negated ? glob.slice(1) : glob) + ;(negated ? exclude : include).push(matcher) + } + if (include.length === 0) { + return null + } + // The globs name package directories, so a file is inside the workspace + // when one of its ancestor directories matches. Testing the file path + // itself would miss everything below the package root. + return (relativePath) => { + const segments = relativePath.split('/') + for (let depth = 1; depth < segments.length; depth++) { + const dir = segments.slice(0, depth).join('/') + if (include.some((re) => re.test(dir)) && !exclude.some((re) => re.test(dir))) { + return true + } + } + return false + } +} + +function loadProductWorkspaces(repoRoot, products) { + const workspaces = new Map() + for (const product of products) { + const declaration = path.join(repoRoot, 'products', product, WORKSPACE_DECLARATION) + if (!fs.existsSync(declaration)) { + continue + } + let matcher + try { + matcher = compileWorkspaceMatcher(parseWorkspacePackageGlobs(fs.readFileSync(declaration, 'utf8'))) + } catch (error) { + console.error( + `Could not read products/${product}/${WORKSPACE_DECLARATION} (${error.message}); its files keep widening to every backend lane` + ) + continue + } + if (matcher) { + workspaces.set(product, matcher) + } + } + return workspaces +} + +function isInProductWorkspace(product, file, productWorkspaces) { + const matcher = productWorkspaces.get(product) + return matcher ? matcher(file.slice(`products/${product}/`.length)) : false +} + // --- Contract surfaces --- const CONTRACT_TASK = 'backend:contract-check' @@ -593,7 +690,14 @@ const feProduct = (product) => `fe:product:${product}` const rustCrate = (crate) => `rust:crate:${crate}` function computeTargets(changedFiles, context) { - const { products, isolatedProducts, rustGraph, tachGraph, contractSurfaces = new Map() } = context + const { + products, + isolatedProducts, + rustGraph, + tachGraph, + contractSurfaces = new Map(), + productWorkspaces = new Map(), + } = context const targets = new Set() const allPyProducts = () => { @@ -735,11 +839,16 @@ function computeTargets(changedFiles, context) { } const isBackend = segments[2] === 'backend' || file.endsWith('.py') const isFrontend = segments[2] === 'frontend' || /\.tsx?$/.test(file) + // Only reached for a file that is neither, and only inside a + // package the product's own pnpm workspace declares. A .py there + // is still backend: the workspace says the directory holds a JS + // package, not that Python cannot be checked into it. + const isWorkspaceOnly = !isBackend && !isFrontend && isInProductWorkspace(product, file, productWorkspaces) if (isFrontend || (!isBackend && !isFrontend)) { targets.add(feProduct(product)) } - if (isBackend || (!isBackend && !isFrontend)) { + if (isBackend || (!isBackend && !isFrontend && !isWorkspaceOnly)) { if (isolatedProducts.has(product)) { targets.add(pyProduct(product)) if (touchesContractSurface(product, file, contractSurfaces)) { @@ -853,6 +962,7 @@ function buildContext(repoRoot) { products, isolatedProducts: listIsolatedProducts(repoRoot, products), contractSurfaces: loadContractSurfaces(repoRoot, products), + productWorkspaces: loadProductWorkspaces(repoRoot, products), rustGraph: loadRustGraph(repoRoot), tachGraph: loadTachGraph(repoRoot), } @@ -862,9 +972,11 @@ module.exports = { computeTargets, buildContext, compileContractMatcher, + compileWorkspaceMatcher, globToRegExp, isProductDirectory, isTripwire, + parseWorkspacePackageGlobs, parseCrateDependencies, parseCrateName, reverseClosure, diff --git a/.github/scripts/trunk-impacted-targets.test.js b/.github/scripts/trunk-impacted-targets.test.js index 1e308816389c..5867316ab303 100644 --- a/.github/scripts/trunk-impacted-targets.test.js +++ b/.github/scripts/trunk-impacted-targets.test.js @@ -13,10 +13,12 @@ const assert = require('node:assert/strict') const { computeTargets, compileContractMatcher, + compileWorkspaceMatcher, globToRegExp, isProductDirectory, isTripwire, parseCrateDependencies, + parseWorkspacePackageGlobs, reverseClosure, ALL, } = require('./trunk-impacted-targets') @@ -44,6 +46,13 @@ const CONTEXT = { }, } +// gamma vendors its own pnpm workspace; alpha and beta keep the conventional +// backend/ + frontend/ layout, so the cases above stay on the old behavior. +const WORKSPACE_CONTEXT = { + ...CONTEXT, + productWorkspaces: new Map([['gamma', compileWorkspaceMatcher(['apps/*', 'packages/*', 'tooling/*'])]]), +} + test('every tripwire forces ALL', () => { const tripwireFiles = [ 'pnpm-lock.yaml', @@ -395,6 +404,68 @@ test('a product file that is neither backend nor frontend claims both domains', assert.equal(targets.includes('fe:product:beta'), true) }) +// A product vendoring its own pnpm workspace has no backend/ + frontend/ split, +// so its manifests and configs land in the "claims both domains" case above and +// drag every backend lane along. Narrowing them is the whole point of reading +// the workspace declaration. +test('a file inside a declared workspace package claims only the product lane', () => { + for (const file of [ + 'products/gamma/packages/agent/package.json', + 'products/gamma/apps/code/snapshots.yml', + 'products/gamma/tooling/config/biome.json', + 'products/gamma/apps/code/assets/icon.svg', + ]) { + assert.deepEqual(computeTargets([file], WORKSPACE_CONTEXT), ['fe:product:gamma'], file) + } +}) + +// The narrowing direction is the dangerous one: a backend lane that stops being +// claimed lets Trunk run this PR beside a conflicting backend PR. The workspace +// declaration says a directory holds a JS package, not that Python cannot be +// checked into it. +test('python inside a declared workspace package still claims the backend lanes', () => { + const targets = computeTargets(['products/gamma/packages/agent/scripts/codegen.py'], WORKSPACE_CONTEXT) + assert.equal(targets.includes('py:core'), true) +}) + +// Only the declared package subtrees narrow. The product root holds the files +// that decide isolation and contract surface, and anything else under the +// product is unclassified in the same way it was before. +test('files outside the declared workspace packages keep widening', () => { + for (const file of ['products/gamma/package.json', 'products/gamma/scripts/release.mjs']) { + assert.equal(computeTargets([file], WORKSPACE_CONTEXT).includes('py:core'), true, file) + } +}) + +// A real pnpm-workspace.yaml carries a catalog: block right after packages:, +// and reading past the list would turn catalog entries into package globs. +test('workspace globs are read only from the packages block', () => { + assert.deepEqual( + parseWorkspacePackageGlobs( + [ + 'packages:', + " - 'apps/*'", + ' - packages/*', + ' - "!packages/legacy"', + '', + 'catalog:', + ' hono: ^1.0.0', + ].join('\n') + ), + ['apps/*', 'packages/*', '!packages/legacy'] + ) +}) + +test('a negated workspace glob excludes its subtree from the narrowing', () => { + const matcher = compileWorkspaceMatcher(['packages/*', '!packages/legacy']) + assert.equal(matcher('packages/agent/package.json'), true) + assert.equal(matcher('packages/legacy/package.json'), false) +}) + +test('a workspace declaration with no packages block yields no matcher', () => { + assert.equal(compileWorkspaceMatcher(parseWorkspacePackageGlobs('catalog:\n hono: ^1.0.0\n')), null) +}) + // tools/ is not one bucket. phrocs is Go with its own CI and nothing imports // it, while hogli-commands is loaded by posthog/conftest.py on every pytest // run, so lumping them together either serializes phrocs needlessly or hands From a11b22c87761acf46ee571c050ff832f92796236 Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi Date: Mon, 3 Aug 2026 08:07:11 -0400 Subject: [PATCH 2/3] perf(ci): keep a vendored workspace's pnpm files out of the backend lanes products/desktop/pnpm-workspace.yaml and products/desktop/pnpm-lock.yaml sit at the product root, outside every package glob the declaration lists, so the narrowing did not reach them and each still claimed all 82 backend lanes. A dependency bump in the vendored workspace serialized against every Python PR in the queue. A pnpm-workspace.yaml makes its directory a workspace root rather than a member of the repo-root one, so the lockfile beside it resolves only that workspace's packages. Neither file is importable from Python. Co-Authored-By: Claude Opus 5 (1M context) --- .github/scripts/trunk-impacted-targets.js | 26 ++++++++++++++++--- .../scripts/trunk-impacted-targets.test.js | 15 +++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/.github/scripts/trunk-impacted-targets.js b/.github/scripts/trunk-impacted-targets.js index 239a32c96127..f8f4859b2116 100644 --- a/.github/scripts/trunk-impacted-targets.js +++ b/.github/scripts/trunk-impacted-targets.js @@ -290,8 +290,8 @@ function listIsolatedProducts(repoRoot, products) { // today) has an apps/ + packages/ layout instead of the backend/ + frontend/ // split the product rules assume. Its manifests, configs, and assets are none // of .py, backend/, or .tsx, so they land in the "could be either" bucket and -// widen to every backend lane — a package.json under packages/ serializing a -// TypeScript-only PR against all of Python. +// widen to every backend lane, which is how a package.json under packages/ +// serializes a TypeScript-only PR against all of Python. // // The workspace file is the product's own declaration of which subtrees are JS // packages, so it is a safer signal than an extension allowlist: a path only @@ -300,6 +300,22 @@ function listIsolatedProducts(repoRoot, products) { // keeps the old widening behavior. const WORKSPACE_DECLARATION = 'pnpm-workspace.yaml' +// pnpm's own two files at the product root. A pnpm-workspace.yaml makes that +// directory a workspace root rather than a member of the repo-root one, so the +// lockfile beside it resolves that workspace's packages and nothing else. The +// repo-root lockfile is a separate file and stays a tripwire in its own right. +// Neither of these is importable from Python, and neither is a contract +// declaration, so without this rule they fall through the layout checks below +// and claim every backend lane: a desktop dependency bump lands in the same +// lane as all of Python. +// +// The self-gating hazard that keeps CONTRACT_DECLARATIONS widening does not +// transfer here. turbo.json and package.json declare a Python import surface, +// so a PR that narrows one and edits under it in the same commit would gate +// itself against its own new contract. This pair declares no Python surface, +// and the .py carve-out below applies whatever the globs say. +const WORKSPACE_OWN_FILES = [WORKSPACE_DECLARATION, 'pnpm-lock.yaml'] + // Minimal reader for the `packages:` block of a pnpm workspace file. Only the // list-of-globs form is understood; anything else yields no globs, which leaves // the product on the old behavior rather than guessing. @@ -378,7 +394,11 @@ function loadProductWorkspaces(repoRoot, products) { function isInProductWorkspace(product, file, productWorkspaces) { const matcher = productWorkspaces.get(product) - return matcher ? matcher(file.slice(`products/${product}/`.length)) : false + if (!matcher) { + return false + } + const relativePath = file.slice(`products/${product}/`.length) + return WORKSPACE_OWN_FILES.includes(relativePath) || matcher(relativePath) } // --- Contract surfaces --- diff --git a/.github/scripts/trunk-impacted-targets.test.js b/.github/scripts/trunk-impacted-targets.test.js index 5867316ab303..ee6e6b1846d0 100644 --- a/.github/scripts/trunk-impacted-targets.test.js +++ b/.github/scripts/trunk-impacted-targets.test.js @@ -437,6 +437,21 @@ test('files outside the declared workspace packages keep widening', () => { } }) +// The workspace declaration and its lockfile sit at the product root, so the +// glob matcher alone leaves them in the "claims both domains" case and a +// dependency bump in the vendored workspace still claims every backend lane. +// The second assertion is the boundary: a product with no declaration keeps +// the old widening, which a basename-only version of this rule would lose. +test('the vendored workspace files claim only the product lane', () => { + for (const file of ['products/gamma/pnpm-workspace.yaml', 'products/gamma/pnpm-lock.yaml']) { + assert.deepEqual(computeTargets([file], WORKSPACE_CONTEXT), ['fe:product:gamma'], file) + } + assert.equal( + computeTargets(['products/alpha/pnpm-lock.yaml'], WORKSPACE_CONTEXT).includes('py:product:alpha'), + true + ) +}) + // A real pnpm-workspace.yaml carries a catalog: block right after packages:, // and reading past the list would turn catalog entries into package globs. test('workspace globs are read only from the packages block', () => { From 0ff22f396657b98a790b68c081388a9251c78160 Mon Sep 17 00:00:00 2001 From: Georges-Antoine Assi Date: Mon, 3 Aug 2026 08:17:25 -0400 Subject: [PATCH 3/3] perf(ci): keep a product with no backend surface off the backend lanes products/desktop is an app imported from another repository. pytest.ini ignores the subtree, ci-backend.yml excludes it from its path filter, tach.toml never declares it, and no Python here imports it. Its vendored .py files under tools/ and every config at its root still read as backend to the layout rules and claimed all 82 backend lanes, so either one serialized against every Python PR in the queue. A product that pytest ignores and tach does not declare now claims its own two lanes instead of all of them. Both declarations are already tripwires, so a PR that detaches a product cannot itself run beside anything. Co-Authored-By: Claude Opus 5 (1M context) --- .github/scripts/trunk-impacted-targets.js | 67 ++++++++++++++++++- .../scripts/trunk-impacted-targets.test.js | 32 +++++++++ 2 files changed, 98 insertions(+), 1 deletion(-) diff --git a/.github/scripts/trunk-impacted-targets.js b/.github/scripts/trunk-impacted-targets.js index f8f4859b2116..7df9c9dcd64e 100644 --- a/.github/scripts/trunk-impacted-targets.js +++ b/.github/scripts/trunk-impacted-targets.js @@ -401,6 +401,63 @@ function isInProductWorkspace(product, file, productWorkspaces) { return WORKSPACE_OWN_FILES.includes(relativePath) || matcher(relativePath) } +// --- Backend-detached products --- + +// The narrowing above stops at the layout rules: a product with a vendored +// workspace still owns every backend lane the moment one of its files reads as +// backend, because the product rules assume every product is a Django product +// whose Python some other product may import. products/desktop is not one. It +// is a standalone app imported from another repository, with no manifest.tsx, +// no backend/, no entry in frontend/src/products.json, and its own desktop-* +// CI. The Python it does carry is a vendored copy of that repository's own +// tooling under tools/, which this repository's suites never load. +// +// Two enforced declarations say so, and both have to hold: +// +// 1. pytest.ini ignores the subtree, so no backend test collects a single +// file under it. ci-backend.yml carries the same exclusion in its path +// filter, but a filter tuned to over-run is not a safe source for lane +// assignment, while an --ignore is a statement that the suite does not +// cover the path at all. +// 2. The product is absent from tach.toml, the enforced Python module graph, +// so no declared module may import it. +// +// A product satisfying both cannot fail another product's backend suite, so +// its files claim its own lanes instead of all of them. Either condition +// missing keeps the old widening, and so does an unreadable pytest.ini or an +// unavailable tach graph. Both declarations are already tripwires, so a PR +// that detaches a product cannot itself run beside anything. +const PYTEST_CONFIG = 'pytest.ini' + +// Reads the --ignore paths out of pytest's addopts. Nothing matching yields an +// empty list, which leaves every product on the old widening. +function parsePytestIgnores(text) { + return [...text.matchAll(/--ignore[= ](\S+)/g)].map((match) => match[1].replace(/\/+$/, '')) +} + +function loadBackendDetachedProducts(repoRoot, products, tachGraph) { + if (!tachGraph) { + return new Set() + } + let ignored + try { + ignored = new Set(parsePytestIgnores(fs.readFileSync(path.join(repoRoot, PYTEST_CONFIG), 'utf8'))) + } catch (error) { + console.error(`Could not read ${PYTEST_CONFIG} (${error.message}); every product widens to all backend lanes`) + return new Set() + } + const detached = new Set() + for (const product of products) { + // tach spells its modules both ways across the file, so a product + // counts as declared under either spelling. + const declared = tachGraph.graph.has(product) || tachGraph.graph.has(product.replace(/_/g, '-')) + if (ignored.has(`products/${product}`) && !declared) { + detached.add(product) + } + } + return detached +} + // --- Contract surfaces --- const CONTRACT_TASK = 'backend:contract-check' @@ -717,6 +774,7 @@ function computeTargets(changedFiles, context) { tachGraph, contractSurfaces = new Map(), productWorkspaces = new Map(), + backendDetachedProducts = new Set(), } = context const targets = new Set() @@ -874,6 +932,10 @@ function computeTargets(changedFiles, context) { if (touchesContractSurface(product, file, contractSurfaces)) { changedIsolatedProducts.add(product) } + } else if (backendDetachedProducts.has(product)) { + // No backend suite covers this product and no declared + // module imports it, so the lane it keeps is its own. + targets.add(pyProduct(product)) } else { allPyProducts() } @@ -978,13 +1040,15 @@ function loadTachGraph(repoRoot) { function buildContext(repoRoot) { const products = listProducts(repoRoot) + const tachGraph = loadTachGraph(repoRoot) return { products, isolatedProducts: listIsolatedProducts(repoRoot, products), contractSurfaces: loadContractSurfaces(repoRoot, products), productWorkspaces: loadProductWorkspaces(repoRoot, products), + backendDetachedProducts: loadBackendDetachedProducts(repoRoot, products, tachGraph), rustGraph: loadRustGraph(repoRoot), - tachGraph: loadTachGraph(repoRoot), + tachGraph, } } @@ -996,6 +1060,7 @@ module.exports = { globToRegExp, isProductDirectory, isTripwire, + parsePytestIgnores, parseWorkspacePackageGlobs, parseCrateDependencies, parseCrateName, diff --git a/.github/scripts/trunk-impacted-targets.test.js b/.github/scripts/trunk-impacted-targets.test.js index ee6e6b1846d0..d55f976b8f6b 100644 --- a/.github/scripts/trunk-impacted-targets.test.js +++ b/.github/scripts/trunk-impacted-targets.test.js @@ -18,6 +18,7 @@ const { isProductDirectory, isTripwire, parseCrateDependencies, + parsePytestIgnores, parseWorkspacePackageGlobs, reverseClosure, ALL, @@ -452,6 +453,37 @@ test('the vendored workspace files claim only the product lane', () => { ) }) +// delta stands in for products/desktop: an app imported from another +// repository that pytest.ini ignores and tach.toml never declares. Its +// vendored .py files read as backend to the layout rules, so without the +// detachment check they claim every backend lane for suites that never run on +// them. +const DETACHED_CONTEXT = { + ...WORKSPACE_CONTEXT, + products: [...CONTEXT.products, 'delta'], + backendDetachedProducts: new Set(['delta']), +} + +test('a backend-detached product keeps its own lane instead of every backend lane', () => { + assert.deepEqual(computeTargets(['products/delta/tools/agent/policy.py'], DETACHED_CONTEXT), ['py:product:delta']) + assert.deepEqual(computeTargets(['products/delta/biome.json'], DETACHED_CONTEXT), [ + 'fe:product:delta', + 'py:product:delta', + ]) + // gamma is ignored by neither declaration, so the same shapes still widen. + assert.equal(computeTargets(['products/gamma/tools/agent/policy.py'], DETACHED_CONTEXT).includes('py:core'), true) +}) + +// pytest.ini spells the list inside one long addopts line, so a reader anchored +// to the start of a line finds nothing and silently leaves every product +// widening. +test('pytest ignores are read from anywhere in addopts', () => { + assert.deepEqual( + parsePytestIgnores('addopts = -p no:warnings --ignore=tools/hogli --ignore=products/desktop --reuse-db'), + ['tools/hogli', 'products/desktop'] + ) +}) + // A real pnpm-workspace.yaml carries a catalog: block right after packages:, // and reading past the list would turn catalog entries into package globs. test('workspace globs are read only from the packages block', () => {