Epic 15: monorepo root discovery, dependency install, and ephemeral test.services - #55
Merged
Conversation
make lint/format/fix/test/security now discover each language's project root from its manifest (pyproject.toml/setup.py/setup.cfg for Python, package.json for JS/TS) and run that language's tools with cwd set there, so local config (tsconfig.json, vite.config.ts path aliases) resolves correctly instead of always assuming repo root. New lib/project-discover.sh (sourced helper, mirrors the dispatch_plugin_target pattern rather than duplicating logic per HAS_<LANG> block). Optional .devrail.yml `projects:` override for layouts autodetection can't infer. Single-root projects (manifest at repo root, the common case) are unaffected — falls back to "." with byte-identical output when no per-project signal exists. Scoped to Python + JavaScript per the issue's reproduction case; Go/Rust/Ansible root-awareness and dependency-install-before-test (issue #52) are tracked as follow-on stories. Closes #53 Story 15.1
- Warn (don't silently ignore) when a projects: override path doesn't exist as a directory. - Extend tests/test-project-discover.sh with make _format/_fix/ _security integration coverage (previously only _lint/_test were automated; format/fix/security had only been checked manually). - Rewrite the test script to copy fixtures into a mktemp WORKDIR with a cleanup trap before running make targets, matching tests/test-plugin-loader.sh's convention, instead of bind-mounting tests/fixtures/ directly as a writable workspace — the direct mount let a Docker bind-mount quirk leave a stray root-owned Makefile inside tracked fixture directories. - Remove tests/fixtures/declared-lang-no-manifest/script.py, which contradicted the fixture's own "ships zero .py files" description. Story 15.1
make test now autodetects and installs a project's own dependencies before pytest/vitest run, so tests no longer fail at import time with ModuleNotFoundError/unresolved-import errors (issue #52). Python: uv.lock -> uv export | uv pip install --system; else requirements*.txt -> pip install -r; else pyproject.toml/setup.py -> pip install -e . (all system-installed, --break-system-packages — not uv sync, whose isolated venv the globally-installed pytest can't see into; caught by hand during implementation). JavaScript: package-lock.json -> npm ci. New lib/dependency-install.sh (sourced helper, mirrors Story 15.1's lib/project-discover.sh). New .devrail.yml test.install/test.setup overrides. Container image now ships uv (one line in install-python.sh — uv is PyPI-distributed, no new Dockerfile stage). A failed install fails make test immediately — the test suite never runs against a broken install (a local rc=$? placement bug initially let failures through silently; fixed by moving the exit-code capture into an explicit else branch and verified with a dedicated failing-install fixture). Scoped to uv+pip (Python) and npm (JS) per issue #52's literal FastAPI+Vue reproduction case; poetry/pipenv/pnpm/yarn are not installed in the container and are explicitly deferred. Closes #52 Story 15.2
- requirements.txt now wins outright over requirements-dev.txt (or any other requirements*.txt variant) instead of picking whichever sorts first alphabetically, which silently preferred the -dev file. - Add JS-side AC 7 regression coverage (package.json present, no package-lock.json -> no install attempted), reusing Story 15.1's monorepo-python-js fixture. - Add tests/fixtures/python-multi-requirements/ proving the requirements.txt precedence fix. - Document the repeated-yq-read-per-call trade-off as a deliberate, currently-unproblematic choice rather than a silent gap. Story 15.2
make lint/format/fix/test/security now discover Go (go.mod) and Rust (Cargo.toml) module roots the same way Story 15.1 does for Python/JS, and run golangci-lint/gofumpt/go test/govulncheck and cargo clippy/ fmt/test/audit/deny with cwd set there. This closes a real, previously-unfixed instance of issue #53: go test ./..., golangci-lint run ./..., cargo test, cargo clippy, and cargo fmt --check all failed outright ("directory prefix . does not contain main module", "could not find Cargo.toml") when the module wasn't rooted at the repo root — reproduced against real fixtures before writing any code. The epic's original assumption that Go/Rust "already resolve fine from repo root" was wrong. Ansible was in the epic's original scope for this story too, but investigation found ansible-lint already recursively discovers playbooks regardless of cwd — no change needed, so it's excluded here rather than silently given discovery logic it doesn't require. Generalizes Story 15.1's existing lib/project-discover.sh (two new ~10-line autodetect functions, no new library, no new dependency- install component — go test/cargo test already fetch their own deps automatically). Closes #53 Story 15.3
- Add tests/fixtures/go-multi-root/ and rust-multi-root/ (two module roots each) plus make _test assertions proving AC 7's multi-root claim, which the first pass never actually tested. - Add _format/_fix/_security integration coverage for the Go/Rust monorepo fixtures (previously only _lint/_test were covered) — the exact class of gap Story 15.1's own review caught, reintroduced here despite this story's Dev Notes explicitly warning against it. Story 15.3
.devrail.yml `test.services: [postgres:16, redis:7]` starts throwaway service containers before `make test` and tears them down afterward (pass or fail), injecting DATABASE_URL/REDIS_URL. Orchestration is entirely host-side. The toolchain container has no docker CLI or /var/run/docker.sock mount — deliberately, confirmed by hand before writing any code, to avoid mounting the host's Docker socket into it (a real privilege-escalation surface this feature doesn't need). A new host-side prerequisite, _test-services-up, mirrors _extended-image's existing pattern (including its local-vs- extracted-from-image script resolution, for consumer template repos that inherit this Makefile without scripts/), and feeds two new recursively-expanded flag variables into the existing shared DOCKER_RUN macro — reusing the docker_network/env plumbing from issue #48 rather than inventing a parallel path. A no-op for every target except test. Scoped to postgres:<tag>/redis:<tag> only, matching the epic's own example; anything else fails fast rather than being silently skipped. test.services and docker_network are mutually exclusive (only one --network flag reaches docker run). A SIGKILL'd run can leave orphaned containers/network behind (a trap can't catch SIGKILL); the next make test run detects and cleans up stale state automatically — verified for real by killing a run mid-flight, not just by reading the trap code. docker-compose.test.yml autodetection is explicitly out of scope. Closes #52 Story 15.4
_test-services-host-bin lacked the HAS_PLUGINS_DECLARED-style guard its own doc comment claimed to mirror from _devrail-host-bin, so every consumer repo without a local scripts/ paid a docker create/cp/rm cost on every image-tag bump even when test.services was never declared. Added a matching HAS_TEST_SERVICES_DECLARED guard on both the extraction target and _test-services-up's invocation, and hardened test:'s cleanup trap to check the extracted script's existence rather than assume it, since skipping extraction makes that no longer a safe assumption. Also rejected duplicate service kinds (e.g. two postgres:<tag> entries) during validation — previously both containers would start, but the env file's last-line-wins semantics meant the first became silently unreachable via DATABASE_URL for the whole test run. Story 15.4 code review.
CI's fresh full image build picked up typescript@7.0.2 (npm install -g
floats to latest), which fully removed the baseUrl compiler option the
Story 15.1 tsconfig.json fixture used with "bundler" moduleResolution
— tsc now errors (TS5102/TS5090) instead of just deprecation-warning.
Local development on this branch never caught this because iterative
testing used a fast lib/scripts overlay on an older pinned base image,
never a fresh full docker build.
paths entries under "bundler" resolution don't need baseUrl — made
them explicitly relative instead ("./src/*"), verified against the
real image. Re-ran the full existing regression suite (46 project-
discover, 12 dependency-install, 19 test-services, 16 plugin-resolver,
11 plugin-build-pipeline, 15 plugin-execution, 4 kotlin-plugin-
extraction) against a real `docker build .` image — all green.
… sleep(3) CI failed a different assertion than the one it was written to prove: sigkill/orphan-actually-left-behind failed on GitHub Actions even though it passed locally every time. Root cause: the fixed `sleep 3` before the kill was calibrated on a machine where the pre-container work (host-bin extraction into a cache-empty workspace: docker create + 2 docker cp + docker rm, then network create + container start) reliably finished within 3 seconds. On the CI runner it didn't — the kill fired mid-extraction, before any devrail-test-* resource existed, so there was nothing to orphan and the assertion this case exists to prove never got a chance to be true (the follow-on stale-state-detected assertion failed as a direct consequence). Replaced the fixed sleep with a poll loop that waits for actual evidence (a devrail-test-* container or network existing) up to 60s, then kills. Verified locally: 19/19 still green.
The SIGKILL rerun failed in CI (exit 2, expected 0) with no visibility into why — the test only prints PASS/FAIL summaries, never the captured `make test` log, so there's nothing to diagnose a CI-only failure from. This isn't a guess at the underlying cause; it's the missing instrumentation needed to find it without more blind round-trips. Passing locally 19/19 either way (the dump only fires on a nonzero exit).
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #52, closes #53. Four stories, implemented and code-reviewed via the formal BMad create-story/dev-story/code-review cycle (see Story 15.1-15.4 files in
OrgDocs/development-standards):lib/project-discover.sh: autodetect Python/JS/TS project roots in monorepos (pyproject.toml/setup.py/setup.cfg,package.json), run_lint/_format/_fix/_test/_securityper discovered root. Optional.devrail.ymlprojects:override.lib/dependency-install.sh: autodetect and install project dependencies (uv/pip,npm) beforemake test, withtest.install/test.setupoverrides. A failed install failsmake testimmediately.go.mod) and Rust (Cargo.toml).scripts/test-services.sh: ephemeral Postgres/Redis containers formake test, orchestrated entirely host-side (the toolchain container has no Docker socket, deliberately). InjectsDATABASE_URL/REDIS_URL. Mutually exclusive withdocker_network. Self-heals from orphaned state left by aSIGKILL'd run.Each story's own code-review pass found and fixed real issues in-session before merge — see commit messages (
fix(makefile): address Story 15.X code-review findings) for specifics.Test plan
tests/test-project-discover.sh— 46/46 (Stories 15.1 + 15.3)tests/test-dependency-install.sh— 12/12 (Story 15.2)tests/test-test-services.sh— 19/19, including a genuine mid-flightSIGKILLofmake testproving orphan detection/self-heal (Story 15.4)make check(shellcheck + shfmt) clean.github/workflows/ci.ymlfor each story🤖 Generated with Claude Code