[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-opus-5-5

Refs #513 (partial: no false vex and no misleading pin; vendor still counts the patch as applied when nothing is wired, so #513 stays open for that)
Fixes #459

Both are p3 (Maven). I took them this run because every p1/p2 issue is already claimed, waiting on agent:needs-human, or covered by an open PR (#549 → #1698). Both are false-VEX correctness bugs that share one code path.

Root cause

The v5 Maven reactor planner (vendor/jvm/maven_reactor.rs, Reactor::rewrite_declarations) handled a ${prop} version it could not resolve with certainty as if pinning were safe. vex liveness then attested whenever any pom held the suffixed version.

• Vendored Maven reactor reports applied and VEX attests not_affected when a ${property} version is left unresolved, so the build keeps Central's unpatched jar (also triggered by Maven 4 <parent/> inference) #513: when the planner can't find ${prop} in the checkout (an external parent defines it, or a Maven 4 <parent/> is never inferred), the property_unresolved branch warned and continued, but it left the local root pinned. The module's explicit version overrides that pin, so the build kept Central's jar. Meanwhile vendor reported applied, and vex attested not_affected because the pin's suffixed version sits in the root pom.
• Vendored Maven reactor ignores profile <properties>, so a version an active profile raises is silently downgraded to the patched base (1.11.0 → 1.10.0-socket.*) with exit 0 and no warning #459: Pom::parse read only top-level <properties>, so a profile override was invisible. ${prop} was rewritten to the suffixed base, which silently downgraded the version the profile selects.

Changes

• An undefined version property now leaves the local root unpinned (a property_unresolved warning that says so). This matches the existing conflicting_literal_version handling. The same applies to a possibly-matching declaration whose ${...} groupId/artifactId is undefined and that carries its own version.
• Profile <properties> are parsed. One walk (version_override) checks every pom that reads the declaration, meaning the declaring pom and each inheritor. It covers both an inheritor's own properties (existing behavior) and every profile on that pom's chain layered over its pom. If any of them moves the version off the base, the declaration is left as is with conflicting_literal_version and the root is unpinned. .mvn/maven.config user properties still beat profiles.
• A model 4.1.0 <parent/> with no coordinates resolves to ../pom.xml, so a property defined in the checkout's root is found and patched (Vendored Maven reactor reports applied and VEX attests not_affected when a ${property} version is left unresolved, so the build keeps Central's unpatched jar (also triggered by Maven 4 <parent/> inference) #513 case 2). A Maven 4 child's ${project.version} / ${project.groupId} come from that inferred parent. Inference is limited to model 4.1.0.
• vex liveness for a reactor (apply::entry_wired_checked → new maven_reactor::attestable_checked) re-plans the declarations. It withholds when any declaration is left at a version that may still read the base. Without this, a rewritten sibling module kept attesting. wired_checked itself is unchanged, because the prune GC and --check use it to decide whether a tree is still referenced, so a referenced tree is never pruned.

Tests (red → green)

Issue	Test	Red on main
#513 case 1	maven_reactor::tests::a_property_from_an_external_parent_leaves_the_root_unpinned	yes
#513 case 1	unresolvable_property_and_range_warn (its "still pinned" assertion encoded the bug and now asserts unpinned)	yes
#513 case 2	a_maven_4_inferred_parent_is_local	yes
#513 (coords)	an_unresolved_coordinate_with_a_version_leaves_the_root_unpinned	added after review
#513 vex	an_uncertain_module_beside_a_rewritten_one_is_not_attestable	added after review
#459	a_profile_overriding_the_version_property_is_left_alone	yes
#459	a_profile_up_the_chain_or_on_a_nested_property_is_weighed (parent profile; profile on a nested property)	yes
#459	an_inheriting_modules_profile_keeps_the_managed_property	yes
no false positive	a_profile_agreeing_with_the_base_or_beaten_by_maven_config_is_rewritten	passes on both
Maven 4 scope	maven_4_parent_inference_is_4_1_only_and_carries_the_version	added after review
Bugbot (profile → undefined name)	extra case in a_profile_up_the_chain_or_on_a_nested_property_is_weighed	added after Bugbot
#513 + #459 end to end	vendor_jvm_cli::maven_reactor_uncertain_property_version_withholds_vex (vendor exits 0 with the warning and leaves the poms untouched; vex skips with vendor_unwired, including when a sibling module at the literal base was rewritten. That sibling case attests not_affected if attestable_checked is reverted)	yes (failed at the "pinned" assertion)

Commands run locally:

• cargo test -p socket-patch-core --all-features --lib vendor::jvm: 240 passed. On the test-only commit (253ab73), 6 of the new or updated unit tests failed.
• cargo test -p socket-patch-cli --all-features --test vendor_jvm_cli: 52 passed. Red on 253ab73.
• cargo fmt --all -- --check and cargo clippy --workspace --all-features -- -D warnings: clean.
• cargo test -p socket-patch-core --all-features --lib: 6871 passed. The 4 failures are chmod-based write-failure tests in untouched modules (copy_tree, vlt_heal, pypi_poetry, pypi_requirements) that can't fail as intended because this container runs as root. A full cargo test --workspace build would not fit this container's disk allowance, so CI covers the remaining suites.
• Real Maven 3.9.11 e2e (SOCKET_PATCH_MAVEN_E2E_REQUIRED=1): e2e_vendor_jvm_build maven reactor tests (2) and e2e_vendor_maven_build (1) pass.

No wrapper (npm/, pypi/, gem/) changes are needed. They only dispatch to the binary.

/code-review high findings not fixed

• Profiles evaluated one at a time. An override that appears only when two profiles are active together (parent profile A plus module profile B) is missed. Evaluating every subset adds cost and code for a rare layout. Every single-profile override is caught.
• A property defined only inside a profile. It is reported as property_unresolved ("not defined in the checkout") and the root is unpinned. That is conservative: before, the pin was overridden by the explicit version anyway, and vex attested falsely.
• Unpinning the whole local root drops pin coverage for other modules. This is intentional and matches conflicting_literal_version. A pin that one module's explicit version overrides can't be trusted, and vex must not attest from it.

Known gaps (added by the final reviewer)

• vendor still reports applied when it leaves the root unpinned. In the no-sibling case (the only declaration is uncertain), vendor exits 0 with summary.applied: 1 and writes no pin, and an immediate vendor --check exits 1 with "vendored artifact is no longer wired into the build". Vendored Maven reactor reports applied and VEX attests not_affected when a ${property} version is left unresolved, so the build keeps Central's unpatched jar (also triggered by Maven 4 <parent/> inference) #513 asks for vendor to report the patch as not applied; this PR does not do that. The existing conflicting_literal_version path on main behaves the same way.
• The liveness change is not vex-only. entry_wired_checked feeds Discovery::vendor_entry_live, so vendor's keep_older_vendored_patch (an older, partly wired vendoring is no longer kept) and scan's takeover warnings also see the stricter answer. wired_checked (prune GC, vendor --check) is unchanged.

https://claude.ai/code/session_011f9aePMHGzPvTJK2MCHChi

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.

Autofix Details

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

• ✅ Fixed: VEX ignores uncertain Maven declarations
  • Changed vex_sources to gate on both Err and Ok(false) from entry_wired_checked, preventing attestation when Maven declarations may still read the base due to unresolved properties.

Or push these changes by commenting:

@cursor push b856788bb6

Preview (b856788bb6)

diff --git a/crates/socket-patch-cli/src/commands/vex_sources.rs b/crates/socket-patch-cli/src/commands/vex_sources.rs
--- a/crates/socket-patch-cli/src/commands/vex_sources.rs
+++ b/crates/socket-patch-cli/src/commands/vex_sources.rs
@@ -464,16 +464,26 @@
             .as_ref()
             .filter(|e| socket_patch_core::vendor::jvm::apply::is_jvm_entry(e))
         {
-            if let Err(detail) =
-                socket_patch_core::vendor::jvm::apply::entry_wired_checked(root, entry)
-            {
-                const CODE: &str = "vendor_jvm_shape_unsupported";
-                gated.push(failed(&cand.key, CODE));
-                notes.push(note(
-                    CODE,
-                    format!("{}: cannot establish JVM wiring: {detail}", cand.key),
-                ));
-                continue;
+            match socket_patch_core::vendor::jvm::apply::entry_wired_checked(root, entry) {
+                Err(detail) => {
+                    const CODE: &str = "vendor_jvm_shape_unsupported";
+                    gated.push(failed(&cand.key, CODE));
+                    notes.push(note(
+                        CODE,
+                        format!("{}: cannot establish JVM wiring: {detail}", cand.key),
+                    ));
+                    continue;
+                }
+                Ok(false) => {
+                    const CODE: &str = "vendor_jvm_uncertain";
+                    gated.push(failed(&cand.key, CODE));
+                    notes.push(note(
+                        CODE,
+                        format!("{}: Maven declaration may still read the base", cand.key),
+                    ));
+                    continue;
+                }
+                Ok(true) => {}
             }
         }
         let basis = if let Some(vref) = cand

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

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

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 4c1b5b2. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review at 4c1b5b2 (burn-down agent).

• CI: 50/50 check runs on 4c1b5b2 green or skipped (31 success, 19 skipped).
• Bugbot: reviewed 4c1b5b2, no new issues; both earlier threads (jvm/apply.rs, jvm/maven_reactor.rs) resolved.
• Reviewer focus: maven_reactor::attestable_checked now withholds VEX for a reactor whose declarations may still read the base version; wired_checked (prune GC / --check) is intentionally unchanged.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Ready for review at 4c1b5b2 (burn-down agent).

• CI: 50/50 check runs on 4c1b5b2 green (31 success, 19 skipped), including ci-ok.
• Bugbot: reviewed 4c1b5b2 (success). Of its two earlier findings, one was confirmed and fixed in 4c1b5b2, and one was answered as a false positive. Both threads are resolved.
• Mergeable against main. No CHANGELOG.md change.
• Slack announcement could not be sent from this run (the Slack send tool is not available); the next run will retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. The v5 Maven reactor planner no longer pins the local root when a dependency's ${prop} version (or a possibly-matching ${...} groupId/artifactId with its own version) can't be resolved in the checkout (#513), and it now reads profile <properties>, so a profile that moves the version off the base leaves the declaration alone (#459). vex liveness re-plans the reactor and withholds attestation when any declaration may still read the base version; Maven 4.1.0 <parent/> is inferred from ../pom.xml.

Risk: medium. The logic only unpins or withholds more often, never less, but the stricter liveness answer reaches vendor's keep-older check and scan's takeover warnings as well as vex (now noted in the description).

Look here

• maven_reactor.rs:468:`` attestable_checked, the new `vex` gate
• maven_reactor.rs:1664:``` version_override, the profile and inheritor walk
• maven_reactor.rs:1780 and :1905:````` property_unresolved now unpins the root
• apply.rs:860: liveness switched to attestable_checked

Verified

• cargo test -p socket-patch-core --all-features --lib vendor::jvm (240 passed) and cargo test -p socket-patch-cli --all-features --test vendor_jvm_cli (52 passed).
• Mutation check: breaking parent inference, the coordinate unpin and the undefined-profile-value rule made 4 new unit tests fail; reverting apply.rs:860 made maven_reactor_uncertain_property_version_withholds_vex fail.
• CHANGELOG.md untouched; no debug leftovers or unrelated edits; no new path handling.
• CI 50/50 green on 4c1b5b2; Cursor Bugbot passed.

Changes I made. No code. Added a "Known gaps" section to the description (the two open questions below).

Open questions

• In the no-sibling case vendor exits 0 with applied: 1 but writes no pin, and vendor --check then fails with "no longer wired". Vendored Maven reactor reports applied and VEX attests not_affected when a ${property} version is left unresolved, so the build keeps Central's unpatched jar (also triggered by Maven 4 <parent/> inference) #513 asks for vendor to report not applied. Safer than main (no false vex), and conflicting_literal_version already behaves this way, but you may want a follow-up issue or to keep Vendored Maven reactor reports applied and VEX attests not_affected when a ${property} version is left unresolved, so the build keeps Central's unpatched jar (also triggered by Maven 4 <parent/> inference) #513 open rather than letting Fixes #513 close it.
• Profiles are weighed one at a time (a two-profile combination override is missed); disclosed in the description.

Auto-merge is armed, so approving sends it straight to the merge queue.

---

Generated by Claude Code