[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.

Review follow-up

• The reviewer found that unbounded block nesting overflowed the stack. Fixed in b75e13c: past 64 levels the file reads as unreadable. Covered by the regression test deep_nesting_is_unreadable_not_a_stack_overflow.

/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

---

Note

Medium Risk
Changes how untrusted project .yarnrc files drive npm crawl roots and yarn redirect paths (global folder / offline mirror), with behavior shifts for edge-case syntax; mitigated by yarn-parity tests and nesting limits.

Overview
Fixes #1901 by replacing three ad-hoc .yarnrc line parsers with a single formats::yarn::classic_rc module that mirrors yarn 1.22: lockfile grammar first, YAML fallback when the lockfile parse fails anywhere, and no settings when both fail.

The npm crawler now derives --modules-folder / --install.modules-folder from that reader (e.g. key: 'deps' works via YAML; key 'deps', ~/x, and unquoted C:\deps are unset like yarn). The hosted redirect uses the same reader for yarn-offline-mirror (only boolean false disables the mirror) and global-folder. Inline tokenizers in the crawler and redirect are removed.

The new parser includes a 64-level nesting cap on untrusted .yarnrc files and extensive regression tests, including an async crawl that roots at deps/ only when yarn would.

^{Reviewed by Cursor Bugbot for commit a8a8aff. Configure here.}

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[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

[Wenxin Jiang (Wenxin-Jiang)]

[agent] Reviewed 11551ef. The shared parser and caller precedence look correct. One low-priority robustness finding is inline; no major correctness issue found. Validation: source/regression-test review and available exact-head CI, plus an optimized standalone probe of the actual parser module.

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

[agent] Removed from the merge queue because clippy failed on the queue commit (run 41489069374): an unused import: crate::formats::text::strip_bom in npm_crawler.rs. This PR moved the crawler's .yarnrc reads into classic_rc, and main has since dropped the import's last other use, so neither side failed alone, only the combination. I merged the latest main and removed the import in a8a8aff. Locally, cargo clippy --locked --workspace --all-features -- -D warnings is clean, the yarn and npm_crawler lib tests pass (510) and crawler_npm_e2e passes (128). The PR needs to be re-queued once CI is green.

---

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

[Mikola Lysenko (mikolalysenko)]

[final reviewer] Re-enqueued at a8a8affd0: approved (approval stays valid after the post-approval pushes), ci-ok and clippy green, mergeable, no unresolved threads. Armed squash auto-merge, which put it back into the merge queue after the earlier dequeue.

---

Generated by Claude Code