[Mikola Lysenko (mikolalysenko)]

Vendored JVM (Maven reactor) liveness attested in cases where hosted mode and VEX discovery don't. The owner rule says hosted's stricter behavior is ground truth, so this PR brings vendored mode in line.

What attested wrongly, and the fix

J2: executable classifier and type copies. The project could also declare an executable copy of the patched GA: <classifier>tests</classifier>, a native classifier, or <type>test-jar</type>. The planner only warned (or, for typed copies, skipped silently), so the entry still attested. The classifier-less pin doesn't retarget the copy, and the vendored tree serves only the main jar, so Maven fetches the public release and its code runs.

• Now a used (non-dependencyManagement) executable copy is uncertain, and so not attestable, unless every pom that reads it resolves the suffixed version. That check uses the same inheritor/profile override check as the main jar.
• The rule is shared in formats::maven:
  • is_main_jar: untyped or jar, no classifier.
  • is_executable_variant: a type counts as the classifier it implies (test-jar → tests, ejb-client → client, java-source / javadoc → non-executable). Every other type except pom is executable.
  • is_executable_classifier: everything except sources / javadoc. Hosted mode and discovery use it too.
• Vendored mode is stricter than discovery in one case: a copy inside a <profile> also blocks attestation, because the profile may be active.

J8: unscoped "wired" check. maven_reactor::wired_checked searched every reactor pom's text for the suffixed version. A suffixed version that appeared only in a build plugin's dependencies, a <profile>, an unused property, another artifact's declaration or a classifier copy kept the entry wired.

• Now "wired" needs a <dependencies> or <dependencyManagement> declaration outside <profiles> that names the main jar at the suffixed version. Doc::declarations already excludes <build> and <reporting>.
• ${...} is interpolated in the declaring pom and in every pom that inherits it, as Maven interpolates after inheritance.
• A version-less declaration must also read the pin from the nearest local <dependencyManagement> on its chain. One pinned local root no longer proves an unpinned sibling root.
• vex, vendor --check and the planner's refuse_unwired use this check.

J15: fail-open on read errors. entry_wired was unwrap_or(true), so an unreadable or unparseable pom made Discovery::vendor_entry_live report a JVM entry as live. It now fails closed.

GC, the takeover classifier and vendor --check

• scan --prune GC: unchanged on purpose. It uses entry_references, which still answers "keep" on a read error. The old text search stays as maven_reactor::references_checked for the GC only, because the .mvn/maven.config tail repository also serves plugin resolution: a tree a <build> plugin or a profile still names must not be reverted out from under the build.
• scan's overlap takeover classifier: now uses a tri-state: apply::entry_liveness, Discovery::vendor_entry_liveness and LedgerLiveness::vendor_entry_state. "Unknown" is not "dead": an unreadable pom, or a tree still referenced without attestable wiring, gives None. The classifier stays silent then, instead of emitting redirect_supersedes_vendored ("remove ") for an entry the GC keeps.
• vendor --check: now agrees with vex. An intact, wired entry that isn't attestable stays verified, and the run gets the new warning vendor_jvm_unattested, which names the declarations. The new code is documented in CLI_CONTRACT.md. It's a warning, not a failure, because of the contract's unattested-references rule: liveness gates keep such wiring live, and re-vendoring cannot clear it.

Other behavior changes

• vendor's kept-older-patch rule: an unreadable kept entry now means the unserved upgrade fails as vendor_prebuilt_required instead of being a benign skip.
• refuse_unwired: a plan whose only wiring is a rewritten profile declaration, with no root pin, is refused as nothing_wired.
• A typed executable copy now adds a classifier_declared degraded warning to the plan.

Review findings (/code-review high)

All nine are confirmed and fixed. Each behavior test fails when its fix is neutralized.

1. test-jar skipped before the classifier check. Fixed with the shared type rule. plugin_dependencies_exclusions_and_other_types_are_untouched pinned the old silent pass-through; it now expects executable types to warn and not attest, while pom and java-source still pass silently.
2. Takeover regression on undecided claims. Fixed with the tri-state. Test: maven_takeover_stays_silent_on_an_undecided_vendored_claim (CLI), with a superseded control case.
3. Classifier copy version never override-checked. Fixed. Test: a_classifier_copy_a_reader_moves_back_to_the_base_is_not_attestable.
4. One pinned root proved an unpinned sibling root live. Fixed with the version-less management check. Test: a_version_less_declaration_under_an_unpinned_root_is_not_attestable.
5. vendor --check ignored J2. It now runs unattestable_checked and warns vendor_jvm_unattested. Test: vendor_check_reports_an_unattestable_entry.
6. Interpolation only in the declaring pom. Fixed by also interpolating in inheritors. Test: a_parent_declaration_its_modules_resolve_to_the_pin_is_wired. attestable_checked still treats a parent declaration whose ${...} the parent itself can't resolve as uncertain (property_unresolved); that rule predates this PR and I left it strict.
7. Duplicate version interpolation. Now uses Doc::coordinate.
8. Copy-pasted main-jar predicate. Extracted to Doc::is_main_jar / formats::maven::is_main_jar.
9. Uncalled maven_reactor::wired. Removed.

Tests

• Results:
  • cargo test -p socket-patch-core --lib -- vendor::jvm:: vex::discover:: formats::maven: 710 passed
  • golden committed_fixture_corpus_matches_golden: passes, output unchanged
  • -- vendor:: patch::redirect:: utils::purl_key: 3756 passed
  • CLI lib -- takeover commands::vendor::: 76 passed
  • --test vendor_jvm_cli: 54 passed
  • --test maven_sidecar_cli: 28 passed
  • --test e2e_vendor_jvm_build: 35 passed, 6 ignored
  • --test e2e_vendor_maven_build: 4 passed, 1 ignored
• cargo clippy --locked --workspace --all-features -- -D warnings and cargo fmt --all -- --check are clean.

Review findings, round 2 (/code-review high on e0c351f+98716dd, cloud routine)

1. Hosted mode and discovery ignored <type>-implied executable copies: confirmed and fixed. The vendored reactor treats a type like test-jar (that is, the tests jar), ejb-client, ejb, bundle or an unknown type as an executable copy. The hosted rewriter and VEX discovery only looked at <classifier>. So a test-jar declaration at the base release was pinned by hosted mode and attested by discovery, even though Maven puts the unpatched tests jar on the classpath. All three now share one rule: formats::maven::is_variant / variant_label and is_executable_variant.

   • Discovery's main-jar version grouping now skips such variants.
   • Discovery's management lookup is keyed by type as well as classifier.

   Tests: maven_executable_classifier_variant_blocks_the_redirect (hosted) and executable_classifier_copy_off_the_pin_is_not_attested (discovery) both gain a test-jar case.

   • Behavior change: hosted mode no longer redirects a GA that has an executable type variant at the patched release, and it warns redirect_maven_classifier_unsupported.

2. The kept-patch rule asked the attestation question: confirmed and fixed. keep_older_vendored_patch used vendor_entry_live. An older JVM vendoring that was still wired but not attestable (for example, an executable classifier copy beside it) therefore gave way, and every re-run failed with vendor_prebuilt_required. The older patch is now kept unless vendor_entry_liveness proves it dead. There is no new CLI test: the path needs a full JVM vendor-and-upgrade fixture. It relies on entry_liveness, which is already covered.

Declined:

• vex reports vendor_unwired without the reactor's reason for a wired-but-unattestable JVM entry. This affects the note only, and the entry is still withheld. The fix needs a new UnattestedKind path for JVM entries, which is a follow-up.
• A mixed Maven+Gradle entry's Gradle-half reason. This is a report-only gap.
• classifier_declared for sources/javadoc is reported as degraded. That's a code/category nit.
• The <pom> vs <pom>:<line> mix, repeated reactor parses, inheritor walks and a dead match arm. These are report format, performance and style issues.

Tests (cloud run): cargo test -p socket-patch-core --lib -- formats::maven vex::discover:: patch::redirect:: vendor::jvm gives 1581 passed, golden included. The 1 failure is the chmod test, which root ignores. CLI --lib -- vex_sources vendor and --test vendor_jvm_cli pass. clippy and fmt are clean.

Landing note: is_executable_classifier and is_executable_variant overlap with #2005, which also adds is_executable_classifier. When landing #2005 after this PR, keep this PR's shared helpers.

---

Note

Medium Risk
Changes security-relevant attestability and liveness semantics for Maven vendored patches, with broad impact on scan, vendor upgrade, and check flows; behavior is heavily tested but misclassification could affect prune vs takeover guidance.

Overview
Vendored Maven reactor entries no longer count as fully attested when the build can still pull unpatched bytes, matching hosted redirect and VEX discovery. Wiring now requires a main-jar declaration Maven always reads (not plugin deps, profiles, or plain text matches), with stricter handling for executable classifier/test-jar copies, version-less deps under unpinned roots, and property/profile overrides.

entry_wired fails closed on unreadable POMs, and a new entry_liveness tri-state (Some(true|false) vs None) drives scan's overlap takeover classifier and the kept-older-patch rule so undecided claims are not treated as dead (GC still uses the broader entry_references).

vendor --check keeps such entries verified but emits warning vendor_jvm_unattested with declaration locations. Shared helpers in formats::maven unify main-jar vs executable-variant logic across redirect rewrite, reactor planning, and VEX discovery (including <type>-implied variants like test-jar). CLI_CONTRACT.md documents the new warning.

^{Reviewed by Cursor Bugbot for commit 11e501d. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

• ✅ Fixed: Version-less pin ignores profile overrides
  • Added managed_version_can_move() to detect when profiles or inheritors provide different managed versions, preventing unpatched JARs from reaching the classpath undetected.

Or push these changes by commenting:

@cursor push 911c1aa479

Preview (911c1aa479)

diff --git a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs
--- a/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs
+++ b/crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs
@@ -1808,6 +1808,68 @@
             .flatten()
     }
 
+    /// Whether a profile or inheritor can provide a different managed
+    /// version for `g:a` than what `rel` reads from top-level management,
+    /// either through profile property redefinition or profile-specific
+    /// `<dependencyManagement>` entries.
+    fn managed_version_can_move(&self, rel: &str, g: &str, a: &str, base: &str) -> bool {
+        // If the base managed version uses properties, check if profiles
+        // or inheritors redefine them to produce a different version.
+        if base.contains("${") {
+            if self
+                .version_moved(rel, base, &|v: &str| v != base)
+                .is_some()
+            {
+                return true;
+            }
+        }
+
+        // Check if any profile or inheritor has management that provides
+        // a different version (profile management or inheritor management).
+        std::iter::once(rel)
+            .chain(self.inheritors(rel))
+            .any(|reader| {
+                self.chain(reader).any(|at| {
+                    let doc = &self.poms[at].doc;
+
+                    // Check profile management entries
+                    let profile_manages_differently = doc.declarations().into_iter().any(|dep| {
+                        !doc.is_top_level_managed(dep) && doc.is_main_jar(dep) && {
+                            let resolve = |v: &str| self.interpolate(reader, v, 0);
+                            doc.coordinate(dep, "groupId", &resolve).as_deref() == Some(g)
+                                && doc.coordinate(dep, "artifactId", &resolve).as_deref() == Some(a)
+                                && doc.coordinate(dep, "version", &resolve).as_deref() != Some(base)
+                        }
+                    });
+
+                    if profile_manages_differently {
+                        return true;
+                    }
+
+                    // For inheritors, check if their top-level management differs
+                    if reader != rel {
+                        let inheritor_manages_differently =
+                            doc.declarations().into_iter().any(|dep| {
+                                doc.is_top_level_managed(dep) && doc.is_main_jar(dep) && {
+                                    let resolve = |v: &str| self.interpolate(reader, v, 0);
+                                    doc.coordinate(dep, "groupId", &resolve).as_deref() == Some(g)
+                                        && doc.coordinate(dep, "artifactId", &resolve).as_deref()
+                                            == Some(a)
+                                        && doc.coordinate(dep, "version", &resolve).as_deref()
+                                            != Some(base)
+                                }
+                            });
+
+                        if inheritor_manages_differently {
+                            return true;
+                        }
+                    }
+
+                    false
+                })
+            })
+    }
+
     /// Whether every pom that reads `dep` of `rel` resolves its version
     /// to `sv`: `rel` and each pom inheriting it, with or without a
     /// profile on its chain active (see [`Self::version_moved`]).
@@ -2050,13 +2112,20 @@
                 // A version-less declaration reads its management: under a
                 // local root left unpinned (management conflicting or
                 // unresolved outside the checkout) that may be the base.
-                if !managed_section
-                    && self
-                        .managed_version(rel, patch.group_id, patch.artifact_id)
-                        .as_deref()
-                        != Some(sv)
-                {
-                    uncertain.insert(at);
+                if !managed_section {
+                    let managed = self.managed_version(rel, patch.group_id, patch.artifact_id);
+                    if managed.as_deref() != Some(sv)
+                        || managed.as_ref().is_some_and(|mv| {
+                            self.managed_version_can_move(
+                                rel,
+                                patch.group_id,
+                                patch.artifact_id,
+                                mv,
+                            )
+                        })
+                    {
+                        uncertain.insert(at);
+                    }
                 }
                 continue;
             };

_{You can send follow-ups to the cloud agent here.}

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

Ready for review at 98716dd.

• CI: 30/32 non-skipped checks green. The only failures are the main-wide e2e … pdm:: --ignored leg (module 'hishel' has no attribute 'SyncSqliteStorage', also red on main; Fix pdm e2e red on every PR: pin hishel<2 #2028 fixes it) and the ci-ok aggregate that follows from it.
• Bugbot: reviewed 98716dd; no unresolved findings.
• Promoted from draft by the burn-down agent this run. No Slack announcement was sent this run (the Slack send action wasn't available), so there's no slack-announced marker yet.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief (for the human approver)

What it does: this PR tightens liveness and attestation for vendored Maven reactor entries.

• Wired: "wired" now needs a <dependencies> or <dependencyManagement> declaration of the main jar at the suffixed version, outside profiles and <build>/<reporting>. Before, any text match in a pom counted.
• Blocks attestation: executable classifier or type copies (tests, test-jar, ejb, ...), and version-less declarations whose root management doesn't always resolve to the pin.
• Fails closed: entry_wired now fails closed on read errors.
• Unchanged: the scan --prune GC keeps its old fail-open text search, now named references_checked.

Risk: low-medium. Almost every change makes attestation stricter, so the likely failure is losing an attestation, not a false one. Deletion paths (GC, entry_in_use) keep their old semantics. The planner discards uncertain, so the new checks can't change a plan; they only affect attestation and a new vendor --check warning, vendor_jvm_unattested.

Look here

• maven_reactor.rs:1763 wires
• maven_reactor.rs:1817 always_managed_at: version-less declarations
• maven_reactor.rs:2034-2062:```` executable variants marked uncertain
• apply.rs:1003-1019 and 1226-1240: fail-closed entry_wired and check_entry
• scan/mod.rs:1073-1086: the takeover tri-state

Verified

• I read the full diff (9 files) and checked every caller of entry_wired, vendor_entry_live and entry_liveness.
• references_checked has the same body as the old wired_checked, and moving NON_EXECUTABLE_CLASSIFIERS to formats::maven doesn't change hosted or discovery semantics.
• The new tests reach every new branch. CLI_CONTRACT.md documents the new code. CHANGELOG.md is untouched.
• CI: 30/32 non-skipped checks green. The only failures are the main-wide pdm/hishel leg and the ci-ok aggregate; Fix pdm e2e red on every PR: pin hishel<2 #2028 fixes both.
• Bugbot: clean on this head.

Changes I made: none.

Open questions (non-blocking)

• Hard failure on an older ledger shape: vendor --check now hard-fails an older ledger entry whose only wiring is a rewritten profile declaration with no root pin ("no longer wired"), and re-vendoring refuses nothing_wired. The contract says unattested references warn. This is rare, but the message misleads.
• Kept-patch rule wider than described: the kept-older-patch rule (vendor.rs:179) also stops protecting a kept entry that is wired but not attestable under the new rules, not only the unreadable case the description names.
• Perf: Reactor::wires recomputes inheritors(rel) for every main-jar declaration before comparing coordinates. A literal group/artifact prefilter would avoid that.

Auto-merge is not armed yet, because CI is red on the main-wide pdm failure. Once #2028 lands and this goes green, the next final-reviewer run arms it, and approving then sends it straight to the merge queue.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

bugbot run

---

Generated by Claude Code

[cursor]

✅ 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 11e501d. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Re-enqueued by the final reviewer (head 11e501d7). The merge queue removed this PR at 01:55 UTC when the burn-down merged main at 7bacb8b. The PR is still approved, ci-ok and clippy are green on the new head, it is mergeable, and it has no unresolved threads. Approvals stay valid across later pushes, so it went back into the queue at 02:49 UTC.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Re-labeled Ready for review by the burn-down agent after resolving merge conflicts with main.

• Head: 11e501d7 (merged main 7bacb8b)
• CI: 31/31 green (19 skipped)
• Bugbot: reviewed 11e501d7, no new issues
• Conflict: vendor/jvm/apply.rs entry_wired. Main (Stop attesting sbt pins a build reassignment or uncommitted wiring bypasses #2030) split entry_wired_checked into entry_wiring_checked(.., attest) and kept entry_wired fail-open with attest=false. I kept this PR's fail-closed entry_wired_checked(..).unwrap_or(false) and entry_liveness. With the attest variant, an sbt/scala-cli attestability Err now reads as not wired rather than wired, which is the fail-closed direction this PR wants. Worth a look from the reviewer.
• Slack announcement could not be sent from this run.

---

Generated by Claude Code