feat(cli): detect project roots across ecosystems, not just Node - #214
feat(cli): detect project roots across ecosystems, not just Node#214ophiocus wants to merge 4 commits into
Conversation
adrian-lorenzo
left a comment
There was a problem hiding this comment.
Thanks for the substantial contribution!
scopeCommand() places discovered directory names directly into shell commands. These names come from repository contents and may legally contain shell metacharacters. I reproduced command substitution with a hostile directory name, so please shell-quote these paths and add a regression test covering the generated commands.
There is also a functional gap for nested Node roots: a repository containing only frontend/package.json produces packageManager: "none" and no provision command, while its checks still run from frontend. Please ensure dependencies are installed for nested Node roots and cover this layout end to end.
This is a strong direction; once these cases are fixed, it should be ready for another pass.
|
Both addressed in 9452354 — thank you for reproducing the substitution rather than just flagging it; that made the fix precise.
Full CLI suite: 105 tests, 0 failures on Node 24. |
|
One more commit to honor the full sentence of your review — "cover this layout end to end" deserved more than unit tests. cbb13de adds the true e2e: |
The theam#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 <noreply@anthropic.com>
`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 theam#199 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
The theam#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 <noreply@anthropic.com>
cbb13de to
b89455a
Compare
Closes #199 — and big thanks to @AndreyGritsa, whose nine-Poetry-roots-plus-Next.js layout from the issue thread is now a test case in this PR. That fixture reshaped the design: the fix isn't an adapter table alone, it's multi-root, multi-ecosystem detection, because a repository with no root
package.jsonand independent Python packages beside a frontend was previously invisible toinit.What changed
packages/cli/src/detect.mjs:detectDeploymentProvider: adding an ecosystem is one entry, not a branch. Each adapter only proposes tools the project actually configures (ruff/mypy/pytest frompyproject.toml, phpunit/pest from the files present, and so on).(cd scraper && poetry run pytest)— so a change anywhere in the repository has a gate. Provisioning is per root (poetry installin the touched package), not one repo-wide install, which a layout like Andrey's can't express.runner/agent-clis) are absorbed into their workspace root, whose scripts already fan out to them. This is what keeps existing Node repositories detecting identically to before.test/,fixtures/,samples/,examples/hold manifests on purpose; proposing their checks as repository defaults would be noise (found by running the new detection against claude-dev-kit, wheresamples/phpvolunteered its phpunit as a repo default).test:runfallback whentestis absent — the watch-free vitest convention, and literally the script name in the frontend from the issue thread.detect()also returns two new advisory fields,ecosystemsandroots(path/ecosystem/manager), whichinitcan surface in its summary; existing consumers are untouched.apps/docs/docs/faq.md: the "Does this work for non-Node projects?" answer now describes what detection actually does instead of promising only a marked slot.How it's verified
packages/cli/test/detect.test.mjs, 5 cases: single-root npm unchanged; pnpm workspace absorption incl. the nested-member case; the polyglot no-root-manifest monorepo from Stack detection covers only the Node ecosystem — every other stack starts on an empty job site #199; PHP + Go roots with migration discovery; fixture/sample dirs never become roots.main'sdetect()and this branch'sdetect()against facility's own checkout — every shared output field is identical, byte for byte. CDK's real tree confirms the fixture-skip.@AndreyGritsa — your offer to validate against the real monorepo stands very welcome: this should now discover all nine Poetry roots plus
frontend/, proposepoetry installper touched package, and unionpoetry run pytestwithnpm run test:run. I'd love to hear where it guesses wrong.