Repository navigation
Route JVM layout through one module and fix three JVM scope bugs - #1032
Merged
Merged
Conversation
vendor::jvm::layout now owns every JVM layout fact that was spelled in
several places:
- maven2 paths: group_path, version_dir, file_name, artifact_path,
registry_base/registry_url. Deletes 13 production copies of
`group.replace('.', "/")` (maven_repo, maven_crawler, jvm/mod, gradle x2,
coursier_tree, sbt owned_file, vex, jvm_jar, redirect/mod,
upstream/maven, jvm_cache x2) and redirect's local_repo_artifact_path.
- coordinates: the path guard (was maven_crawler::is_safe_maven_coordinate,
now is_path_safe) and the stricter writer grammar (was
jvm::safe_coordinates) live side by side with their relationship
documented; the two grammars are kept as they were.
- committed trees: MAVEN2_TREE, GRADLE_TREE, COURSIER_TREE, VENDOR_TREES,
ORPHAN_PATHS, CAPTURED_FILES and the per-version marker name. Deletes the
three duplicate `socket-patch.vendor.json` constants, two literals, the
CLI's own orphan list and group_commit's own captured list.
- tree index rows: one validator shared by the Gradle and Coursier indexes
(each keeps its own uuid rule).
- the `jvm` ledger ecosystem name and its maven meaning (ledger_ecosystem),
used by revert dispatch, path, redownload and hosted takeover.
- build markers: one table per tool (BuildTool::markers) and one stat rule
(marker_present: exists, following symlinks). Replaces gradle_cache's
GRADLE_MARKERS/SETTINGS, maven_crawler's SCALA_TOOL_MARKERS and its two
probes, redirect's GRADLE_ROOT_FILES, jvm/mod's GRADLE_FILES,
scala_guidance's MILL_MARKERS/SCALA_CLI_MARKERS, maven_repo's
project_has_gradle and the hand-written Gradle lists in not_build_root,
sbt and buildSrc. JVM_PROJECT_MARKERS is derived from the same constants.
The sbt reactor-wired probe now uses maven_reactor's own BEGIN_MARKER and
PIN_TAG instead of copying them.
Behavior is unchanged except the stat rule: a Gradle marker that is a
directory, or a Scala marker that is a dangling symlink, now counts the same
way everywhere (previously three different rules), and the eject snapshot
captures the whole JVM captured-file list (adding the maven2 and Gradle
tree .gitattributes it previously missed).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
A local scan, apply, rollback, vendor or VEX run in an sbt build whose only marker is project/build.properties (a build defined in project/*.scala), or a scala-cli build with only .scala-build/, found no JVM roots at all: is_jvm_project matched only the basename list, and the Scala-tool markers that list leaves out were checked only after that early return, so the Coursier and Ivy caches the contract promises were never crawled. is_jvm_project now asks layout::is_jvm_build, which covers every tool marker including the root-relative ones. Root detection by basename (JVM_PROJECT_MARKERS, scan policy and the in-memory engine) is unchanged. The regression tests pin both markers alone and every tool marker. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The v5 JVM backend records its ledger entries as `jvm`, which is not an --ecosystems name, so ecosystem_in_scope sent them down the unknown-name arm: with `--ecosystems maven` every JVM entry was out of scope. The vendor GC passes, rollback's vendored leg and repair's health loop all silently skipped them, while rollback's manifest leg (purl-based) kept the same packages in scope. ecosystem_in_scope now folds the ledger name through layout::ledger_ecosystem first, the rule revert dispatch already used. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
maven_repo::service_preflight, the download plan's gate, checked only the committed-tree hot path for the v5 JVM shapes. vendor_maven stops earlier for a Gradle or sbt subproject (not_build_root), for unsafe coordinates or an unreadable ledger, for the sbt / scala-cli resolution gate's skips and for an empty patch, so a batched grant could name uuids the vendor loop then never fetched (a leaked server-side build and quota). The preflight now runs the same jvm_prelude (the checks vendor_maven_jvm makes first, gathered in the layout commit) before it plans, and it also applies not_build_root and the empty-patch no-op. A regression test drives a Gradle subproject, an sbt build without resolution evidence and a Gradle root through both the plan and the backend and requires them to agree. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Review of the layout consolidation found copies it missed: - Two identical is_settings_file helpers (vendor/jvm/apply.rs and vendor/jvm/gradle.rs), the settings arrays in gradle.rs's applied check and maven_repo::not_build_root's ancestor loop, the Groovy/Kotlin pick in gradle.rs's target, and the inline matches! in the CLI's hosted created_settings_over_existing. All now go through layout::is_gradle_settings or layout::GRADLE_SETTINGS_FILES. - The ".scala-build" literal in sbt_evidence::SKIP_DIRS and scala_evidence::discover now uses layout::SCALA_CLI_DIR. scan/policy.rs dir_markers keeps is_file on purpose (the root-marker report lists regular files beside the non-JVM manifests); a comment says so. New layout tests pin the settings names and their Groovy-then-Kotlin order, assert that every tree-relative constant (gitattributes, gitignores, the scala-cli guard, the Maven tail dir and repo URL, the index paths) sits under its tree constant, and pin the marker stat rule's edges: a directory named build.gradle counts, a dangling symlink at pom.xml does not. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko)
force-pushed
the
arch-fix/jvm-layout
branch
from
October 7, 2026 16:09
3c335a7 to
c081a8e
Compare
Mikola Lysenko (mikolalysenko)
marked this pull request as ready for review
October 7, 2026 16:09
Picks up #1018 (CI workflow only) and re-triggers CodeQL default setup, whose dynamic run failed on runner infra and cannot be retried. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
bugbot run Generated by Claude Code |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit b441f55. Configure here.
Tanmay Singla (Tanmay182003)
approved these changes
Oct 7, 2026
5 tasks
Collaborator
Author
|
[agent] Ready for review at
Generated by Claude Code |
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
Oct 8, 2026
Resolve conflicts in vendor/path.rs (keep main's deepest-level symlink probe, use layout::ledger_ecosystem and layout::VENDOR_TREES) and vex/discover/maven.rs (take main's formats::maven imports, drop local_repo_artifact_path in favour of layout::artifact_path). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
github-merge-queue
Bot
removed this pull request from the merge queue due to a manual request
Oct 8, 2026
Pick up #1093 so PR CI runs without the macOS legs. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
4 tasks
Resolve conflicts with #1050 (vendored-entry liveness): keep its fail-closed Gradle references_checked and ledger-owned orphan check, and route both through vendor::jvm::layout (LEDGER_OWNED_PATHS replaces ORPHAN_PATHS there, re-exported from jvm::apply; settings files come from layout::GRADLE_SETTINGS_FILES). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko)
removed this pull request from the merge queue due to a manual request
Oct 8, 2026
Mikola Lysenko (mikolalysenko)
added a commit
that referenced
this pull request
Oct 8, 2026
Adopt #1032's jvm::layout module: group_commit captures through layout::CAPTURED_FILES (now also listing the Gradle script and vendor .gitattributes this PR captures) plus the derived maven-metadata paths; drop this PR's duplicate VENDOR_TREES for layout::VENDOR_TREES; the takeover reach check uses layout::LEDGER_ECOSYSTEM. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko)
added a commit
that referenced
this pull request
Oct 8, 2026
Brings in #1032 (one JVM layout module). Conflicts: - maven_repo.rs: keep this PR's router (legacy <repository> backend deleted) and port #1032 onto what survives: layout:: paths, registry URLs, coordinate guards, Gradle marker tables, MARKER_FILE and LEDGER_ECOSYSTEM; split jvm_prelude out of vendor_maven_jvm so service_preflight applies the same coordinate, ledger and sbt / scala-cli gate stops (main's preflight-parity test ported). - jvm/sbt.rs: reactor_wired keeps this PR's single-pom coverage (no declares_modules gate) with main's BEGIN_MARKER / PIN_TAG / MAVEN2_TREE. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko)
pushed a commit
that referenced
this pull request
Oct 8, 2026
Resolve the three conflicts by keeping both sides: - scan/policy.rs: keep the PR's dir_markers/lock_markers split (lock markers decide nested roots for #554) and re-apply main's #1032 change inside dir_markers: the JVM manifest fallback now reads vendor::jvm::layout::JVM_PROJECT_MARKERS, with main's is_file comment. - vex/discover/mod.rs: Discovery keeps the PR's `shadowed` refs (#828) alongside main's `read` and `withheld` evidence (#1050). Shadowed refs carry DIAG_REF_UNATTRIBUTABLE, so vendor_entry_in_use already keeps their entries through unattributable_mention. - vex/discover/testing/golden.rs: destructure all three fields. Co-Authored-By: Claude <noreply@anthropic.com>
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
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.
Problem
Architecture audit §3.B / §3.F ("Maven repo path", "Root and JVM markers"): JVM layout facts were spelled in many places and had started to drift.
group.replace('.', "/")plus the artifact leaf), 4 copies of thesocket-patch.vendor.jsonmarker name, 2 coordinate validators in different modules, separate tree-root, orphan and captured-file lists in core and CLI, two copies of the tree-index row validator, and 4 JVM build-marker tables checked with 3 different stat rules (metadata,symlink_metadata,is_file).project/build.properties(or a scala-cli build with only.scala-build/) got no JVM discovery at all.is_jvm_projectreturned early before the Scala-tool markers were consulted, which contradicts CLI_CONTRACT.jvm, which is not an--ecosystemsname. Under--ecosystems maven, every JVM entry was dropped from the vendor GC passes, from rollback's vendored leg and from repair's health loop.maven_repo::service_preflightchecked only the committed-tree hot path for JVM shapes.not_build_root, coordinate and ledger refusals, the sbt / scala-cli gate skips and empty patches all stopvendor_mavenbefore it reaches the service, so a batched grant could name uuids the loop never fetched.Change
A new
vendor::jvm::layoutmodule owns these facts, and every call site now goes through it:group_path,version_dir,version_dir_path,file_name,artifact_path,registry_base(moved frommaven_repo::maven_registry_base) andregistry_url.is_path_safe(wasmaven_crawler::is_safe_maven_coordinate) andsafe_coordinates(wasjvm::safe_coordinates). Both grammars now sit in this one module, and the doc comment explains how they relate. They are not merged: the single-pom backend accepts coordinates the v5 writers refuse, and unifying them would change behavior (see Deferred).MAVEN2_TREE,GRADLE_TREE,COURSIER_TREE,VENDOR_TREES,ORPHAN_PATHS,CAPTURED_FILES,MARKER_FILE,tree_dir, andindex_row(the shared Gradle / Coursier index row check).LEDGER_ECOSYSTEMandledger_ecosystem()(jvm→maven). Revert dispatch,path::vendor_dir_symlink, redownload, hosted takeover andecosystem_in_scopeall use it. This fixes B17.BuildTool::markers()holds one table per tool.marker_presentapplies one stat rule: the path exists, following symlinks.has_build,is_jvm_buildandis_scala_tool_buildare the shared checks.JVM_PROJECT_MARKERSis built from the same constants and is still basename-only for root detection.is_jvm_projectnow usesis_jvm_build, which fixes B65.GRADLE_SETTINGS_FILES(Groovy, then Kotlin) andis_gradle_settings(rel)replace the twois_settings_filehelpers, the settings arrays ingradle.rsandnot_build_root, the Groovy/Kotlin pick for a new settings file and the CLI hostedmatches!.SCALA_CLI_DIRreplaces the.scala-buildliterals insbt_evidenceandscala_evidence.vendor_maven_jvmmakes first into onejvm_prelude. The B18 commit makesservice_preflightrun that prelude too, and the preflight also appliesnot_build_rootand the empty-patch no-op.maven_reactor::BEGIN_MARKER/PIN_TAGinstead of copying the literals.Other behavior changes, all small:
metadata, which follows symlinks) changes two edge cases, and a layout test pins both:not_build_rootchecks usedis_file, so a directory namedbuild.gradlewas not a Gradle build. Now it is one for the crawler,m2_gate,gradle_scanandnot_build_root.symlink_metadata, so a dangling.scala-buildorproject/build.propertiessymlink made the project a Scala-tool build. Now it does not..gitattributesfiles, because it uses the shared captured-file list.project/build.propertiesand.scala-build/as JVM-root markers.Duplicates deleted (before → after)
replace('.', "/"), production code)local_repo_artifact_path/group_id_to_pathhelperslayout::artifact_path/group_path)maven_registry_base+ hand-built registry URLsregistry_urlsocket-patch.vendor.jsonconstants / literals.socket/vendor/maven2/gradle/coursier)is_settings_file×2, arrays ×2, CLImatches!, Groovy/Kotlin pick)GRADLE_SETTINGS_FILES/is_gradle_settings).scala-buildliterals (production)SCALA_CLI_DIR)Two checks keep a different stat rule on purpose:
scan/policy.rs::dir_markerskeepsis_file. The root-marker report lists regular files, and it checks the JVM build files beside non-JVM manifests (package.json,Cargo.toml, ...) under the same rule. A comment there says so. As a result, a directory namedbuild.gradlemarks a Gradle build for the crawler but is not listed as a policy root marker.sbt_evidencekeeps its regular-file check (see Deferred).Testing
Failing-first: before the fixes, all four new or changed tests failed. With the fixes, they pass.
vendor::maven_repo::tests::jvm_service_preflight_matches_what_the_backend_asks_for(B18)crawlers::maven_crawler::scala_cache_tests::local_scala_caches_only_for_scala_tool_projects(B65)crawlers::jvm_cache::tests::every_marker_makes_a_jvm_project(B65)commands::vendor::scope_and_hint_tests::jvm_ledger_entries_are_in_the_maven_scope(B17)Commands run (macOS, all via the shared heavy-job limiter,
CARGO_INCREMENTAL=0 -j4):cargo clippy -p socket-patch-core -p socket-patch-cli --all-features -- -D warnings -A unused_variables: clean.-A unused_variablesonly hides a macOS-only warning already on main,python_crawler.rs:2734 unix_default. CI runs on Linux.cargo test -p socket-patch-core --lib: 5581 passed, 1 failed. The failure isutils::digest::tests::production_digests_go_through_the_helpers, the B01 failure already on main (stalePENDING_INLINE_DIGESTSentries), which Fix main CI red on stale digest pending-list entries #1016 fixes. It is not caused by this PR.cargo test -p socket-patch-cli --lib: 862 passed.cargo clippy -p socket-patch-core -p socket-patch-cli --all-features -- -D warnings -A unused_variables: clean.cargo test -p socket-patch-core --lib -- vendor::jvm vendor::maven_repo crawlers::sbt_evidence crawlers::scala_evidence crawlers::maven_crawler: 365 passed. This includes the newlayout::tests::{gradle_settings_names, tree_relative_constants_sit_under_their_tree, marker_present_edges}.cargo test -p socket-patch-cli --bins --lib -- scan::: 248 passed.cargo test -p socket-patch-cli --test e2e_maven --test e2e_sbt_vendor --test e2e_scala_cli_vendor --test e2e_sbt_hosted --test gradle_agent_cli --test contract_gradle_codes --test vendor_jvm_cli --test e2e_socket_yml_policy: all pass.cargo test -p socket-patch-cli --test e2e_maven --test e2e_sbt_vendor --test e2e_scala_cli_vendor --test e2e_sbt_hosted --test e2e_vex_vendor --test gradle_agent_cli --test contract_gradle_codes --test cli_parse_vendor --test in_process_rollback_vendored --test covgap_commands_vendor: all pass.e2e_*_build,docker_e2e_*).Deferred
Agent-mode JVM crawl finds no Coursier cache for a scala-cli or sbt root that only SCALA_TOOL_MARKERS recognizes, because six marker lists disagree #1014 is not closed. This PR fixes its proven defects A and B (the crawl gate) and moves its marker lists into one module. It keeps the existing rule that
.mill-versionmakes a build ambiguous for sbt but is not a Mill build file for scala-cli, andsbt_evidencekeeps its regular-file check on the shared sbt constants. Settling those is the issue's "decide once" step.B63 (the JVM upstream base is Central or
SOCKET_MAVEN_REGISTRYonly). It is not small: a fix would mean reading settings.xml mirrors and pom, Gradle and sbt resolvers. This PR does give it one place to land,layout::registry_base/registry_url, which now serves every JVM upstream URL including agent-modejvm_jar.Merging the two coordinate grammars.
is_path_safeandsafe_coordinatesstill differ. Single-pom vendoring accepts coordinates that the v5 shapes refuse withunsafe_coordinates. Moving to one grammar changes behavior, which needs a decision.B62 (JVM trees invisible to orphan detection beyond the empty-ledger guard) belongs with Scan every vendored-write file for vendored references (#832, #958) #1015 / E61. This PR only consolidates the path list it would use (
layout::ORPHAN_PATHS).The tree-relative constants (
*_GITATTRIBUTES_REL,TREE_GITIGNORE_REL,GUARD_REL,TAIL_DIR,REPO_URL, the*-index.tsvpaths) are still string literals in their backend modules, becauseconcat!cannot take aconst.layout::tests::tree_relative_constants_sit_under_their_treechecks that each one sits under its tree constant, so moving a tree root fails that test.Test fixtures (
coursier_cacheandscala_cache_testshelpers,test_support/service_fixture.rs, the bench crate) still spell the maven2 path themselves on purpose, so they act as an independent oracle.Maven and NuGet are lower priority per the maintainer. The scope here is the consolidation plus B17, B18 and B65.
🤖 Generated with Claude Code
Note
Medium Risk
Large touch surface across cache crawlers and in-place Maven patching path guards, but mostly consolidation; marker stat-rule and preflight alignment changes could shift which projects are scanned or granted downloads.
Overview
Introduces
vendor::jvm::layoutas the single source for Maven2 paths, coordinate guards (is_path_safe/safe_coordinates), vendor tree constants, Gradle/scala-cli build markers, andledger_ecosystem(jvm→maven). Crawlers, vendor, scan, redirect, and CLI call sites are rewired to use it instead of duplicated helpers and marker tables.Behavior fixes: sbt/scala-cli roots with only
project/build.propertiesor.scala-build/now trigger JVM cache discovery;--ecosystems mavenincludes ledgerjvmentries; JVMservice_preflightsharesjvm_preludewithvendor_maven_jvmso grants are not issued when vendoring would refuse early. Build-marker detection uses onemetadata-based rule (directories named like Gradle files count; dangling symlink markers do not). CLI contract documents the expanded JVM root markers.Reviewed by Cursor Bugbot for commit b441f55. Configure here.
Generated by Claude Code