Skip to content

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

Merged
Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-maven-reactor-vex-literal-base
Oct 29, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-maven-reactor-vex-literal-base

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

Fixes #584

Summary

After a Maven reactor is vendored, a module added later (or a dependency
added to an existing module) that declares the patched artifact at its
plain base version builds Central's unpatched jar. vendor --check
already reported this as drift, but vex kept attesting
not_affected. Now vex skips the package as vendor_unwired until
vendor is re-run, and attests again once it is.

Root cause

vex decides whether a vendored Maven reactor entry is still live with
maven_reactor::attestable_checked. Since #1787 that function re-runs the
planner's declaration walk (rewrite_declarations), but it only looked
at the uncertain set (${prop} versions it can't resolve, or that a
profile or inheriting pom overrides). A declaration at the literal base
version is something the planner would rewrite. It produces an edit,
not an uncertain entry, so the check ignored it.

Fix

attestable_checked now also withholds when the walk still finds a
declaration it would rewrite: Ok(edits.is_empty() && uncertain.is_empty()).
Right after a successful vendor the re-plan is idempotent, so that map
is empty. The unit tests' vendor() helper asserts this. A healthy
reactor therefore still attests. The same rule covers a module left on
an earlier patch's suffix, which builds that patch's jar, not this
one.

Tests (red → green)

Issue Test Without fix With fix
#584 (added module / added dependency / earlier suffix) maven_reactor::tests::a_base_declaration_added_after_vendoring_is_not_attestable left: (true, true) right: (true, false) pass
#584 (CLI: vendor --check fails, vex skips vendor_unwired and writes no file, re-vendor attests again) vendor_jvm_cli::maven_reactor_module_added_at_base_after_vendoring_withholds_vex vex emitted verified/not_affected (the issue's exact symptom) pass

Commands run locally (Linux, as root):

  • cargo fmt --all -- --check: clean
  • cargo clippy --workspace --all-features -- -D warnings: clean
  • cargo test -p socket-patch-core --lib maven_reactor: 87 passed
  • cargo test -p socket-patch-cli --test vendor_jvm_cli: passed
  • cargo test -p socket-patch-cli --test e2e_vendor_jvm_build -- --ignored maven_reactor (Maven at /opt/maven): 2 passed
  • cargo test --workspace --all-features --no-fail-fast: 15248 passed, 11 failed. None of the failures touch the JVM code, and none can pass in this sandbox. 9 simulate write or remove failures with file permissions, which root bypasses: repair ×2, in_process_redirect ×3, and core copy_tree, vlt_heal, pypi_poetry, pypi_requirements. The other 2 are e2e_bun_lockb: the patch API is unreachable through the sandbox proxy, and the local bun is 1.4.4. CI runs these as non-root with network access.

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

/code-review high findings not fixed here

  • A new root module that has no versioned declaration and no pin (e.g. its own external parent). Telling whether that root should be pinned depends on management outside the checkout. vendor --check resolves that through the local Maven repo (local_external_poms). A no-repo re-plan inside vex would wrongly withhold for roots vendor left unpinned on purpose (conflicting_managed_version / management_unresolved). This is a different shape from 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 and should be a follow-up that passes the local repo to vex's liveness check.
  • Range or classifier declarations are only warned about and still attest. That was already true before this PR and already shows at the first vendor, so it isn't drift. Left for a separate issue.
  • Perf: the liveness walk builds throwaway warning strings. It runs once per entry and is negligible next to Reactor::discover.

https://claude.ai/code/session_011f9aeRZEvH2uAFQPt5iJJJ


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
After a reactor was vendored, adding a module (or a dependency) that
declares the patched artifact at its plain base version made that
module build Central's unpatched jar. `vendor --check` reported the
drift, but `vex` still attested not_affected, because its liveness
check only looked for ${property} versions it could not resolve.

The check now also withholds attestation when the planner would still
rewrite a declaration, so `vex` skips the package as vendor_unwired
until `vendor` is re-run. Fixes #584.

Assisted-by: Claude Code:claude-opus-5-5
A module still declaring an earlier patch's suffixed version builds
that patch's jar, so the reactor must not attest the current one.
Pin that behavior with a test.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 29, 2026 01:10
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

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

Copy link
Copy Markdown
Collaborator Author

Burn-down agent: labeled Ready for review at 5e82537.

  • CI: 82 check runs: 32 success, 50 skipped, 0 failing on 5e82537; mergeable, no conflicts.
  • Cursor Bugbot: reviewed 5e82537, no issues found; no unresolved review threads.
  • No CHANGELOG.md change.
  • Slack announcement could not be sent in this run (no Slack send tool available); the next run will retry.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Final review brief

What it does: After a Maven reactor is vendored, a module (or dependency) added later at the plain base version builds the unpatched jar. vendor --check already flagged that, but vex kept attesting not_affected. Now vex skips the package as vendor_unwired until vendor is re-run, by also requiring that the planner's own re-walk finds nothing left to rewrite.

Risk: low. It's a one-line change to the liveness predicate (edits.is_empty() && uncertain.is_empty()) and only makes vex stricter. False withholds are ruled out because the re-plan right after a successful vendor is idempotent, and the test helper asserts that for every unit fixture.

Look here:

  • maven_reactor.rs:471-506:`` the predicate change
  • maven_reactor.rs:3429-3441:``` the idempotence assertion the fix relies on
  • maven_reactor.rs:5530:```` unit cases (added module, added dep, earlier patch's suffix)
  • vendor_jvm_cli.rs:1244: CLI test: vendor --check fails, vex withholds, re-vendor attests again

Verified: I read the full diff and rewrite_declarations. Edits are produced only for base-like versions, the same ones vendor rewrites, so the check can't disagree with the planner. CHANGELOG.md is untouched and there are no stray edits. CI: ci-ok and clippy green on 5e82537, all checks pass. Cursor Bugbot: success, no threads.

Changes I made: none.

Open questions: none blocking. The follow-ups in the description (a new root module with no pin; range or classifier declarations) predate this PR.

I'm arming auto-merge, so approving sends it straight to the merge queue.


Generated by Claude Code

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 29, 2026
@mikolalysenko Mikola Lysenko (mikolalysenko) removed the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 29, 2026
Resolve the attestable_checked conflict: keep the #584 doc and
predicate, take main's new `list` parameter.

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

Copy link
Copy Markdown
Collaborator Author

BugBot review


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 eecdbb2. Configure here.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

[agent] Re-enqueued once (final reviewer) after the 13:40 merge-queue eviction: main has since been merged into the branch, head eecdbb2 is approved, ci-ok/clippy green, mergeable, no open threads. Approvals stay valid across later pushes per the repo owner's rule.


Generated by Claude Code

Merged via the queue into main with commit b291caa Oct 29, 2026
50 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-maven-reactor-vex-literal-base branch October 29, 2026 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants