perf(ci): keep JS-only products out of the backend merge-queue lanes

.github/scripts/trunk-impacted-targets.js

@@ -284,6 +284,180 @@ 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, 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
+// 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'
+
+// 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.
+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)
+    if (!matcher) {
+        return false
+    }
+    const relativePath = file.slice(`products/${product}/`.length)
+    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'
@@ -593,7 +767,15 @@ 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(),
+        backendDetachedProducts = new Set(),
+    } = context
     const targets = new Set()
 
     const allPyProducts = () => {
@@ -735,16 +917,25 @@ 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)) {
                         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()
                 }
@@ -849,22 +1040,28 @@ 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,
     }
 }
 
 module.exports = {
     computeTargets,
     buildContext,
     compileContractMatcher,
+    compileWorkspaceMatcher,
     globToRegExp,
     isProductDirectory,
     isTripwire,
+    parsePytestIgnores,
+    parseWorkspacePackageGlobs,
     parseCrateDependencies,
     parseCrateName,
     reverseClosure,

.github/scripts/trunk-impacted-targets.test.js

@@ -13,10 +13,13 @@ const assert = require('node:assert/strict')
 const {
     computeTargets,
     compileContractMatcher,
+    compileWorkspaceMatcher,
     globToRegExp,
     isProductDirectory,
     isTripwire,
     parseCrateDependencies,
+    parsePytestIgnores,
+    parseWorkspacePackageGlobs,
     reverseClosure,
     ALL,
 } = require('./trunk-impacted-targets')
@@ -44,6 +47,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 +405,114 @@ 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)
+    }
+})
+
+// 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
+    )
+})
+
+// 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', () => {
+    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