Skip to content

Route JVM layout through one module and fix three JVM scope bugs - #1032

Merged
Mikola Lysenko (mikolalysenko) merged 11 commits into
mainfrom
arch-fix/jvm-layout
Oct 8, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 11 commits into
mainfrom
arch-fix/jvm-layout

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

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.

  • About 13 production copies of the maven2 path construction (group.replace('.', "/") plus the artifact leaf), 4 copies of the socket-patch.vendor.json marker 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).
  • B65 (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, register E69): an sbt build whose only marker is project/build.properties (or a scala-cli build with only .scala-build/) got no JVM discovery at all. is_jvm_project returned early before the Scala-tool markers were consulted, which contradicts CLI_CONTRACT.
  • B17: v5 JVM ledger entries are recorded as jvm, which is not an --ecosystems name. Under --ecosystems maven, every JVM entry was dropped from the vendor GC passes, from rollback's vendored leg and from repair's health loop.
  • B18: maven_repo::service_preflight checked 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 stop vendor_maven before it reaches the service, so a batched grant could name uuids the loop never fetched.

Change

A new vendor::jvm::layout module owns these facts, and every call site now goes through it:

  • maven2 paths: group_path, version_dir, version_dir_path, file_name, artifact_path, registry_base (moved from maven_repo::maven_registry_base) and registry_url.
  • Coordinates: is_path_safe (was maven_crawler::is_safe_maven_coordinate) and safe_coordinates (was jvm::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).
  • Committed trees: MAVEN2_TREE, GRADLE_TREE, COURSIER_TREE, VENDOR_TREES, ORPHAN_PATHS, CAPTURED_FILES, MARKER_FILE, tree_dir, and index_row (the shared Gradle / Coursier index row check).
  • Ledger name: LEDGER_ECOSYSTEM and ledger_ecosystem() (jvm → maven). Revert dispatch, path::vendor_dir_symlink, redownload, hosted takeover and ecosystem_in_scope all use it. This fixes B17.
  • Build markers:
    • BuildTool::markers() holds one table per tool.
    • marker_present applies one stat rule: the path exists, following symlinks.
    • has_build, is_jvm_build and is_scala_tool_build are the shared checks.
    • JVM_PROJECT_MARKERS is built from the same constants and is still basename-only for root detection.
    • is_jvm_project now uses is_jvm_build, which fixes B65.
  • Gradle settings and scala-cli names: GRADLE_SETTINGS_FILES (Groovy, then Kotlin) and is_gradle_settings(rel) replace the two is_settings_file helpers, the settings arrays in gradle.rs and not_build_root, the Groovy/Kotlin pick for a new settings file and the CLI hosted matches!. SCALA_CLI_DIR replaces the .scala-build literals in sbt_evidence and scala_evidence.
  • B18: the consolidation commit gathers the checks vendor_maven_jvm makes first into one jvm_prelude. The B18 commit makes service_preflight run that prelude too, and the preflight also applies not_build_root and the empty-patch no-op.
  • The sbt reactor-wired probe now uses maven_reactor::BEGIN_MARKER / PIN_TAG instead of copying the literals.

Other behavior changes, all small:

  • The single stat rule (metadata, which follows symlinks) changes two edge cases, and a layout test pins both:
    • A directory named like a build file now counts. Before, the Gradle and not_build_root checks used is_file, so a directory named build.gradle was not a Gradle build. Now it is one for the crawler, m2_gate, gradle_scan and not_build_root.
    • A dangling symlink at a marker no longer counts. Before, the Scala-tool gate used symlink_metadata, so a dangling .scala-build or project/build.properties symlink made the project a Scala-tool build. Now it does not.
  • The eject snapshot now covers the maven2 and Gradle tree .gitattributes files, because it uses the shared captured-file list.
  • The contract text now names project/build.properties and .scala-build/ as JVM-root markers.

Duplicates deleted (before → after)

What Before After
maven2 path construction (replace('.', "/"), production code) 14 1
local_repo_artifact_path / group_id_to_path helpers 3 0 (layout::artifact_path / group_path)
maven_registry_base + hand-built registry URLs 1 + 5 1 + registry_url
socket-patch.vendor.json constants / literals 4 + 2 1 (state's, re-exported)
Coordinate validators 2, in 2 modules 2, in 1 module (grammars unchanged)
Tree index row validators 2 1
Tree-root literals (.socket/vendor/maven2 / gradle / coursier) 9 3 constants
Orphan path lists (core + CLI) 2 1
Captured-file lists (core + group_commit) 2 1
JVM marker tables (JVM_PROJECT_MARKERS, SCALA_TOOL_MARKERS, GRADLE_MARKERS/SETTINGS, GRADLE_ROOT_FILES, jvm GRADLE_FILES, MILL/SCALA_CLI_MARKERS, project_has_gradle, not_build_root / sbt / buildSrc lists) about 11 1 module
Gradle settings-file name checks (is_settings_file ×2, arrays ×2, CLI matches!, Groovy/Kotlin pick) 6 1 (GRADLE_SETTINGS_FILES / is_gradle_settings)
.scala-build literals (production) 2 0 (SCALA_CLI_DIR)
Build-marker stat rules 3 1, plus 2 deliberate exceptions (see below)

Two checks keep a different stat rule on purpose:

  • scan/policy.rs::dir_markers keeps is_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 named build.gradle marks a Gradle build for the crawler but is not listed as a policy root marker.
  • sbt_evidence keeps 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_variables only 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 is utils::digest::tests::production_digests_go_through_the_helpers, the B01 failure already on main (stale PENDING_INLINE_DIGESTS entries), 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.
  • After the review fixes (head c081a8e):
    • 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 new layout::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.
  • Not run, left to CI: the real-build-tool and docker suites (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-version makes a build ambiguous for sbt but is not a Mill build file for scala-cli, and sbt_evidence keeps 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_REGISTRY only). 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-mode jvm_jar.

  • Merging the two coordinate grammars. is_path_safe and safe_coordinates still differ. Single-pom vendoring accepts coordinates that the v5 shapes refuse with unsafe_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.tsv paths) are still string literals in their backend modules, because concat! cannot take a const. layout::tests::tree_relative_constants_sit_under_their_tree checks that each one sits under its tree constant, so moving a tree root fails that test.

  • Test fixtures (coursier_cache and scala_cache_tests helpers, 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::layout as the single source for Maven2 paths, coordinate guards (is_path_safe / safe_coordinates), vendor tree constants, Gradle/scala-cli build markers, and ledger_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.properties or .scala-build/ now trigger JVM cache discovery; --ecosystems maven includes ledger jvm entries; JVM service_preflight shares jvm_prelude with vendor_maven_jvm so grants are not issued when vendoring would refuse early. Build-marker detection uses one metadata-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

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>
@mikolalysenko Mikola Lysenko (mikolalysenko) added the arch-refactor PR opened by the scheduled architecture refactor routine label Oct 7, 2026
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>
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>
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.

@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator Author

[agent] Ready for review at b441f55940ffa30c60ce213b490f02bfe4fff835.

  • CI: required ci-ok green. 532 success / 7 skipped / 0 failing of 564 check runs. 25 non-required macOS/Windows native / install-proof legs are still queued on the runner backlog.
  • Bugbot reviewed b441f55940 with no unresolved findings; no open review threads.
  • Mergeable, no conflicts.
  • Reviewer focus: the consolidated maven2 path helper and the --ecosystems maven handling of v5 jvm ledger entries (B17).

Generated by Claude Code

@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 7, 2026
@github-merge-queue
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>
@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 8, 2026
@github-merge-queue
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>
Resolve conflicts with #1038 (shared repo-root walk, Option m2_repo) and
#1045; route #1035's superseded-checksum path and the hosted engine's
Gradle root files through vendor::jvm::layout.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@mikolalysenko Mikola Lysenko (mikolalysenko) removed the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 8, 2026
@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 8, 2026
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>
@mikolalysenko
Mikola Lysenko (mikolalysenko) removed this pull request from the merge queue due to a manual request Oct 8, 2026
@mikolalysenko
Mikola Lysenko (mikolalysenko) added this pull request to the merge queue Oct 8, 2026
Merged via the queue into main with commit 9472be4 Oct 8, 2026
455 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the arch-fix/jvm-layout branch October 8, 2026 13:15
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-refactor PR opened by the scheduled architecture refactor routine

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants