[Mikola Lysenko (mikolalysenko)]

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

Fixes #1901

Summary

Yarn classic's .yarnrc was read by two tokenizers that had drifted:
the npm crawler's parse_yarnrc_modules_folder / split_yarnrc_token,
and the hosted redirect's yarnrc_setting / yarnrc_token. The
offline-mirror read was a third variant, using unquote_rc_value. All
of them now go through one reader, formats::yarn::classic_rc, which
reads the file the way yarn 1.22 does.

Root cause

Each module tokenized .yarnrc line by line in its own way. yarn 1
actually parses the whole file with its lockfile grammar
(lockfile/parse.js). If that fails anywhere in the file, yarn
re-reads the whole file as YAML (js-yaml safeLoad, FAILSAFE schema).
If YAML fails too, yarn exits with an error. No per-line reader can
model that. For example, one unreadable yarn-path ~/y.js line makes
yarn ignore --modules-folder deps on the line above.

What yarn 1.22.22 actually does

I ran yarn install with each .yarnrc below and recorded where
is-number landed:

.yarnrc	yarn installs into
--modules-folder deps / "deps" / deps # c / --modules-folder:deps	deps
--modules-folder: 'deps', "--modules-folder": 'deps', --modules-folder : 'deps' (YAML fallback)	deps
--modules-folder: my deps (YAML fallback)	my deps
--modules-folder, --install.modules-folder deps	deps
--modules-folder 'deps', --modules-folder:'deps', ~/x, C:\deps, C:deps, deps, x, trueDeps, a tab separator	node_modules (not read)
--modules-folder deps + yarn-path ~/y.js; odd indentation	node_modules (the file falls to YAML and reads as a plain string)
"--modules-folder" 'deps', false, 0, a nested block	yarn exits with an error

Correction to the issue: the issue says the redirect's reader gets
--modules-folder 'deps' right (deps). It doesn't: yarn ignores that
line. Both old readers were wrong for it.

Changes

• crates/socket-patch-core/src/formats/yarn/classic_rc.rs (new):
  • yarn's tokenizer and parser, ported from lockfile/parse.js:
    booleans, numbers, a, b multi-keys, indent rules, nested blocks,
    and a lone \r.
  • The YAML fallback, using the serde_saphyr parser already in use
    for .yarnrc.yml. It matches js-yaml FAILSAFE: an empty value is
    null, a duplicate key or an unknown tag makes the file unreadable,
    and an alias reads its anchored value.
  • Public API: settings / scalar / take_str.
• npm_crawler::parse_yarnrc_modules_folder is one settings call.
  split_yarnrc_token and the crawler's line loop are deleted.
• patch/redirect/yarnrc.rs: yarnrc_setting and yarnrc_token are
  deleted. global-folder and yarn-offline-mirror read through
  classic_rc. The mirror is disabled only by a boolean false,
  because yarn's getOfflineMirrorPath checks === false and resolves
  a string "false" as a path.

Tests (red → green)

• npm_crawler::tests::yarnrc_modules_folder_reads_like_yarn (new) and
  npm_crawler::tests::test_yarnrc_single_quoted_modules_folder_is_a_crawl_root
  (new). The second crawls a real tree: deps/ for
  --modules-folder: 'deps', and only node_modules/ for
  --modules-folder 'deps'.
• npm_crawler::tests::test_parse_yarnrc_modules_folder: unquoted
  C:\deps is now unset, matching yarn.
• formats::yarn::classic_rc::tests (new) pin every table row, plus
  the js-yaml and lone-\r cases.
• Red on main's reader: cargo test -p socket-patch-core --lib -- yarnrc had 3 failures. Green: cargo test -p socket-patch-core --lib -- classic_rc yarnrc yarn mirror gives 466 passed. Also green:
  crawler_npm_e2e (123 passed), plus the yarn and modules-folder cases
  in e2e_vex_redirect, e2e_vex_vendor and
  in_process_alternate_installers (real yarn 1.22.22).
• cargo fmt --all -- --check and cargo clippy --workspace --all-features -- -D warnings are clean.
• cargo test --workspace --all-features --no-fail-fast locally: every
  failure is environmental and none involves yarn. The container runs as
  root, so the permission-based write-failure tests in core lib,
  in_process_redirect and repair don't fail as intended. The patch
  API is unreachable through the sandbox proxy, and the local bun
  version differs from CI's (e2e_bun_lockb). CI runs these normally.
• No npm, PyPI or gem wrapper reads .yarnrc, so none needed changes.

Per-issue checklist:

• Tokenize yarn-classic .yarnrc lines in one formats/yarn helper: the crawler's copy keeps single quotes that redirect's reader strips #1901: one reader in formats/yarn/, used by the crawler and by
  redirect. A single-quoted test, and an untokenizable-value test
  ignored by both callers.

/code-review high

I ran it twice. The second pass was on the whole-file reader. I fixed
every confirmed finding: the YAML null value, duplicate keys, tags,
aliases, \r handling, an empty comma key, string "false", double
parsing, and stale docs. One finding is not fixed here:

• Rc hierarchy. When a nearer .yarnrc is unreadable, or sets an
  empty, boolean or numeric --modules-folder, the crawler falls back
  to an ancestor's value. yarn would error or behave differently. That
  is yarnrc_modules_folder's hierarchy merge, not the tokenizer.
  Changing it would affect a different code path, which Fix crawlers missing parent dirs under --cwd . (#1902) #1906 also
  touches. Left as a follow-up.

https://claude.ai/code/session_011f9aeSZeF6iZU51H3F7GgF

---

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 11551ef. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Labeled Ready for review by the burn-down agent.

• Head: 11551ef6
• CI: 32/32 green (50 skipped), mergeable
• Bugbot: reviewed 11551ef6, no findings; no open review threads
• Reviewer focus: the new shared classic .yarnrc tokenizer in formats/yarn/classic_rc.rs now backs both the npm crawler and the redirect writer, so check quoting/comment edge cases match what yarn 1 accepts.

Slack announcement could not be sent in this run (no Slack send tool available); the next run will retry.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief (11551ef)

What it does: Yarn classic's .yarnrc was read by two drifted per-line tokenizers: the npm crawler's --modules-folder read and the hosted redirect's global-folder / yarn-offline-mirror read (#1901). This PR adds one reader, formats::yarn::classic_rc, that ports yarn 1.22's whole-file parse: the lockfile grammar first, then js-yaml's FAILSAFE YAML fallback if any line fails. Both callers now use it.

Risk: medium. The parsing is well pinned by tests checked against real yarn 1.22.22. The behavior change to watch is that results are now per file, not per line, as in yarn. One line yarn can't tokenize (e.g. yarn-path ~/y.js) means the file is read as YAML instead, which can drop a --modules-folder, global-folder or mirror setting another line sets. Some values that used to be read are now unset (unquoted C:\deps, --modules-folder 'deps'), and a quoted yarn-offline-mirror "false" is now a path rather than "disabled" (yarn checks === false).

Look here:

• formats/yarn/classic_rc.rs:77: tokenize, a port of yarn's lockfile/parse.js tokenizer
• formats/yarn/classic_rc.rs:161:`` parse_block, the parser (multi-keys, nested blocks, indents)
• formats/yarn/classic_rc.rs:246:``` yaml_fallback (FAILSAFE: empty is null, duplicate key or unknown tag fails the file)
• patch/redirect/yarnrc.rs:137: the mirror is disabled only by a boolean false
• crawlers/npm_crawler.rs:345: --modules-folder read is now one settings call

Verified: Read the full diff (4 files, +587/-141) and checked the port line by line against yarn 1.22's tokenise and Parser.parse: the escaped-quote rule, the lone \r and indent handling, true/false prefix tokens, and the "Invalid number of spaces" check all match. Comment tokens are kept in the stream rather than skipped as in yarn's next(), but that gives the same result for every top-level setting. The npm crawler test that changed (C:\deps unquoted is now unset) matches what yarn does. CI: ci-ok and clippy green, 32/32 ran checks pass (50 skipped by path filters). Bugbot reviewed this head with no findings, and there are no open review threads. CHANGELOG.md is untouched. I didn't run tests locally; CI covers the change.

Changes I made: none.

Open questions: none blocking. Reviewer call: accept the stricter whole-file semantics above. They match yarn, but a user with one stray unreadable line in .yarnrc will now see their --modules-folder ignored, which is also what yarn itself does.

Auto-merge is armed, so approving sends it straight to the merge queue.

---

Generated by Claude Code