GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Fix Maven reactor pinning unresolved properties (#513, #459) by mikolalysenko · Pull Request #1787 · SocketDev/socket-patch

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 28, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Start fix for #513, #459

cc4b0be

Assisted-by: Claude Code:claude-opus-5-5

This was referenced Oct 28, 2026

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

Closed

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

Closed

Claude (claude) added 3 commits

October 28, 2026 15:45


          Test Maven reactor pins on uncertain properties

253ab73

A reactor module whose version is an undefined ${prop} or one a
profile raises still got a root pin and a rewrite, so the build kept
an unpatched jar while vex attested not_affected. These tests capture
#513 (external-parent property, Maven 4 <parent/> inference) and
#459 (profile overrides) and fail on main.

Assisted-by: Claude Code:claude-opus-5-5


          Leave Maven roots unpinned on uncertain properties

cd2f751

The vendored Maven reactor planner pinned and rewrote a ${prop}
version it could not resolve with certainty, so the build kept
Central's unpatched jar while vendor reported applied and vex attested
not_affected.

- An undefined property now leaves the local root unpinned, like a
  conflicting literal: the module's explicit version beats any pin.
- Profile <properties> are read. A version a profile on the chain of
  the declaring pom (or of a pom inheriting it) would move off the
  base is left as is with conflicting_literal_version, and the root
  is not pinned. maven.config user properties still win.
- A Maven 4 <parent/> without coordinates resolves to ../pom.xml, so
  a property defined in the checkout's root is found and patched.

Fixes #513, #459.

Assisted-by: Claude Code:claude-opus-5-5


          Withhold Maven vex on modules left at base

1d77282

Unpinning the root was not enough: vex counted the patch as wired
whenever any pom named the suffixed version, so a rewritten sibling
let it attest while a module left at an uncertain ${prop} built the
unpatched jar.

- vex liveness for a reactor now re-plans the declarations and
  withholds when one is left at an undefined, inheritor-overridden or
  profile-overridden version. The GC and --check reference test is
  unchanged, so a referenced tree is never pruned.
- A possibly-matching declaration with an undefined ${...} groupId or
  artifactId and its own version also leaves the root unpinned.
- Parent inference is limited to model 4.1.0, and a Maven 4 child's
  ${project.version}/${project.groupId} come from the inferred parent.
- The inheritor and profile override searches are one walk.

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) marked this pull request as ready for review

October 28, 2026 16:34

Mikola Lysenko (mikolalysenko) commented Oct 28, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 28, 2026

View reviewed changes

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

crates/socket-patch-core/src/vendor/jvm/apply.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread

crates/socket-patch-core/src/vendor/jvm/maven_reactor.rs Resolved

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) mentioned this pull request

Oct 28, 2026

Swarm health manager log #1382

Open


          Treat a profile that undefines the version as moved

4c1b5b2

A profile (or inheriting pom) that points the version property at a
name the checkout does not define made the planner skip it and still
rewrite the declaration to the suffixed base. Such an override now
counts as moving the version, so the declaration is left as is and
the root is not pinned.

The CLI regression test now also covers a sibling module at the
literal base: it is rewritten and wired, yet vex still withholds
because the uncertain module builds the unpatched jar.

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) commented Oct 28, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Oct 28, 2026

View reviewed changes

cursor Bot left a comment

Copy link

Copy Markdown

There was a problem hiding this comment.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) mentioned this pull request

Oct 28, 2026

Vendored Maven reactor vex attests not_affected after a module added later declares the base version, although vendor --check flags the drift and that module builds Central's unpatched jar #584

Closed

Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label

Oct 28, 2026

Mikola Lysenko (mikolalysenko) commented Oct 28, 2026

Copy link

Copy Markdown

Collaborator Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Oct 28, 2026

Copy link

Copy Markdown

Collaborator Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) enabled auto-merge

October 28, 2026 19:07

Mikola Lysenko (mikolalysenko) commented Oct 28, 2026

Copy link

Copy Markdown

Collaborator Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Tanmay Singla (Tanmay182003) approved these changes

Oct 28, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) added this pull request to the merge queue

Oct 28, 2026

Merged via the queue into main with commit a9d2312

Oct 28, 2026

50 checks passed

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) deleted the agent/fix-maven-reactor-uncertain-property branch

October 28, 2026 21:26

This was referenced Oct 29, 2026

CI performance dashboard #1182

Open

Fix Maven reactor VEX over a base-version module (#584) #1802

Merged

Fix Maven vendor reporting unwired patch applied (#513) #1826

Merged

Reviewers

cursor[bot]

cursor[bot] left review comments

Tanmay Singla (Tanmay182003)

Tanmay182003 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

Ready for review

Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

Uh oh!

There was an error while loading. Please reload this page.

3 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Root cause

Changes

Tests (red → green)

/code-review high findings not fixed

Known gaps (added by the final reviewer)

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 28, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 28, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 28, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 28, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 28, 2026

Final review brief

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mikola Lysenko (mikolalysenko) commented Oct 28, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot left a comment •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.