Fix yarn classic check of dangling descriptors (#1379)

[Mikola Lysenko (mikolalysenko)]

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

Fixes #1379

Summary

Releases since #591 refuse to write a yarn classic yarn.lock block whose dependencies: / optionalDependencies: sub-map names a descriptor (name@range) that has no lock block. But a lock an older release already vendored in that state was still reported healthy: vendor --check returned vendor_check_ok (exit 0) and repair left it alone. Meanwhile yarn 1 installs that dependency unpinned, and --offline installs fail. I confirmed with real yarn 1.22.22 that yarn install --frozen-lockfile accepts such a lock silently.

Now:

• vendor --check fails the entry with vendor_check_failed, and the reason names the dangling descriptors.
• repair reports the entry failed with vendor_dep_manifest_unlocked (the code new runs already refuse with) and leaves the lock as it is.
• Both point to the remedy: yarn install (without --frozen-lockfile) to lock the descriptors, then commit yarn.lock. I verified with real yarn 1.22.22 that this adds the missing block and keeps the vendored file: resolution.

Root cause

The check that refuses new writes (unlocked_descriptors, which reads the patched package.json) had no read-side twin over the lock blocks already wired to an entry. So nothing ever audited a lock written before that refusal existed.

Changes

• formats/yarn/classic_deps.rs: block_dep_descriptors reads a block's own sub-maps back (quoted or bare tokens, the same way split_key_patterns reads keys). unlocked_among is the shared "which of these descriptors has no block" check. unlocked_descriptors now goes through it too.
• vendor/yarn_classic_lock.rs: dangling_descriptors(entry, root) checks every block whose resolved points into the entry's uuid dir. It skips descriptors that the recorded pre-vendor block already named, because yarn 1 writes no block for a workspace member or a link: dependency, and those are upstream's, not the patch's.
• vendor/npm_flavor.rs: dangling_lock_dependencies dispatches by flavor. check_npm_wiring (used by vendor --check) calls it for the non-package-lock flavors.
• commands/vendored_backend/repair.rs: a healthy npm entry whose lock has dangling descriptors is reported as failed.
• CLI_CONTRACT.md: documents the new vendor --check drift cause and the repair use of vendor_dep_manifest_unlocked.

Test evidence

Issue	Regression test(s)	Red without fix	Green with fix
#1379 (vendor --check)	covgap_commands_vendor::yarn_classic_check_and_repair_flag_a_dangling_dependency_descriptor, yarn_classic_lock::tests::issue_1379_check_flags_a_dangling_descriptor_an_old_release_wrote	both FAILED with the check/repair call sites disabled	ok
#1379 (repair)	same CLI test (repair leg: failed / vendor_dep_manifest_unlocked, lock unchanged)	FAILED	ok
boundary: upstream-unlocked descriptor (workspace member)	issue_1379_check_ignores_an_unlocked_descriptor_upstream_had	n/a (guards against false positives)	ok
parser	classic_deps::tests::block_dep_descriptors_reads_the_blocks_own_sub_maps, yarn_token_reads_quoted_and_bare_tokens	n/a	ok

Commands run locally:

• cargo fmt --all -- --check: clean
• cargo clippy --workspace --all-features -- -D warnings: clean
• cargo test --workspace --all-features --no-fail-fast: 13897 passed, 13 failed. Each failure is environmental and none touch this change. 12 rely on chmod-based write failures, which the sandbox's root user bypasses (*_state_write_failure_*, *_unremovable*, wire_*_failure_*, relax_loop_must_not_traverse_symlinked_root, *write_failures*, *invalidation_failure*, repair_cleanup_failure*). The other one, e2e_bun_lockb::binary_shared_bundled_record_hosted_pin_is_managed, needs patches-api.socket.dev, which the sandbox can't reach.
• scripts/yarn-classic-vex-matrix.sh 1.22.22 (real yarn, all 4 suites): 65/65 cells pass.
• No npm/pypi/gem wrapper changes were needed (the CLI behavior is the same through every wrapper).

/code-review high

Fixed: false positives on upstream-unlocked descriptors such as workspace members (findings 1 and 3), the contract's vendor --check section (5), \\-escape handling consistent with key parsing (6), the shared uuid predicate (7), and the duplicated flavor dispatch (8). Not changed:

• (2) repair runs the new check only for an artifact it judges healthy. For a missing or corrupt artifact it still redownloads first, because the remedy (yarn install) needs the vendored tarball on disk. Failing before the redownload would leave the user stuck. The next repair or vendor --check reports the dangling descriptor.
• (4) The lock is read and the key set built once per yarn-classic ledger entry. That is the same per-entry cost as the existing package-lock check, and ledgers are small.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JKRFFfsrTtKJs3Sj5BDygE

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ 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 f22885a. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review at f22885a.

• CI: 36/36 non-skipped checks green on the head (48 skipped by path filters); no main-wide failures.
• Bugbot: reviewed f22885a, no findings.
• Look at: yarn classic vendor --check/repair now flags vendored blocks whose dependency descriptors do not resolve to a lock block.
• Slack announcement not sent this run (Slack send unavailable); next run retries.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

[final reviewer] Review of f22885a: the change is correct for the case #1379 describes, and its tests fail without it (core issue_1379* 4/4 and CLI yarn_classic_check_and_repair_flag_a_dangling_dependency_descriptor pass locally; CI 36/36 green). I'm holding the brief because of one product question about when the new check fails closed.

Question. Should dangling_descriptors (yarn_classic_lock.rs:554-588) also exempt descriptors that yarn 1 never gives a block, using the project rather than only the recorded WiringRecord.original? That means link:/portal: ranges and names that are workspace members of the root package.json.

Today the only exemption is "the recorded pre-vendor block already named it". That leaves two cases where vendor --check and repair stay red on a lockfile that installs fine:

1. No recorded original. The block already pointed into .socket/vendor and the ledger had no earlier entry to carry original forward from, for example a rebuilt or lost ledger. Any upstream workspace-member or link: dependency then gets flagged.
2. A pre-Yarn classic: a patch that adds a dependency to the package's own package.json leaves vendored yarn.lock with a dangling dependency (offline frozen install fails, lock churns), and hosted silently installs without it #591 patch added a dependency on a workspace member. yarn 1 links the member and installs fine, but the check reports "installs them unpinned". The advice it gives (yarn install to lock them) never adds a block, so the check can't be cleared.

Both are rare. Fixing them means reading the workspace globs (no shared helper exists for yarn 1 yet). Not fixing them means a CI check that some users can't turn green. Please pick one:

• (a) Merge as is and file a follow-up.
• (b) Add the link:/portal: and workspace-member exemption in this PR.
• (c) Downgrade the case with no recorded original to a warning.

Smaller notes (non-blocking):

• repair runs the check only on the ArtifactHealth::Healthy path (repair.rs:476-494).``` A stale or corrupt artifact gets rebuilt and reported as success while the lock still dangles, and the next --check fails.
• In CLI_CONTRACT.md, the vendor_dep_manifest_unlocked row is still classed refused, but repair now emits it as failed. The new sentence explains this; the category cell reads oddly next to it.
• The doc comment says keys and sub-map entries "compare alike", but yarn_token keeps \" while split_key_patterns has no escape handling. npm names can't contain ", so this has no practical effect.

Labelled agent:needs-human for the question above. Once it's answered, the burn-down or I can make the change and post the brief.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Yarn 1 is not a high priority and for these weird features the best thing to do is pick the simplest option. If supporting them adds too much complexity then better to ignore for now. It doesn't seem like many packages would be installed using this mechanism and if a user did do it, then we probably wouldn't want to patch it.