Repository navigation
Fix Maven 4 implicit subprojects missed outside vendored (#1809) - #1886
Mikola Lysenko (mikolalysenko) wants to merge 3 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
A Maven 4 root (model 4.1.0, pom packaging, no <subprojects> list) builds every direct subdirectory holding a pom.xml. Only the vendored planner knew this. Hosted scan pinned just the root pom while a module kept its own version, VEX attested that pin as not_affected, the sbt ambiguity guard let such a root through, and the project scope of the Maven cache skipped the modules' dependencies. Every caller now asks one maven_reactor predicate that follows the planner's own discovery rule, including the discoverSubprojects opt-out in .mvn/maven.config. The hosted engine reads each subdirectory's pom.xml beside such a root so the rewriter can refuse it as a reactor. Fixes #1809 Assisted-by: Claude Code:claude-opus-5-5
Review follow-ups for the Maven 4 reactor fix. The hosted engine, VEX and the Maven cache scope now find discovered subprojects through the planner's own probe, so a .git directory or a directory named pom.xml never makes a root a reactor in one place but not another. A refused reactor root pins nothing, so the hosted confirmation probe no longer counts a suffixed version an older release left in it, or one a module pom names, as a redirected dependency. The CLI contract now describes Maven 4 discovered reactors. Refs #1809 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
|
[burn-down agent] Labeled Ready for review at b57c4a5.
Generated by Claude Code |
Final review briefWhat it does: Maven 4 builds every direct subdirectory that has a Risk: low to medium. The logic reuses the planner's existing predicate, and the Look here
Verified: I read the full diff. The rewriter, VEX and the planner all use the same probe and exclude Changes I made: none. Open questions:
Auto-merge is armed, so approving sends this straight to the merge queue. Generated by Claude Code |
LLM Description written by Claude Code:claude-opus-5-5
Fixes #1809
Summary
Maven 4 builds every direct subdirectory holding a
pom.xmlunder a 4.1.0,pom-packaged root that lists no<subprojects>/<modules>. #1807 taught this to the vendored reactor planner. Every other place that asks "is this a reactor?" still usedmaven_reactor::declares_modules, which only sees lists that are written out. The user-visible results:<version>. It reported the dependency as redirected, and the build still used the unpatched jar.not_affectedinstead of reportingvex_maven_reactor_root.vendor_jvm_build_ambiguousguard let such a pom root sitting next tobuild.sbtthrough.Root cause and fix
The reactor rule existed only inside
Reactor::discover. It is now exposed fromvendor/jvm/maven_reactor.rs, and every caller uses it:is_reactor_root(read, list): declared lists, or Maven 4 discovery with at least one discovered subproject.may_discover_subprojects(pom, maven_config): the planner'sdiscovers_subprojects, plus themaven.project.discoverSubprojectsopt-out (now onediscovery_onhelper, shared with the planner).implicit_subprojects/implicit_subprojects_in(view): the one probe for "a direct subdirectory holding apom.xml", with.gitexcluded.Callers:
patch/redirect/mod.rsrewrite_maven_pomrefuses the root withredirect_maven_multimodule_unsupported.hosted/engine.rsread_candidate_filesreads each discovered<dir>/pom.xmlas advisory input. These files are never edited, and an unreadable one still counts as present. The rewriter needs them to decide.hosted/engine.rsconfirm: module poms never feed the needle probe. Once the root is refused as a reactor, a stale suffixed version inpom.xml/.mvn/(written by an older release) no longer confirms the dependency. Code review raised this; it also covers declared reactors from Hosted Maven mode edits only the root pom of a multi-module build, leaves child literals unpatched, and still attests VEX #261.vex/discover/maven.rs: an implicit reactor root's hosted pin is marked unattested (MavenReactorRoot).vendor/jvm/sbt.rsplan/plan_with_digestnow take aListFn, so the ambiguity guard sees discovered subprojects.vendor/jvm/mod.rsdetect/detect_buildstake aListFn, soMavenShapeclassifies correctly.crawlers/maven_scope.rs: discovered subprojects (recursively, as Maven does) seed the scope.CLI_CONTRACT.md: the hosted Maven rewriter andvex_maven_reactor_roottext now cover discovered reactors.Tests (red → green)
Each new test fails with discovery stubbed out (
may_discover_subprojectsforced tofalse) and passes with the fix. Six tests failed in the red run. The stale-pin case was checked separately by disabling the new probe filter.patch::redirect::tests::maven_4_implicit_subprojects_root_is_refused(also controls: no subproject, model 4.0.0, discovery off)hosted::engine::tests::a_maven_4_implicit_reactor_root_is_refusedvex::discover::maven::tests::hosted_pin_in_a_maven_4_implicit_reactor_root_is_not_attested(same controls)vendor::jvm::sbt::tests::detect_routes_sbt_and_keeps_existing_backends(new implicit-root case, plus a lone-root control)MavenShapevendor::jvm::tests::a_maven_4_implicit_subprojects_root_is_a_reactorcrawlers::maven_scope::tests::maven_4_discovered_subprojects_seed_the_scope(discovery on and off)Run locally:
cargo fmt --all -- --check: cleancargo clippy --workspace --all-features -- -D warnings: cleancargo test --workspace --all-features --no-fail-fast: every Maven/JVM, hosted, VEX and crawler suite passes. 11 failures, all from the sandbox and none in code this PR touches: (a) write/unremovable-file tests that can't simulate a failure as root (copy_tree::relax_loop_must_not_traverse_symlinked_root,vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry,pypi_poetry::wire_write_failure_…,pypi_requirements::wire_failure_rolls_back_…, three inin_process_redirect, two inrepair), which other agent PRs also report; (b) twoe2e_bun_lockbtests that need the live patch API (patches-api.socket.dev), blocked by the sandbox proxy (tunnel error: unsuccessful)There is no local Maven 4 e2e. The CI real-Maven legs (
e2e_redirect_maven_build,e2e_vendor_maven_build) run against this change.Review notes (code-review high)
Fixed:
maven_scope.rs.Not changed, deliberately:
sbt::legacy_pom_wiredkeepsdeclares_modules. It recognizes pre-v5 single-pom wiring, which read only the root pom and so could have wired a Maven 4 implicit root. Switching it tois_reactor_rootwould route such a legacy-wired root to sbt and orphan its<repository>wiring, whichvendor --revertmust unwind first.detect/detect_buildstake aListFnonly forMavenShape, which nothing outside tests reads today. I kept it soDetecteddoesn't misreport an implicit reactor asSingle. The extra listing happens only when the root pom is a discovering 4.1.0 pom.maven_scopereads a reactor pom a second time to check discovery. That cost is one small file read per reactor pom on the crawl path. Moving the flag intoPomModelwould mean changing the parser shared with the planner, so I left it out of this fix.https://claude.ai/code/session_011f9aeRdqEhwVwx91en466Q
Generated by Claude Code