[Mikola Lysenko (mikolalysenko)]

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

Fixes #655

Summary

A Maven reactor module that names the patched library through properties
(<groupId>${ct.group}</groupId> / <artifactId>${ct.artifact}</artifactId>,
defined in a local parent) was skipped by vendor. Its literal
<version>1.10.0</version> stayed, which beats the root's
dependencyManagement pin, so that module built Central's unpatched jar
while vendor reported applied: 1, vendor --check passed and vex
attested not_affected.

Root cause

Doc::keyed_declarations (vendor/jvm/maven_reactor.rs) compared the raw
groupId/artifactId text with the patch coordinates. Maven interpolates
those elements before matching. Every caller (the rewrite, the
external-management check, and unplan's version restore) went through it,
so the fix is at that boundary: it now takes a resolver, and Reactor
supplies its existing property lookup for the pom's context.

Changes

• Match declarations on interpolated coordinates in rewrite, external
  management and revert (revert reuses the reactor unplan already
  discovers).
• A coordinate whose property isn't defined in the checkout, and that could
  still be the patched GA (literal text around ${...} must fit, jar type,
  no classifier), gets a property_unresolved degraded warning instead of
  passing silently.
• A module that inherits a ${prop}-named declaration and overrides the
  property to name another artifact: the declaration is left alone, with a
  coordinates_overridden warning and the root unpinned (the same pattern
  as the existing version-override guard).
• Declarations matched through properties get a separate record-key ordinal
  (...:<section>:$:<n>), so revert of projects vendored by older releases
  still keys literal declarations as before.
• ${project.groupId}, ${project.artifactId} (and pom.*,
  project.parent.groupId) resolve in the reactor lookup.

Tests

Red without the fix, green with it:

Issue	Test	Red on main	Green
#655	maven_reactor::tests::property_coordinates_are_interpolated_and_restored (unit: vendor rewrites, re-plan writes nothing, revert byte-exact)	✅ assertion fails	✅
#655	maven_reactor::tests::unresolvable_property_coordinate_warns	✅ no warning	✅
#655	e2e_vendor_jvm_build::maven_reactor_property_coordinates_vendor_fresh_offline_build_and_revert (real Maven 3.9: fresh checkout, purged ~/.m2, mvn -o resolves the vendored jar in a and b; byte-exact revert)	✅ a's version not rewritten	✅

Plus review-driven unit tests: property_coordinates_keep_literal_record_keys,
property_coordinates_another_module_overrides_are_left_alone,
project_group_id_coordinate_resolves,
unresolvable_coordinate_of_another_artifact_does_not_warn,
may_interpolate_to_keeps_literal_text.

Commands run locally:

• cargo fmt --all -- --check: clean
• cargo clippy --workspace --all-features -- -D warnings: clean
• cargo test -p socket-patch-core --lib: 6807 passed. 4 failures are
  write-failure/unremovable-file simulations that can't fail when running as
  root in this container (copy_tree::relax_loop_must_not_traverse_symlinked_root,
  vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry,
  pypi_poetry::wire_write_failure_*, pypi_requirements::wire_failure_*);
  none touch Maven.
• cargo test --workspace --all-features --no-fail-fast: every suite
  passes apart from those root-only cases (in_process_redirect,
  repair) and e2e_bun_lockb (the known Nightly e2e_bun_lockb red: offline takeover test predates #1648 #1770 breakage on main, fixed in
  CI: update bun.lockb offline takeover test (#1770) #1771/Fix nightly bun.lockb e2e for the hosted-pin eject #1772).
• cargo test -p socket-patch-cli --all-features --test e2e_vendor_jvm_build -- --ignored maven_reactor
  with SOCKET_PATCH_MAVEN_E2E_REQUIRED=1: 2 passed.

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

Review (/code-review high) findings not fixed

• Revert when Reactor::discover fails (e.g. a nested .mvn added
  after vendoring): a ${prop}-named declaration isn't matched and keeps
  the SV. It's reported as still references <sv> drift and the tree is
  kept, so nothing is silent; literal declarations behave as before.
• Coordinates interpolated in an ancestor's context: a parent declaration
  whose property is defined only in a child gets a false
  property_unresolved warning (degraded, never a wrong rewrite). Matching
  Maven exactly needs per-inheritor planning, the same gap the version path
  has.
• ${...} coordinates in external parents and BOMs (jar_declaration)
  are still literal-only. That's a separate code path (Vendored Maven reactor pins over an imported BOM or external parent, so a build that uses 1.11.0 is silently downgraded to 1.10.0-socket.* and a later BOM bump never takes effect #488 external
  management) and a natural follow-up.
• Mirror of the override case (from the final review brief): a parent
  declaration whose ${prop} names another artifact in the parent but the
  patched one in a child that overrides the property isn't matched, so that
  child keeps the parent's literal version without a warning. This isn't a
  regression (nothing property-named matched before this PR); it needs the
  same per-inheritor planning as the ancestor-context item above.
• Double walk of declarations for the warning: a cleanup with no
  correctness impact.

https://claude.ai/code/session_011f9aePV9JEJ6eGAd9Um1Ke

---

Generated by Claude Code

[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 16e930e. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review at 16e930e9 (burn-down agent).

• CI: 32 success + 50 skipped on this head, 0 failing; mergeable into main.
• Bugbot: reviewed 16e930e, no findings, 0 open threads.
• Reviewer note: look at the property-resolver change in Doc::keyed_declarations (vendor/jvm/maven_reactor.rs); it is shared by rewrite, external-management and revert.
• Slack announcement not sent this run (Slack send tool unavailable to the agent).

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Ready for review at 16e930e9 (burn-down agent).

• CI: 82/82 check runs green or skipped (32 success, 50 skipped); mergeable, no conflicts.
• Bugbot: reviewed 16e930e9, no findings; no unresolved review threads.
• Look at the ${prop} interpolation edge cases in maven_reactor.rs (inherited/undefined properties).

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. Fixes #655: a Maven reactor module that names the patched library through properties (${ct.group}:${ct.artifact}) was skipped by vendor, so it kept its literal version and built Central's unpatched jar while vendor/--check/vex all reported success. Declarations are now matched on interpolated coordinates (rewrite, external-management check and revert), with degraded warnings when a property is undefined (property_unresolved) or an inheriting module re-points it at another artifact (coordinates_overridden).

Risk: medium-low. Product code in the Maven vendoring path, but the change is at one matching boundary and literal declarations behave as before: property-matched declarations get their own record-key section (…:<section>:$:<n>), so reverting projects vendored by older releases keys literal declarations unchanged.

Look here

• Doc::keyed_declarations + :$ key section — the matching boundary and record-key compatibility.
• coordinates_overridden guard — inherited ${prop} declaration that a child resolves to another GA is left alone and the root unpinned.
• warn_unresolved_coordinates / may_interpolate_to — when an undefined property could be the patched GA.
• revert path: restore_versions resolver and project.groupId/artifactId lookups.``````
• real-Maven e2e — fresh offline reactor build resolves the vendored jar in both modules, revert byte-exact.

Verified

• Read the full diff. Walked may_interpolate_to edge cases (empty properties, adjacent literals, unclosed ${) and the key-ordinal compatibility; no issues.
• cargo test -p socket-patch-core --lib maven_reactor: 74 passed (incl. the 7 new tests).
• Real Maven 3.9.11: e2e_vendor_jvm_build -- --ignored maven_reactor_property_coordinates passes — modules a and b both resolve commons-text-1.10.0-socket.* from .socket/vendor in a fresh offline checkout, and revert restores the snapshot.
• cargo clippy -p socket-patch-core --all-targets -D warnings: nothing in maven_reactor.rs (only the pre-existing test-target lints elsewhere). CI 32 passed / 50 skipped of 82; ci-ok green. Cursor Bugbot: passed. CHANGELOG.md untouched.

Changes I made: none.

Open questions (non-blocking, for a follow-up if you care): the mirror case isn't warned — a parent declaration whose ${prop} resolves to some other artifact in the parent but to the patched GA in a child that overrides the property is not matched in the parent's context, so that child would keep the parent's literal version silently. Rare in practice; the new guard covers the opposite direction.

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

---

Generated by Claude Code