From 125844220a66f8770527c19fc7e38e3f393fdf06 Mon Sep 17 00:00:00 2001 From: Carlos Santana <1540596+ophiocus@users.noreply.github.com> Date: Sat, 29 Aug 2026 15:28:50 -0500 Subject: [PATCH 1/4] feat(cli): detect project roots across ecosystems, not just Node MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `init` proposed checks and provisioning only for a repository-root Node project; anything else — a Poetry monorepo, a PHP service beside a Go worker — started on exactly the empty job site the method doc warns about. Detection now walks the tree for project roots (Node, Python, PHP, Go, Ruby, Rust, Java), absorbs workspace members into their workspace root, skips fixture/sample directories, and proposes per-root provisioning plus a union of root-scoped checks. Node workspace repositories detect identically to before. Closes #199 Co-Authored-By: Claude Opus 5 --- apps/docs/docs/faq.md | 10 +- packages/cli/src/detect.mjs | 376 ++++++++++++++++++++++++++++-- packages/cli/test/detect.test.mjs | 135 +++++++++++ 3 files changed, 495 insertions(+), 26 deletions(-) create mode 100644 packages/cli/test/detect.test.mjs diff --git a/apps/docs/docs/faq.md b/apps/docs/docs/faq.md index 7ed5842f..453bb7bd 100644 --- a/apps/docs/docs/faq.md +++ b/apps/docs/docs/faq.md @@ -59,9 +59,13 @@ the code in your repo. ## Does this work for non-Node projects? Yes. The CLI and the vendored guards need Node on the runner (present on all -GitHub-hosted runners) — your project doesn't. `init` detects pnpm/yarn/npm -and otherwise leaves a marked slot for your toolchain steps. The provision -command is yours: `make db`, `docker compose up -d`, `mix ecto.setup`. +GitHub-hosted runners) — your project doesn't. `init` detects project roots +across ecosystems — Node (pnpm/yarn/npm), Python (Poetry/uv/PDM/Hatch/pip), +PHP (Composer), Go, Ruby, Rust, and Java — including several side by side in +one repository, and proposes per-root provision and check commands. Anything +it cannot see still gets a marked slot for your toolchain steps, and the +provision command stays yours: `make db`, `docker compose up -d`, +`mix ecto.setup`. ## Can I use my existing AGENTS.md / CLAUDE.md / .claude setup? diff --git a/packages/cli/src/detect.mjs b/packages/cli/src/detect.mjs index 21df3626..5be334e1 100644 --- a/packages/cli/src/detect.mjs +++ b/packages/cli/src/detect.mjs @@ -1,8 +1,45 @@ // Stack detection: read the target repo and propose sensible defaults so // `init` asks six short questions instead of twenty. +// +// Detection is multi-root and multi-ecosystem. A repository may hold several +// independent project roots — nine Poetry packages beside a Next.js app, a PHP +// service beside a Go worker — and a change may touch any subset of them. So +// `init` discovers every root rather than picking the first one it finds, and +// the checks it proposes are the union over those roots. import { existsSync, readFileSync, readdirSync } from "node:fs"; import { execFileSync } from "node:child_process"; -import { join } from "node:path"; +import { join, relative, sep } from "node:path"; + +// How far below the repository root a project root may sit. Three levels +// covers the layouts we have seen (`services/api/`, `packages/x/y/`) without +// walking build output that slipped past SKIP_DIRS. +const MAX_ROOT_DEPTH = 3; + +const SKIP_DIRS = new Set([ + "node_modules", + "vendor", + "venv", + "dist", + "build", + "target", + "coverage", + "__pycache__", + "bin", + "obj", + "tmp", + // Fixtures and samples hold manifests on purpose; proposing their checks as + // repository defaults would be noise. The adapters still read `tests/` on a + // root they already claimed — this only stops the walk treating such + // directories as project roots of their own. + "test", + "tests", + "fixtures", + "__fixtures__", + "examples", + "samples", + "e2e", + "testdata", +]); function readJson(path) { try { @@ -12,6 +49,14 @@ function readJson(path) { } } +function readText(path) { + try { + return readFileSync(path, "utf8"); + } catch { + return null; + } +} + function git(cwd, args) { try { return execFileSync("git", args, { cwd, encoding: "utf8", stdio: ["ignore", "pipe", "ignore"] }).trim(); @@ -20,29 +65,316 @@ function git(cwd, args) { } } +// --- ecosystem adapters --------------------------------------------------- +// Each adapter answers one question about one directory: is there a project +// root of my kind here, and if so what provisions it, what checks it, and +// where its migrations live. A table, in the same spirit as +// detectDeploymentProvider — adding an ecosystem is one entry, not a branch. + +const ADAPTERS = [ + { + id: "node", + detect(dir) { + const pkg = readJson(join(dir, "package.json")); + if (!pkg) return null; + const scripts = pkg.scripts ?? {}; + const manager = existsSync(join(dir, "pnpm-lock.yaml")) + ? "pnpm" + : existsSync(join(dir, "yarn.lock")) + ? "yarn" + : "npm"; + const runner = manager === "npm" ? "npm run" : `${manager} run`; + const checks = []; + // `test:run` is the watch-free vitest convention; honored when `test` + // itself is absent so those repos do not lose their test gate. + for (const candidates of [["typecheck"], ["lint"], ["test", "test:run"], ["build"]]) { + const name = candidates.find((candidate) => scripts[candidate]); + if (name) checks.push(`${runner} ${name}`); + } + // The toolchain step already installs Node dependencies, so provisioning + // here means only the project's own extra setup. + const pnpmWorkspace = readText(join(dir, "pnpm-workspace.yaml")); + const declared = Array.isArray(pkg.workspaces) ? pkg.workspaces : (pkg.workspaces?.packages ?? []); + const workspaceGlobs = pnpmWorkspace ? pnpmWorkspaceGlobs(pnpmWorkspace) : declared; + + return { + manager, + provision: scripts.setup ? `${runner} setup` : "", + checks, + workspaceGlobs, + migrationDirs: ["migrations", "supabase/migrations", "db/migrations", "prisma/migrations"], + }; + }, + }, + { + id: "python", + detect(dir) { + const pyproject = readText(join(dir, "pyproject.toml")); + const requirements = existsSync(join(dir, "requirements.txt")); + if (!pyproject && !requirements) return null; + + let manager = "pip"; + let provision = requirements ? "pip install -r requirements.txt" : "pip install -e ."; + let prefix = ""; + if (pyproject?.includes("[tool.poetry")) { + manager = "poetry"; + provision = "poetry install"; + prefix = "poetry run "; + } else if (pyproject?.includes("[tool.uv") || existsSync(join(dir, "uv.lock"))) { + manager = "uv"; + provision = "uv sync"; + prefix = "uv run "; + } else if (pyproject?.includes("[tool.pdm")) { + manager = "pdm"; + provision = "pdm install"; + prefix = "pdm run "; + } else if (pyproject?.includes("[tool.hatch")) { + manager = "hatch"; + provision = "hatch env create"; + prefix = "hatch run "; + } + + // Only propose a tool the project actually configures — the same rule the + // Node adapter follows by reading `scripts`. + const checks = []; + if (pyproject?.includes("[tool.ruff")) checks.push(`${prefix}ruff check .`); + if (pyproject?.includes("[tool.mypy") || existsSync(join(dir, "mypy.ini"))) { + checks.push(`${prefix}mypy .`); + } + const hasTests = + pyproject?.includes("[tool.pytest") || + existsSync(join(dir, "pytest.ini")) || + existsSync(join(dir, "tests")) || + existsSync(join(dir, "test")); + if (hasTests) checks.push(`${prefix}pytest`); + + return { manager, provision, checks, migrationDirs: ["alembic/versions", "migrations"] }; + }, + }, + { + id: "php", + detect(dir) { + const composer = readJson(join(dir, "composer.json")); + if (!composer) return null; + const scripts = composer.scripts ?? {}; + const checks = []; + if (scripts.lint) checks.push("composer run lint"); + if (scripts.test) checks.push("composer run test"); + else if (existsSync(join(dir, "tests/Pest.php"))) checks.push("vendor/bin/pest"); + else if ( + existsSync(join(dir, "phpunit.xml")) || + existsSync(join(dir, "phpunit.xml.dist")) || + existsSync(join(dir, "tests")) + ) { + checks.push("vendor/bin/phpunit"); + } + return { + manager: "composer", + provision: "composer install --no-interaction --prefer-dist", + checks, + migrationDirs: ["database/migrations", "migrations"], + }; + }, + }, + { + id: "go", + detect(dir) { + if (!existsSync(join(dir, "go.mod"))) return null; + return { + manager: "go", + provision: "go mod download", + checks: ["go build ./...", "go vet ./...", "go test ./..."], + migrationDirs: ["migrations", "db/migrations"], + }; + }, + }, + { + id: "ruby", + detect(dir) { + if (!existsSync(join(dir, "Gemfile"))) return null; + const checks = []; + if (existsSync(join(dir, ".rubocop.yml"))) checks.push("bundle exec rubocop"); + if (existsSync(join(dir, "spec"))) checks.push("bundle exec rspec"); + else if (existsSync(join(dir, "test"))) checks.push("bundle exec rake test"); + return { + manager: "bundler", + provision: "bundle install", + checks, + migrationDirs: ["db/migrate"], + }; + }, + }, + { + id: "rust", + detect(dir) { + const cargo = readText(join(dir, "Cargo.toml")); + if (cargo === null) return null; + return { + manager: "cargo", + provision: "cargo fetch", + checks: ["cargo build --locked", "cargo test --locked"], + workspaceGlobs: cargoWorkspaceGlobs(cargo), + migrationDirs: ["migrations"], + }; + }, + }, + { + id: "java", + detect(dir) { + const maven = existsSync(join(dir, "pom.xml")); + const gradle = existsSync(join(dir, "build.gradle")) || existsSync(join(dir, "build.gradle.kts")); + if (!maven && !gradle) return null; + if (maven) { + return { manager: "maven", provision: "mvn -B -DskipTests install", checks: ["mvn -B verify"], migrationDirs: ["src/main/resources/db/migration"] }; + } + const wrapper = existsSync(join(dir, "gradlew")); + const cmd = wrapper ? "./gradlew" : "gradle"; + return { + manager: "gradle", + provision: `${cmd} --no-daemon dependencies`, + checks: [`${cmd} --no-daemon build`], + migrationDirs: ["src/main/resources/db/migration"], + }; + }, + }, +]; + +// --- workspace absorption ------------------------------------------------- +// A workspace member is not an independent root: the workspace root's own +// scripts already fan out to it. Listing members separately would provision and +// check the same package twice. Only roots that no parent workspace claims — +// the nine Poetry packages in a polyglot repo, a Go worker beside a PHP app — +// stand on their own. + +function pnpmWorkspaceGlobs(text) { + const globs = []; + let inPackages = false; + for (const line of text.split(/\r?\n/)) { + if (/^packages:\s*$/.test(line)) { + inPackages = true; + continue; + } + if (!inPackages) continue; + if (line.trim() === "" || line.trim().startsWith("#")) continue; + const item = line.match(/^\s+-\s*['"]?([^'"#\s]+)['"]?\s*$/); + if (item) { + globs.push(item[1]); + continue; + } + break; // next top-level key ends the list + } + return globs; +} + +function cargoWorkspaceGlobs(text) { + const section = text.match(/\[workspace\]([\s\S]*?)(?=\n\[|$)/); + if (!section) return []; + const members = section[1].match(/members\s*=\s*\[([\s\S]*?)\]/); + if (!members) return []; + return [...members[1].matchAll(/["']([^"']+)["']/g)].map((m) => m[1]); +} + +// Workspace globs are the small subset npm/pnpm/cargo actually use: a literal +// path, `dir/*`, or `dir/**`. +function globToRegExp(glob) { + const escaped = glob + .replace(/[.+^${}()|[\]\\]/g, "\\$&") + .replace(/\*\*/g, "") + .replace(/\*/g, "[^/]+") + .replace(//g, ".+"); + return new RegExp(`^${escaped}$`); +} + +function absorbWorkspaceMembers(roots) { + const workspaces = roots.filter((r) => r.workspaceGlobs?.length); + if (!workspaces.length) return roots; + return roots.filter((root) => { + for (const parent of workspaces) { + if (parent === root || parent.ecosystem !== root.ecosystem) continue; + const prefix = parent.path === "." ? "" : `${parent.path}/`; + if (!root.path.startsWith(prefix) || root.path === parent.path) continue; + const inner = root.path.slice(prefix.length); + // A root nested below a member (`runner/agent-clis` under the `runner` + // member) belongs to that member, so every ancestor path is tested. + const segments = inner.split("/"); + for (let i = 1; i <= segments.length; i++) { + const ancestor = segments.slice(0, i).join("/"); + if (parent.workspaceGlobs.some((glob) => globToRegExp(glob).test(ancestor))) return false; + } + } + return true; + }); +} + +// Walk the tree and collect every project root. A single directory can hold +// roots of more than one ecosystem (a PHP service that also builds assets), and +// both are kept — dropping one is how `init` ends up provisioning half a repo. +function detectRoots(dir) { + const found = []; + walk(dir, 0); + const roots = absorbWorkspaceMembers(found); + // Shallow roots first, then alphabetical: a stable order so generated + // manifests do not churn between runs. + roots.sort((a, b) => a.path.split("/").length - b.path.split("/").length || a.path.localeCompare(b.path)); + return roots; + + function walk(current, depth) { + const rel = relative(dir, current).split(sep).join("/") || "."; + for (const adapter of ADAPTERS) { + const hit = adapter.detect(current); + if (hit) found.push({ path: rel, ecosystem: adapter.id, ...hit }); + } + if (depth >= MAX_ROOT_DEPTH) return; + let entries; + try { + entries = readdirSync(current, { withFileTypes: true }); + } catch { + return; + } + for (const entry of entries) { + if (!entry.isDirectory()) continue; + if (entry.name.startsWith(".") || SKIP_DIRS.has(entry.name)) continue; + walk(join(current, entry.name), depth + 1); + } + } +} + +// A command for a root below the repository root has to run there. Roots at +// "." are emitted verbatim, so single-root repositories are unchanged. +function scopeCommand(rootPath, command) { + return rootPath === "." ? command : `(cd ${rootPath} && ${command})`; +} + export function detect(dir) { - const pkg = readJson(join(dir, "package.json")); - const scripts = pkg?.scripts ?? {}; - - const packageManager = existsSync(join(dir, "pnpm-lock.yaml")) - ? "pnpm" - : existsSync(join(dir, "yarn.lock")) - ? "yarn" - : existsSync(join(dir, "package-lock.json")) - ? "npm" - : pkg - ? "npm" - : "none"; - - const runner = packageManager === "none" ? null : packageManager === "npm" ? "npm run" : `${packageManager} run`; + const roots = detectRoots(dir); + + // `packageManager` still describes the repository-root Node toolchain, which + // is what the CI setup step needs. `ecosystems` and `roots` carry the rest. + const rootNode = roots.find((r) => r.path === "." && r.ecosystem === "node"); + const packageManager = rootNode ? rootNode.manager : "none"; + const ecosystems = [...new Set(roots.map((r) => r.ecosystem))].sort(); + const checks = []; - if (runner) { - for (const name of ["typecheck", "lint", "test", "build"]) { - if (scripts[name]) checks.push(`${runner} ${name}`); + const provisions = []; + for (const root of roots) { + for (const check of root.checks) { + const scoped = scopeCommand(root.path, check); + if (!checks.includes(scoped)) checks.push(scoped); + } + if (root.provision) { + const scoped = scopeCommand(root.path, root.provision); + if (!provisions.includes(scoped)) provisions.push(scoped); } } + const provision = provisions.join(" && "); - const provision = runner && scripts["setup"] ? `${runner} setup` : ""; + const migrationDirs = []; + for (const root of roots) { + for (const candidate of root.migrationDirs ?? []) { + const rel = root.path === "." ? candidate : `${root.path}/${candidate}`; + if (existsSync(join(dir, rel)) && !migrationDirs.includes(rel)) migrationDirs.push(rel); + } + } const isGitRepo = git(dir, ["rev-parse", "--is-inside-work-tree"]) === "true"; let defaultBranch = "main"; @@ -58,10 +390,6 @@ export function detect(dir) { const remoteMatch = remote.match(/[/:]([^/:]+)\/([^/]+?)(\.git)?$/); if (remoteMatch) org = remoteMatch[1]; - const migrationDirs = ["migrations", "supabase/migrations", "db/migrations", "prisma/migrations"].filter((d) => - existsSync(join(dir, d)) - ); - // Existing check workflows (by their `name:`), so the doctor knows what to // watch. Facility's own workflows are excluded — the watchtower covers them. const workflowNames = []; @@ -87,6 +415,8 @@ export function detect(dir) { isGitRepo, defaultBranch, packageManager, + ecosystems, + roots: roots.map((r) => ({ path: r.path, ecosystem: r.ecosystem, manager: r.manager })), checks, provision, org, diff --git a/packages/cli/test/detect.test.mjs b/packages/cli/test/detect.test.mjs new file mode 100644 index 00000000..2767bc05 --- /dev/null +++ b/packages/cli/test/detect.test.mjs @@ -0,0 +1,135 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join, dirname, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { pathToFileURL } from "node:url"; + +const pkgRoot = resolve(dirname(fileURLToPath(import.meta.url)), ".."); +const { detect } = await import(pathToFileURL(join(pkgRoot, "src", "detect.mjs"))); + +function makeRepo() { + return mkdtempSync(join(tmpdir(), "facility-detect-")); +} + +function write(dir, path, content) { + mkdirSync(join(dir, dirname(path)), { recursive: true }); + writeFileSync(join(dir, path), content); +} + +function pkgJson(scripts = {}) { + return JSON.stringify({ name: "x", private: true, scripts }, null, 2) + "\n"; +} + +test("single-root npm repo detects exactly as before", () => { + const dir = makeRepo(); + try { + write(dir, "package.json", pkgJson({ lint: "eslint .", test: "vitest run", setup: "docker compose up -d" })); + write(dir, "package-lock.json", "{}\n"); + const d = detect(dir); + assert.equal(d.packageManager, "npm"); + assert.deepEqual(d.checks, ["npm run lint", "npm run test"]); + assert.equal(d.provision, "npm run setup"); + assert.deepEqual(d.ecosystems, ["node"]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("pnpm workspace members are absorbed by the root, nested packages included", () => { + const dir = makeRepo(); + try { + write(dir, "package.json", pkgJson({ test: "pnpm -r test", build: "pnpm -r build" })); + write(dir, "pnpm-lock.yaml", "lockfileVersion: 9\n"); + write(dir, "pnpm-workspace.yaml", "packages:\n - packages/*\n - runner\n"); + write(dir, "packages/api/package.json", pkgJson({ test: "vitest run" })); + write(dir, "packages/web/package.json", pkgJson({ build: "next build" })); + write(dir, "runner/package.json", pkgJson({ test: "vitest run" })); + // Nested below a member: belongs to that member, never a root of its own. + write(dir, "runner/agent-clis/package.json", pkgJson({ build: "tsc" })); + const d = detect(dir); + assert.equal(d.packageManager, "pnpm"); + assert.deepEqual( + d.roots.map((r) => r.path), + ["."], + "workspace members and their nested packages must not surface as roots", + ); + assert.deepEqual(d.checks, ["pnpm run test", "pnpm run build"]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("polyglot monorepo without a root package.json finds every ecosystem", () => { + // The layout reported on issue #199: independent Poetry packages beside a + // Next.js frontend, nothing at the repository root. + const dir = makeRepo(); + try { + const poetry = [ + "[tool.poetry]", + 'name = "svc"', + "[tool.ruff]", + 'line-length = 100', + "", + ].join("\n"); + for (const svc of ["scraper", "scheduler", "backend", "mails"]) { + write(dir, `${svc}/pyproject.toml`, poetry); + write(dir, `${svc}/tests/test_smoke.py`, "def test_ok():\n assert True\n"); + } + write(dir, "frontend/package.json", pkgJson({ lint: "next lint", "test:run": "vitest run" })); + write(dir, "frontend/package-lock.json", "{}\n"); + + const d = detect(dir); + assert.equal(d.packageManager, "none", "no repository-root Node toolchain"); + assert.deepEqual(d.ecosystems, ["node", "python"]); + assert.deepEqual( + d.roots.map((r) => `${r.path}:${r.ecosystem}`).sort(), + ["backend:python", "frontend:node", "mails:python", "scheduler:python", "scraper:python"], + ); + // Checks are scoped per root and unioned — a change anywhere has a gate. + assert.ok(d.checks.includes("(cd scraper && poetry run pytest)")); + assert.ok(d.checks.includes("(cd scraper && poetry run ruff check .)")); + assert.ok(d.checks.includes("(cd frontend && npm run lint)")); + assert.ok(d.checks.includes("(cd frontend && npm run test:run)"), "test:run honored when test is absent"); + // Provisioning is per root, not one repo-wide install. + assert.ok(d.provision.includes("(cd scraper && poetry install)")); + assert.ok(d.provision.includes("(cd backend && poetry install)")); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("php and go roots detect their toolchains", () => { + const dir = makeRepo(); + try { + write(dir, "composer.json", JSON.stringify({ name: "acme/app", scripts: { test: "phpunit" } }) + "\n"); + mkdirSync(join(dir, "database/migrations"), { recursive: true }); + write(dir, "worker/go.mod", "module acme/worker\n\ngo 1.22\n"); + const d = detect(dir); + assert.deepEqual(d.ecosystems, ["go", "php"]); + assert.ok(d.checks.includes("composer run test")); + assert.ok(d.checks.includes("(cd worker && go test ./...)")); + assert.ok(d.provision.includes("composer install --no-interaction --prefer-dist")); + assert.ok(d.provision.includes("(cd worker && go mod download)")); + assert.deepEqual(d.suggestedModules, ["database"], "php migrations dir still suggests the database module"); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("fixture and sample directories never become project roots", () => { + const dir = makeRepo(); + try { + write(dir, "package.json", pkgJson({ test: "vitest run" })); + write(dir, "package-lock.json", "{}\n"); + write(dir, "samples/php/composer.json", JSON.stringify({ name: "sample/php" }) + "\n"); + write(dir, "test/fixtures/app/package.json", pkgJson({ test: "echo fixture" })); + const d = detect(dir); + assert.deepEqual(d.ecosystems, ["node"]); + assert.deepEqual(d.roots.map((r) => r.path), ["."]); + assert.deepEqual(d.checks, ["npm run test"]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); From 2a0c252e5e9c1ef8c2df298ed131201149fb04e1 Mon Sep 17 00:00:00 2001 From: Carlos Santana <1540596+ophiocus@users.noreply.github.com> Date: Mon, 31 Aug 2026 09:09:36 -0500 Subject: [PATCH 2/4] ci: retrigger verify after watchtower timeout flake The two 5s timeouts were in services/api watchtower outcome tests (DB-driven; this PR never touches that package) on a run where the serialized suite took 120s. Empty commit to re-run the checks. Co-Authored-By: Claude Opus 5 From 9452354f25bfe9d87e756451bf6004d31ef7c345 Mon Sep 17 00:00:00 2001 From: Carlos Santana <1540596+ophiocus@users.noreply.github.com> Date: Tue, 1 Sep 2026 08:51:13 -0500 Subject: [PATCH 3/4] fix(detect): shell-quote discovered paths; install nested Node roots Review round two. Discovered directory names come from repository contents and reached shell commands unquoted - the reviewer reproduced command substitution with a hostile name - so scoped commands now single-quote the path with embedded quotes escaped, pinned by a regression test using a metacharacter name and an embedded-quote name. A repository whose only Node root is nested (frontend/package.json, nothing at the root) produced no install for it while still running its checks; nested Node roots now contribute a lockfile-aware install (pnpm/yarn/npm ci, npm install without a lock) to the provision chain, covered end to end. Repository-root behavior is unchanged - the CI toolchain steps keep owning the root install. Co-Authored-By: Claude Opus 5 --- packages/cli/src/detect.mjs | 28 +++++++++++++-- packages/cli/test/detect.test.mjs | 58 ++++++++++++++++++++++++++----- 2 files changed, 75 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/detect.mjs b/packages/cli/src/detect.mjs index 5be334e1..62870311 100644 --- a/packages/cli/src/detect.mjs +++ b/packages/cli/src/detect.mjs @@ -93,12 +93,20 @@ const ADAPTERS = [ } // The toolchain step already installs Node dependencies, so provisioning // here means only the project's own extra setup. + const install = existsSync(join(dir, "pnpm-lock.yaml")) + ? "pnpm install --frozen-lockfile" + : existsSync(join(dir, "yarn.lock")) + ? "yarn install --immutable" + : existsSync(join(dir, "package-lock.json")) + ? "npm ci" + : "npm install"; const pnpmWorkspace = readText(join(dir, "pnpm-workspace.yaml")); const declared = Array.isArray(pkg.workspaces) ? pkg.workspaces : (pkg.workspaces?.packages ?? []); const workspaceGlobs = pnpmWorkspace ? pnpmWorkspaceGlobs(pnpmWorkspace) : declared; return { manager, + install, provision: scripts.setup ? `${runner} setup` : "", checks, workspaceGlobs, @@ -341,8 +349,15 @@ function detectRoots(dir) { // A command for a root below the repository root has to run there. Roots at // "." are emitted verbatim, so single-root repositories are unchanged. +// Discovered directory names come from repository contents and may legally +// contain shell metacharacters, so the path is always single-quoted (with +// embedded quotes escaped) before it reaches a shell command. +function shellQuote(value) { + return "'" + value.replace(/'/g, "'\\''") + "'"; +} + function scopeCommand(rootPath, command) { - return rootPath === "." ? command : `(cd ${rootPath} && ${command})`; + return rootPath === "." ? command : `(cd ${shellQuote(rootPath)} && ${command})`; } export function detect(dir) { @@ -361,8 +376,15 @@ export function detect(dir) { const scoped = scopeCommand(root.path, check); if (!checks.includes(scoped)) checks.push(scoped); } - if (root.provision) { - const scoped = scopeCommand(root.path, root.provision); + const provisionParts = []; + if (root.ecosystem === "node" && root.path !== "." && root.install) { + // A nested Node root is outside the CI toolchain steps' install, so its + // checks would otherwise run against an empty node_modules. + provisionParts.push(root.install); + } + if (root.provision) provisionParts.push(root.provision); + for (const part of provisionParts) { + const scoped = scopeCommand(root.path, part); if (!provisions.includes(scoped)) provisions.push(scoped); } } diff --git a/packages/cli/test/detect.test.mjs b/packages/cli/test/detect.test.mjs index 2767bc05..c6b1eef2 100644 --- a/packages/cli/test/detect.test.mjs +++ b/packages/cli/test/detect.test.mjs @@ -88,13 +88,13 @@ test("polyglot monorepo without a root package.json finds every ecosystem", () = ["backend:python", "frontend:node", "mails:python", "scheduler:python", "scraper:python"], ); // Checks are scoped per root and unioned — a change anywhere has a gate. - assert.ok(d.checks.includes("(cd scraper && poetry run pytest)")); - assert.ok(d.checks.includes("(cd scraper && poetry run ruff check .)")); - assert.ok(d.checks.includes("(cd frontend && npm run lint)")); - assert.ok(d.checks.includes("(cd frontend && npm run test:run)"), "test:run honored when test is absent"); + assert.ok(d.checks.includes("(cd 'scraper' && poetry run pytest)")); + assert.ok(d.checks.includes("(cd 'scraper' && poetry run ruff check .)")); + assert.ok(d.checks.includes("(cd 'frontend' && npm run lint)")); + assert.ok(d.checks.includes("(cd 'frontend' && npm run test:run)"), "test:run honored when test is absent"); // Provisioning is per root, not one repo-wide install. - assert.ok(d.provision.includes("(cd scraper && poetry install)")); - assert.ok(d.provision.includes("(cd backend && poetry install)")); + assert.ok(d.provision.includes("(cd 'scraper' && poetry install)")); + assert.ok(d.provision.includes("(cd 'backend' && poetry install)")); } finally { rmSync(dir, { recursive: true, force: true }); } @@ -109,15 +109,57 @@ test("php and go roots detect their toolchains", () => { const d = detect(dir); assert.deepEqual(d.ecosystems, ["go", "php"]); assert.ok(d.checks.includes("composer run test")); - assert.ok(d.checks.includes("(cd worker && go test ./...)")); + assert.ok(d.checks.includes("(cd 'worker' && go test ./...)")); assert.ok(d.provision.includes("composer install --no-interaction --prefer-dist")); - assert.ok(d.provision.includes("(cd worker && go mod download)")); + assert.ok(d.provision.includes("(cd 'worker' && go mod download)")); assert.deepEqual(d.suggestedModules, ["database"], "php migrations dir still suggests the database module"); } finally { rmSync(dir, { recursive: true, force: true }); } }); +test("discovered directory names are shell-quoted in scoped commands", () => { + // Directory names come from repository contents; a hostile name must never + // reach the shell unquoted (reviewer-reproduced command substitution). + const dir = makeRepo(); + try { + const evil = "evil$(echo pwned)"; + write(dir, `${evil}/package.json`, pkgJson({ test: "vitest run" })); + write(dir, `${evil}/package-lock.json`, "{}\n"); + write(dir, "don't/composer.json", JSON.stringify({ name: "a/b", scripts: { test: "phpunit" } }) + "\n"); + const d = detect(dir); + assert.ok( + d.checks.includes("(cd 'evil$(echo pwned)' && npm run test)"), + `metacharacters stay inert inside single quotes: ${JSON.stringify(d.checks)}`, + ); + assert.ok( + d.checks.includes("(cd 'don'\\''t' && composer run test)"), + `embedded quotes are escaped: ${JSON.stringify(d.checks)}`, + ); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test("a repository whose only Node root is nested still installs its dependencies", () => { + // frontend/package.json with nothing at the root: packageManager is "none", + // but the frontend checks must not run against an empty node_modules. + const dir = makeRepo(); + try { + write(dir, "frontend/package.json", pkgJson({ test: "vitest run" })); + write(dir, "frontend/package-lock.json", "{}\n"); + const d = detect(dir); + assert.equal(d.packageManager, "none"); + assert.ok( + d.provision.includes("(cd 'frontend' && npm ci)"), + `nested Node root installs before its checks: ${JSON.stringify(d.provision)}`, + ); + assert.ok(d.checks.includes("(cd 'frontend' && npm run test)")); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + test("fixture and sample directories never become project roots", () => { const dir = makeRepo(); try { From cbb13de5c49daa47cc947cb1cd5653d60f9d46ef Mon Sep 17 00:00:00 2001 From: Carlos Santana <1540596+ophiocus@users.noreply.github.com> Date: Tue, 1 Sep 2026 09:23:09 -0500 Subject: [PATCH 4/4] test(init): prove the nested-only Node root install end to end The #214 review asked for end-to-end coverage, not only unit coverage: init now runs against a repository whose only Node root is nested and the test asserts the install lands in the rendered artifacts - both the .facility.json manifest and the shipped crew workflow carry (cd 'frontend' && npm ci) ahead of the scoped checks. Co-Authored-By: Claude Opus 5 --- packages/cli/test/init.test.mjs | 36 ++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/packages/cli/test/init.test.mjs b/packages/cli/test/init.test.mjs index b33e245b..d82ae509 100644 --- a/packages/cli/test/init.test.mjs +++ b/packages/cli/test/init.test.mjs @@ -1,7 +1,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { execFileSync, spawnSync } from "node:child_process"; -import { lstatSync, mkdtempSync, readFileSync, writeFileSync, existsSync, rmSync } from "node:fs"; +import { lstatSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync, existsSync, rmSync } from "node:fs"; import { createHash } from "node:crypto"; import { pathToFileURL } from "node:url"; import { tmpdir } from "node:os"; @@ -153,6 +153,40 @@ test("local help and leading global flags are side-effect free", () => { } }); +test("init provisions a nested-only Node root end to end", (t) => { + // The #214 review's second half: a repository whose only Node root is + // nested (frontend/package.json, nothing at the top) must come out of init + // with that root's dependency install rendered into the shipped artifacts, + // not just detected in memory. + const dir = mkdtempSync(join(tmpdir(), "facility-nested-")); + t.after(() => rmSync(dir, { recursive: true, force: true })); + execFileSync("git", ["init", "-b", "main"], { cwd: dir }); + mkdirSync(join(dir, "frontend"), { recursive: true }); + writeFileSync( + join(dir, "frontend", "package.json"), + `${JSON.stringify({ name: "web", private: true, scripts: { test: "vitest run" } }, null, 2)}\n`, + ); + writeFileSync(join(dir, "frontend", "package-lock.json"), "{}\n"); + + const result = runCli(["init", "--yes", `--dir=${dir}`, "--org=acme", "--project=7"], dir); + assert.equal(result.status, 0, result.stdout + result.stderr); + + const manifest = JSON.parse(readFileSync(join(dir, ".facility.json"), "utf8")); + assert.ok( + manifest.provision.includes("(cd 'frontend' && npm ci)"), + `manifest provision installs the nested root: ${manifest.provision}`, + ); + assert.ok( + manifest.checks.includes("(cd 'frontend' && npm run test)"), + `manifest checks run from the nested root: ${JSON.stringify(manifest.checks)}`, + ); + const crew = readFileSync(join(dir, ".github/workflows/facility-crew.yml"), "utf8"); + assert.ok( + crew.includes("(cd 'frontend' && npm ci)"), + "the shipped crew workflow provisions the nested root", + ); +}); + test("init installs the method end to end", async (t) => { const dir = makeTargetRepo(); t.after(() => rmSync(dir, { recursive: true, force: true }));