GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||
GeezerVison is showing github.com — Fix berry mode takeover reverting before gates (#468, #369) by mikolalysenko · Pull Request #470 · SocketDev/socket-patch Conversation
|
| 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 (withCOREPACK_NPM_REGISTRY=https://registry.npmjs.org, because[email protected] legacy-cachekey legs could not get yarn 2 here (it iscargo test --workspace --all-features --no-fail-fast: everythingmode_migration_npm berry_*_takeover_* (2): they panic in fixturemode_migration_npm.rs:351, the test's own reqwest fetch ofUnknownIssuer)cargo fmt --check reports the same pre-existing diffs as main (CIpackageManager set: the two-document pnpm-lock.yaml makes vendor refuse, and vendor --revert, rollback and the hosted takeover half-revert the project and break frozen installs #466 item 4 (pnpm vendored → hosted) is the same takeover-ordering🤖 Generated with Claude Code
Note
Medium Risk
Changes Yarn Berry hosted/vendored takeover ordering and lockfile restore preview paths; mistakes could leave projects in a broken patch state, but behavior aligns with documented refuse-before-revert contract and is covered by new integration tests.
Overview
Fixes Yarn Berry mode takeovers (#468, #369) so per-package gates run before tearing down the current mode. Previously, reverting vendored or hosted wiring first could leave a package unpatched when the target mode then refused or skipped it.
Vendored → hosted (hosted.rs): Berry takeover refusal now includes preflight_yarn_berry_hosted_dep (grant must have yarnBerry10c0) ahead of vendored revert; refusals are owned/cloned so skip reasons use the gate’s code; duplicate pre_warnings are deduped by full JSON.
Hosted → vendored (vendor.rs + yarn_berry_vendor_target_preflight): After project-level berry checks, a dry-run restore_upstream populates new RestoreOutcome.staged_text; resolution/lock-entry gates run on that preview so user overrides and conflicting lock entries fail with vendor_override_conflict without removing the hosted pin.
Regression tests cover grants missing the berry checksum and hosted→vendored with an extra locked version or user resolutions.
Reviewed by Cursor Bugbot for commit 852472c. Configure here.
Generated by Claude Code
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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
|
BugBot review Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Ready for review at
Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Reviewed No standalone blocking findings. Both takeover directions run the new package/grant refusal gates before reverting the old mode, and the focused tests preserve the previous wiring byte-for-byte on refusals. Validation: Integration with #465 needs a fix. Do not combine #465 and #470 unchanged: their automatic merge is textually clean, but the combined Berry suite fails berry_crlf_takeovers_round_trip_both_directions. The new target preflight rejects the Socket-owned left-pad@npm:1.3.0 resolution added by #465 as vendor_override_conflict. The preflight needs to recognize owned hosted resolutions or evaluate the restored state. I tested the clean combined tree: 5 passed, 1 failed, versus all 6 passing on #470 alone. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
The hosted->vendored takeover's per-package preflight read the still-hosted package.json / yarn.lock. Hosted wiring that also writes a Socket-owned package.json resolutions pin (#465) was then mistaken for a user override (vendor_override_conflict) and the takeover refused. The preflight now dry-runs the takeover's own restore_upstream and runs resolutions_gate / scan_berry_target on the restored text (RestoreOutcome.staged_text), so only the user's own wiring can refuse. The #369 regression test now mounts the upstream entry the restore reads and drops a leftover debug eprintln. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_014hMbzxwUnf5voAqbc8g4df
Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_014hMbzxwUnf5voAqbc8g4df
|
Fixed the #465 integration in
Validation:
Trade-offs:
Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
BugBot review Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
CI on
I couldn't re-run the job: the API returns 403 with this session's credentials. Please re-run Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Pushed compatibility fix The latest #465 moves checksum validation so an already-complete pin can retain its stored checksum. #470's eager rewrite preflight conflicted with that change. This commit keeps the unconditional grant check at the fresh vendored → hosted takeover boundary and restores #470's original, equivalent inline rewrite gate. Neither PR needs to absorb the other. Hosted → vendored still checks the staged restored files before changing the project. Validation: standalone 124 core Berry tests + 6 takeover tests passed. Combined with #465's latest The historical-installer failures inspected on this head were external patch API HTTP 504s: Poetry 1.2.2/macOS failed before rewriting, and PDM 2.22.4/Linux failed during a rescan. Uploaded artifacts confirm both causes. I retried the affected jobs: Poetry passed; PDM passed as well. Full platform CI is still required. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
CI on
I can't confirm it from here: this sandbox can't reach the patch API or download artifacts, and re-running returns 403. Please re-run Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
A second Python job failed on Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
BugBot review Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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 852472c. Configure here.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
[agent] Status on Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Vendored → hosted takeover on yarn berry deletes the vendored patch, then skips the hosted rewrite when the grant has no yarnBerry10c0 checksum, and still exits 0 "fully hosted" Hosted → vendored takeover on yarn berry reverts the hosted redirect before a per-package vendor refusal, leaving the package unpatched in both modes
There was an error while loading. Please reload this page.