GeezerVison is showing github.com — Fix nightly vlt migration e2e after #2021's heal change by mikolalysenko · Pull Request #2050 · SocketDev/socket-patch
Conversation
Problem
Scheduled CI on main (42763778797, main @ c751724) fails every mode_migration_vlt leg:
e2e-extended on ubuntu: vlt 0.0.0-32, 1.0.0-rc.14 and 1.2.0e2e-macos: vlt 1.2.0e2e-windows: vlt 1.0.0-rc.14 and 1.2.0
That's 6 of the 8 failing jobs in the run. The other two are test (windows-latest, *), which #2049 fixes. The same legs also failed in #2049's Windows dispatch 42772455462. These legs run only at night, so PR and merge-queue CI never catch the failure. Nightly ci-ok stays red until it's fixed.
Every leg fails the same assertion in vlt_pinned_matrix_migration_hosted_scan_keeps_agent_patched_tree (migration test 8):
left: "vlt-lock.json pins Socket-patched packages, but socket-patch could not check 1 installed copies (node_modules is a link, or node_modules/.vlt-lock.json is missing or unreadable); run `vlt ci` to be sure the patched packages are installed."
right: "vlt-lock.json pins Socket-patched packages; fresh checkouts install them with `vlt ci` or `vlt install --frozen-lockfile`. Note: `vlt update` re-resolves from the registry and drops these hosted patches."
Root cause
#2021 changed how the vlt warm-tree heal judges a store entry: now only vlt's hidden lock decides. Without a hidden lock, the copy is kept and reported as unchecked. This is intended, and the commit message says so.
#2021 updated the rule (c) real-vlt e2e in e2e_redirect_vlt_build.rs (vlt_pinned_matrix_hosted_heal_rule_c_no_hidden_lock) to expect advisory_undeterminable. It missed the matching case in mode_migration_vlt.rs. Test 8 deletes node_modules/.vlt-lock.json and still expected the old afterHash outcome, advisory_nothing_stale().
Fix
Test 8 now expects advisory_undeterminable(ids.len()), the same as the rule (c) test, and its doc comment is updated to match. Its other assertions are unchanged and still pass:
- the store entries are kept
- the tree stays patched
The only production behaviour involved is the one #2021 intended. No production code changes.
Proof
Local runs with real vlt (SOCKET_PATCH_VLT_E2E_REQUIRED=1, --include-ignored vlt_pinned_matrix):
- main, vlt 1.2.0: test 8 fails with the exact left/right shown above (reproduced).
- This branch, vlt 1.2.0:
mode_migration_vlt passes, 14 passed, 0 failed.
- This branch, vlt 0.0.0-32: 14 passed, 0 failed.
Checks:
Where tests run
No tests were removed or moved. Test 8 still runs in the nightly mode_migration_vlt legs on all three OSes.
Generated by Claude Code
Note Low Risk
Test-only change aligning e2e expectations with intended heal behavior; no production code touched.
Overview
Aligns migration test 8 (vlt_pinned_matrix_migration_hosted_scan_keeps_agent_patched_tree) with the post-#2021 vlt heal behavior when node_modules/.vlt-lock.json is missing: the hosted scan advisory assertion now expects advisory_undeterminable(ids.len()) instead of advisory_nothing_stale(), matching the existing rule (c) real-vlt e2e.
The test doc comment is updated to state that without a hidden lock, heal rule (c) treats store copies as unchecked and recommends vlt ci, while still asserting the agent-patched store entries are kept and the tree stays patched.
Reviewed by Cursor Bugbot for commit 9baa35f. Configure here.
Generated by Claude Code
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Nightly CI on main has failed every mode_migration_vlt leg (ubuntu x3,
macOS, Windows x2) since #2021. That PR made the vlt warm-tree heal
judge a store entry by vlt's hidden lock alone: without one, the copy
is kept and reported as unchecked ("could not check N installed
copies ... run `vlt ci`"). It updated the rule (c) e2e in
e2e_redirect_vlt_build.rs but missed its twin here, migration test 8,
which removes the hidden lock and still expected the "nothing stale"
advisory. These legs run only at night, so PR and merge-queue CI never
saw it.
Expect advisory_undeterminable(ids.len()), as the rule (c) test does.
The store copies are still kept and the tree stays patched.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_011f9aeQSeGNDaC689NLWPom
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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 9baa35f. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Labeled Ready for review by the burn-down agent.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Final review brief
What it does: Test-only fix. #2021 made the vlt heal treat a store copy with no hidden lock (node_modules/.vlt-lock.json) as "unchecked". It updated the rule (c) e2e test but missed migration test 8, which still expected the old "nothing stale" advisory. That leaves every nightly mode_migration_vlt leg red. This PR changes test 8 to expect advisory_undeterminable(n), the same as the rule (c) test.
Risk: Low. One assertion and a doc comment in an #[ignore] real-vlt e2e test. No production code changes and CHANGELOG.md is untouched.
Look here:
- mode_migration_vlt.rs:573-591: new expectation and doc comment
- e2e_redirect_vlt_build.rs:352-360: the rule (c) test it now matches (same setup: hidden lock removed, then hosted scan)
- vlt_e2e_common/fixture.rs:46: the advisory text. It matches the "left" side of the nightly failure word for word.
Verified: I read the full diff (1 file, +5/−3) and checked that the helper is in scope through vlt_e2e_common::fixture::*. The test setup matches the rule (c) test exactly. The store-kept and tree-patched assertions are unchanged. CI: ci-ok and clippy green, 50/50 checks pass. Bugbot found no issues. I did not rerun real vlt here. The PR reports 14/14 on vlt 1.2.0 and 0.0.0-32, and the nightly legs will confirm it.
Changes I made: none.
Open questions: none. This pairs with #2049; both are needed to turn nightly ci-ok green.
Auto-merge (squash) is armed, so approving sends this straight to the merge queue.
Generated by Claude Code
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Labels
ci-janitor
Opened by the CI janitor routine (flakes, redundant tests, CI perf)
Ready for review
Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|