Repository navigation
Fix hosted yarn classic pins missing berry warning (#907) - #917
Mikola Lysenko (mikolalysenko) merged 9 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
A yarn 2+ install migrates a classic (v1) yarn.lock and re-resolves every entry from the registry, so a hosted pin is silently dropped and the package installs unpatched. Vendored mode already warned about this; hosted mode said nothing. The hosted engine now reads package.json beside a classic yarn.lock, and the classic rewriter warns redirect_yarn_classic_berry_migration_risk once per run when the lock carries a hosted pin, unless package.json pins yarn 1 through packageManager. The yarn 1 check is shared with the vendored probe so both modes agree. Fixes #907 Assisted-by: Claude Code:claude-opus-5-5
A yarn.lock with no root package.json is not a project yarn installs from, so there is nothing to warn about. This also keeps the advisory off rewriter fixtures that carry only a lock. Assisted-by: Claude Code:claude-opus-5-5
main has failed socket-patch-core's lib tests since Gradle support (#646) and the digest helpers (#865) both landed. The guard test production_digests_go_through_the_helpers flags three files #646 added that still hash inline: crawlers/gradle_cache.rs, patch/jvm_jar.rs and patch/sidecars/maven.rs. That breaks test, test-release and coverage on every open PR. Each inline sha1/sha256 call now goes through sha1_hex_of or sha256_hex_of, which compute the same lowercase hex. Behaviour is unchanged. Assisted-by: Claude Code:claude-opus-5-5 (cherry picked from commit 659ac2c)
|
[agent] Generated by Claude Code |
The scan benchmark treats any warning as a failed scenario. Its yarn classic project declared no package manager, so a hosted scan now correctly warns that a yarn 2+ install would drop the pins. Declare packageManager yarn@1.22.22, as the berry fixture declares yarn 4 and as a real classic project would. The measured scan is unchanged. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
|
Ready for review at
Generated by Claude Code |
Bring the branch up to date with main (1c6c509). CLI_CONTRACT.md conflicted on the hosted rewriter's candidate-file paragraph: main added the sbt build files to the read set and a new "Hosted sbt" paragraph, while this branch added the yarn classic berry-migration advisory. The edits touch different clauses, so both are kept. Co-Authored-By: Claude <noreply@anthropic.com>
Pick up the four PRs that landed on main (db83f01) while the previous merge was being validated. No conflicts. Co-Authored-By: Claude <noreply@anthropic.com>
|
bugbot run Generated by Claude Code |
Main's test jobs are red: production_digests_go_through_the_helpers fails because PENDING_INLINE_DIGESTS still names gradle_cache.rs, jvm_jar.rs and sidecars/maven.rs. #955 added them while those files computed digests inline, and #690 (sbt/Mill/scala-cli) then moved them onto utils::digest helpers; the two merged independently, so the ratchet now fails on stale entries on every platform. Remove the three entries so the list matches the production tree. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Qq8uwZ9NTCZXZRygn7woy7 (cherry picked from commit 6cb46c0)
|
[agent] Generated by Claude Code |
|
[agent] The
Generated by Claude Code |
|
[agent] On Generated by Claude Code |
|
[agent] The
Generated by Claude Code |
|
bugbot run Generated by Claude Code |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e0c158c. Configure here.
|
[agent] Blocked: this PR was merged (15:26Z, head
Generated by Claude Code |
With a yarn-offline-mirror configured the classic hosted rewriter refuses every entry, so nothing is pinned, yet it still warned that a berry install would drop the hosted pins (#907's warning counted the refused entries as pinned). The staged takeover reports a retracted purl's first rewrite warning as its cause, so a vendored classic project with a mirror was skipped as redirect_yarn_classic_berry_ migration_risk and the real refusal, redirect_yarn_classic_offline_ mirror, was never reported (in_process_vendor's classic_vendored_to_hosted_takeover_refuses_with_offline_mirror failed once main's #917 landed beside the staged takeover). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* Let a group commit hold vendored artifact deletions until it lands A group commit can now defer the vendored artifact deletions a revert makes (GroupCommit::defer_removals): every per-unit revert removal goes through remove_tree_and_prune (cargo and golang now too, instead of their own remove_tree + prune copies) and the bun workspace tarball removal through remove_mirror, and both queue the deletion for after the commit when the open group asks for it. A rollback_to forgets the queued deletions and a dropped group never makes them, so a staged revert can be undone with its artifact intact. Also: - commit_unjournaled: the all-or-nothing replace without the crash journal, for runs that must write nothing under .socket/; - a journal that had to create .socket/vendor/ prunes it again; - group_commit::exists is public, for overlay-aware existence checks. Audit B03/B14 groundwork. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Share one takeover-reach predicate between the hosted engines The disk flow took over cargo, npm, golang, pypi and Gradle maven entries, while the in-memory engine refused only cargo, npm and golang, so a vendored PyPI package reached the Python rewriters in memory. Both now use hosted::takeover::in_reach (audit B15). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Make the vendored-to-hosted takeover staged and atomic scan/get --mode hosted reverted a vendored package's wiring on disk first and planned the hosted pin afterwards. When the rewriter then refused (a lock-level refusal, a missing berry checksum, unavailable wheel metadata, a Poetry 0.x lock, ...), the package was left unpatched in both modes, and only six hand-copied per-ecosystem pre-gates tried to predict those refusals. The dry run counted every takeover as redirected. The takeover now runs inside the run's group commit: - each vendored revert is staged in the overlay under a savepoint (a failing or drift-keeping revert is rolled back and refused); - the hosted rewrite reads the overlay, so it plans against the reverted project; - a staged purl the rewrite does not pin is retracted: the overlay goes back to its pre-revert state, the purl stays vendored byte for byte (redirect_takeover_kept_vendored, skipped with the cause), and the rest are staged and rewritten again; - the hosted pins and the vendored ledger are written into the same overlay and committed once (journaled); artifacts go after the commit. A dry run does the same and drops the overlay, so it reports the wet outcome. A hosted run without a takeover commits its files unjournaled, putting back the ones replaced if one fails. Deleted: the bun, berry (lock and dep), classic, vlt, Gradle, pypi platform-wheel and requirements pre-gates (9 copies of rewriter logic -> 0; the requirements reach check only explains a retraction now), the dry-run TakeoverPreview path in the engine, and the stranded-takeover reporting (redirect_takeover_unpatched), which can no longer happen. Audit B03, B14, B37 (takeover part). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Keep a staged Gradle takeover from deleting the vendored tree The staged vendored-to-hosted takeover runs the real revert inside a group commit and relies on the overlay plus deferred removals to undo it. The JVM revert deleted the tree files under .socket/vendor/gradle and .socket/vendor/maven2 directly, and wrote or deleted the owned .socket/gradle/.gitattributes, .socket/vendor/.gitattributes and the derived maven-metadata.xml files straight to disk. A dry run, or a takeover the hosted Gradle planner refused and retracted, therefore deleted the vendored jars while the restored wiring still named them. Capture the owned .gitattributes files and the derived metadata in the group overlay, and route the tree-file deletions through group_commit::defer_removal so they happen only after the commit. A new test stages the revert (with and without a sibling version sharing the metadata) and checks that dropping or rolling back the group leaves the project byte-identical and that committing lands the plain revert. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Put back direct hosted writes when the hosted commit fails commit_hosted_writes writes the files the group does not capture (the Gradle hosted index and script under .socket/gradle/) straight to disk before the commit. When writing a later file, saving the vendored ledger or the commit itself failed, the error said nothing was changed while those files stayed on disk. Record their previous bytes and put them back on every failure path except an interrupted journaled commit, which the next locked command finishes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Check that dry-run takeovers leave the whole tree byte-identical Snapshot every project file, .socket/ and the vendored artifacts included, around the dry-run vendored-to-hosted takeover for pnpm, package-lock, vlt, golang and cargo (bun and the uv retract test already compare the artifact). Rewrite the stale CLI_CONTRACT Gradle paragraph that still described the deleted takeover_refusal pre-gate, and the real-Gradle refusal test's doc comment. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Make the yarn hosted preflights private to the redirect module The takeover pre-gates that called preflight_yarn_classic_hosted and preflight_yarn_berry_hosted from outside are gone. The classic one is now private and the berry one pub(crate) (upstream/npm.rs still uses it). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Skip the yarn berry risk warning when an offline mirror refused the pins With a yarn-offline-mirror configured the classic hosted rewriter refuses every entry, so nothing is pinned, yet it still warned that a berry install would drop the hosted pins (#907's warning counted the refused entries as pinned). The staged takeover reports a retracted purl's first rewrite warning as its cause, so a vendored classic project with a mirror was skipped as redirect_yarn_classic_berry_ migration_risk and the real refusal, redirect_yarn_classic_offline_ mirror, was never reported (in_process_vendor's classic_vendored_to_hosted_takeover_refuses_with_offline_mirror failed once main's #917 landed beside the staged takeover). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Keep a yarn berry takeover vendored when the project gates refuse it #657 (merged on main) made the hosted and vendored modes refuse a mixed root package.json, and gated the vendored-to-hosted takeover before its revert. The staged takeover dropped that pre-gate and let the hosted rewriter judge the reverted project, but the berry revert re-renders package.json in its majority line ending, so a mixed manifest passed the rewriter's check after the revert and the takeover went ahead (in_process_vendor berry_takeovers_refuse_before_reverting_the_old_mode failed after the merge). Judge the berry project gates once per staging pass, on the pre-revert overlay, through the rewriter's own preflight_yarn_berry_hosted (the shared berry_gates set, no copied logic). A refused yarn-berry entry is skipped with the gate's code, followed by redirect_takeover_kept_vendored. preflight_yarn_berry_hosted is public again for this caller. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * Keep landed-pin advisories and scope suffixes out of takeover skip reasons When a staged takeover is retracted and no rewriter warning names the package, explain() fell back to the rewrite's first warning as the lock-level cause. That warning can be a success advisory from a pin that did land (redirect_npm_allow_remote, redirect_pnpm_trust_lockfile, redirect_yarn_classic_berry_migration_risk), so the skipped purl and redirect_takeover_kept_vendored reported the wrong code. Skip those advisories when picking the fallback; with nothing else left the reason is NOT_PINNED. names_package accepted `/` as a left boundary unconditionally, so an unscoped name like `node` matched inside `@types/node` and a retracted takeover could inherit another package's warning. A `/` now counts as a boundary only after a path segment, not after an `@scope`. Co-Authored-By: Claude <noreply@anthropic.com> * Label the setup-php pin in ci.yml with its real tag The required 'Audit GHA Workflows' check (zizmor ref-version-mismatch) now fails on every head because the pinned setup-php hash no longer matches the moving v2 tag. Same one-line change as #1118, so it merges cleanly when that lands. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

LLM Description written by Claude Code:claude-opus-5-5
Fixes #907
Summary
A yarn 2+ (berry) install migrates a classic (v1)
yarn.lockand re-resolves every entry from the registry. That drops a hosted pin exactly the way it drops vendored wiring, and the package then installs unpatched with nothing printed. Vendored mode already warned about this (yarn_classic_berry_migration_risk). Hostedscan/getreportedsuccesswith no warning, even whenpackage.jsondeclared"packageManager": "yarn@4.x".Hosted runs now warn
redirect_yarn_classic_berry_migration_risk(inredirect.warnings) once per run when the classic lock carries a hosted pin, whether it was written this run or is already there from an earlier one. A"packageManager": "yarn@1…"pin suppresses it, as it does for vendored. The warning is advisory only: the pin still lands and the exit code is unchanged.Root cause
rewrite_yarn_classic(core/src/patch/redirect/mod.rs) never ran the check that the vendored probe (vendor::yarn_classic_berry_migration_risk) runs.read_candidate_files(core/src/hosted/engine.rs) read the rootpackage.jsononly next to an npm lock or a berryyarn.lock. So the classic rewriter couldn't seepackageManagerat all.Changes
vendor::manifest_pins_yarn_classic: theyarn@1check, pulled out of the vendored probe so both modes agree (yarn@10still doesn't matchyarn@1; a malformed manifest vouches for nothing).package.jsonas advisory input (never rewritten) next to a classicyarn.lock.rewrite_yarn_classicemits the warning when any dep matched a lock entry, the root manifest is present, and it doesn't pin yarn 1. With no root manifest there is no project for yarn to install, so it stays silent. This also keeps the advisory off rewriter fixtures that carry only a lock.CLI_CONTRACT.md(npm-family flavor coverage) anddocs/ecosystems.md.yarn-classicscan fixture now declarespackageManager: yarn@1.22.22, the same way the berry fixture declares yarn 4. The bench counts any warning as a failed scenario, and the measured scan is unchanged.66009a0, Gradle digests throughutils::digest). It fixesproduction_digests_go_through_the_helpers, which is red onmain(9c43dfc) and failscoverage. It becomes a no-op once Route Gradle digests through utils::digest #878 lands.Tests (red → green)
Per-issue checklist:
packageManagerwarns once, including dry run and idempotent re-run:yarn_classic_hosted_pin_warns_berry_migration_risk,yarn_classic_berry_risk_is_once_per_run_and_survives_reruncovgap_commands_scan_hosted::hosted_yarn_classic_pin_warns_berry_migration_risk(no pin andyarn@4.18.1; dry run, wet run, re-run)yarn@1pin suppresses it:yarn_classic_hosted_pin_yarn1_package_manager_suppresses_berry_risk,hosted_yarn_classic_pin_with_yarn1_package_manager_stays_silentyarn_classic_berry_risk_silent_without_a_pinRed: with the rewriter warning disabled, both core warn tests and the CLI warn test fail. With the rewriter restored but the engine's
package.jsonread reverted, the CLI warn test still fails, so both halves are needed. Green with the fix.Commands run locally (Linux, toolchain 1.93.1):
cargo clippy --workspace --all-features -- -D warnings: cleancargo test -p socket-patch-core --all-features: all pass except 4 chmod-based tests (copy_tree,vlt_heal,pypi_poetry,pypi_requirementswrite-failure tests). Those fail only because this sandbox runs as uid 0, and they're in files this PR doesn't touch.covgap_commands_scan_hosted,covgap_commands_rollback,covgap_commands_scan_mod,e2e_redirect_yarn_classic_build,e2e_vendor_yarn_classic_build,e2e_vex_redirect,global_scope_project_state,in_process_get_modes,in_process_rollback_hosted,mode_migration_npm,hosted_memory_engine,hosted_memory_parity,scan: pass.covgap_commands_vendor(3) andin_process_redirect(3) have only chmod-0o555 write-failure tests failing, for the same uid-0 reason.scripts/yarn-classic-vex-matrix.sh 1.22.22(real yarn, required): 40/40 cells PASSsocket-patch-bench run -f yarn-classic: both scenarios validatecargo test -p socket-patch-bench: passcargo fmtisn't enforced by CI andmainisn't fmt-clean, so only the new code was formatted.No wrapper changes: the npm/pypi/gem wrappers only dispatch to the binary.
🤖 Generated with Claude Code
Generated by Claude Code
Note
Low Risk
Advisory warning-only change to hosted yarn classic redirect behavior; no change to pinning logic or exit codes beyond new optional warnings in JSON/stderr.
Overview
Fixes #907: hosted
scan/getnow surfaces the same yarn classic → berry migration trap vendored mode already warned about.When a v1
yarn.lockcarries hosted pins, the yarn classic rewriter emitsredirect_yarn_classic_berry_migration_riskonce per run (dry run, wet run, and idempotent re-run). The pin still applies and exit code stays 0; it's advisory only."packageManager": "yarn@1…"suppresses the warning via sharedmanifest_pins_yarn_classic, refactored from the vendored probe so both modes agree.The hosted engine now loads root
package.jsonas advisory input beside a classicyarn.lock(not only npm locks / berry locks), sopackageManageris visible to the rewriter.Docs (
CLI_CONTRACT.md,docs/ecosystems.md) describe the hosted warning; the yarn-classic bench fixture pinsyarn@1.22.22so benchmarks don't treat the new warning as a failure. Core unit tests andcovgap_commands_scan_hostedintegration tests cover warn/suppress/silent cases.Reviewed by Cursor Bugbot for commit e0c158c. Configure here.