Skip to content

Fix berry mode takeover reverting before gates (#468, #369) - #470

Open
Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
mainfrom
agent/fix-berry-takeover-preflight
Open

Mikola Lysenko (mikolalysenko) wants to merge 2 commits into
mainfrom
agent/fix-berry-takeover-preflight

Conversation

@mikolalysenko

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

Copy link
Copy Markdown
Collaborator

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

Fixes #468
Fixes #369

Summary

Switching a yarn berry package between hosted and vendored mode removed
the old mode's wiring before checking whether the new mode could wire
that package. When the new mode then skipped or refused it, the package
ended up patched in neither mode. Both takeovers now run the target
mode's per-package gates first and, on a refusal, leave the existing
mode byte-identical and report the gate's own code. This is what
CLI_CONTRACT.md already promises ("refuses before reverting").

Root cause (shared)

Each berry takeover preflight only covered the project-level gates
(line endings, cacheKey, compressionLevel). The per-package gates ran
after the revert:

Changes

  • core patch/redirect: new preflight_yarn_berry_hosted_dep(dep). The
    rewriter now uses it for its missing-checksum skip, and the takeover
    uses it as well, so the two can't drift apart.
  • cli scan/hosted.rs: the takeover refusal also runs the per-dep gate
    for berry-vendored entries. Refusal warnings are deduplicated by full
    warning instead of by code, so each purl's detail is kept.
  • core vendor/yarn_berry_lock.rs: new
    yarn_berry_vendor_target_preflight(root, purl) runs the backend's
    resolutions_gate + scan_berry_target against the still-hosted
    files. The verdict matches the restored files, because the hosted
    redirect only rewrites the target entry's resolution:/checksum:
    lines and never its key, other entries or package.json.
  • cli vendor.rs: the hosted→vendored takeover runs that per-target
    preflight after the cached project-level one, before restore_upstream.

Tests

Issue Regression test (crates/socket-patch-cli/tests/in_process_vendor.rs) Without fix With fix
#468 berry_vendored_to_hosted_takeover_keeps_vendored_without_berry_checksum (wet + --dry-run) FAILED (no redirect_yarn_berry_missing_checksum refusal; takeover announced) ok
#369 berry_hosted_to_vendored_takeover_runs_package_gates_first (other locked version, user resolutions; wet + --dry-run) FAILED (vendor_takeover_reverted_redirect, hosted pin gone) ok

The #369 test passes --patch-server-url so the vendor run recognises
the hosted pin as a takeover. Without it, the run takes the eject path,
which was already safe.

Local runs:

  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo test -p socket-patch-cli --test in_process_vendor berry_: 6/6 ok.
  • scripts/yarn-berry-vex-matrix.sh 4.12.0 (with
    COREPACK_NPM_REGISTRY=https://registry.npmjs.org, because
    repo.yarnpkg.com is blocked in this sandbox): all yarn 4 suites ok. The
    two yarn@2.4.3 legacy-cachekey legs could not get yarn 2 here (it is
    not on the npm registry); they are unrelated refusal tests.
  • cargo test --workspace --all-features --no-fail-fast: everything
    passes except:
    • 12 chmod/read-only "write failure" tests, which can't fail under uid 0
      in this sandbox. They fail identically on a main-based branch.
    • mode_migration_npm berry_*_takeover_* (2): they panic in fixture
      setup (mode_migration_npm.rs:351, the test's own reqwest fetch of
      registry metadata hits the sandbox's TLS proxy, UnknownIssuer)
      before any CLI code runs. CI runs them.
  • cargo fmt --check reports the same pre-existing diffs as main (CI
    doesn't gate on fmt); the files this PR adds are rustfmt-clean.

Follow-ups

🤖 Generated with Claude Code


Note

Medium Risk
Changes ordering of yarn berry hosted/vendored takeover and revert paths; incorrect behavior previously left projects in a broken neither-mode state, so the fix is behavior-changing but aligns with the documented refuse-before-revert contract.

Overview
Fixes yarn berry mode takeovers (#468, #369) so the target mode’s per-package gates run before any revert of the current wiring. Previously, vendored→hosted could strip vendored state and then skip hosted redirect when the grant lacked yarnBerry10c0; hosted→vendored could remove the hosted pin before berry’s resolutions/lock gates refused vendoring—leaving the package patched in neither mode while the CLI could still report success.

Vendored→hosted (hosted.rs): berry takeover refusal now includes preflight_yarn_berry_hosted_dep (shared with the berry rewriter in core). Refusal warnings dedupe by full warning JSON; skip reasons use the gate’s code.

Hosted→vendored (vendor.rs): after project-level berry preflight, runs new yarn_berry_vendor_target_preflight on the still-hosted tree before restore_upstream.

Core: extracted preflight_yarn_berry_hosted_dep; added yarn_berry_vendor_target_preflight wrapping the backend’s per-target gates. Tests: regression coverage for missing berry checksum on takeover and for package gates before hosted→vendored revert (wet + dry-run).

Reviewed by Cursor Bugbot for commit 83bb9a9. Configure here.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
Switching a yarn berry package between hosted and vendored mode
removed the old mode's wiring before checking whether the new mode
could wire that package. When the new mode then skipped or refused
it, the package ended up patched in neither mode:

- vendored -> hosted: a grant without the yarnBerry10c0 cache
  checksum deleted the vendored patch, then skipped the redirect,
  and the run still exited 0 saying the package was fully hosted
  (#468).
- hosted -> vendored: another locked version of the name or a
  user-authored resolutions entry restored the registry entry, then
  failed vendoring with vendor_override_conflict (#369).

Both takeovers now run the target mode's per-package gates first and
leave the existing mode byte-identical, reporting the gate's own
code, as CLI_CONTRACT.md already promises.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 1, 2026 14:32
@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 83bb9a9. Configure here.

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

Copy link
Copy Markdown
Collaborator Author

Ready for review at 83bb9a9 (83bb9a99b1700483279c101984dc18315581381f).

  • CI: 96/96 check runs green on the head; 4 skipped by path filters, 0 failing. Mergeable, 0 commits behind main.
  • Bugbot: reviewed 83bb9a9 and found no issues. No unresolved review threads.
  • Reviewer focus: vendor_records_reusing (hosted → vendored) and commands/scan/hosted.rs (vendored → hosted). Both now run the target mode's per-package gates before reverting the existing mode, as CLI_CONTRACT.md requires ("refuses before reverting"). See tests/in_process_vendor.rs for the byte-identical-on-refusal assertions.

Generated by Claude Code

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

2 participants