[Mikola Lysenko (mikolalysenko)]

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

Fixes #1918

What changes for users

A quoted value in pnpm-workspace.yaml, .yarnrc.yml or a pre-pnpm-10 YAML
node_modules/.modules.yaml now reads the same no matter which feature reads
it. Before this PR, modulesDir: 'it''s', lockfileDir: "a'" or a duplicated
packageManager: key could resolve to different directories, registries or
pnpm versions depending on the caller (crawler, hosted governing-root lookup,
version gate, redirect upstream, trust guidance).

Root cause

"Read the last top-level key: of a pnpm YAML settings file and unquote it" was
written several times on top of the shared formats::pnpm::workspace::top_level_key,
each copy with its own unquote rule:

• workspace::yaml_top_level_value: trim_matches(['"', '\'']) (strips unpaired
  and repeated quotes, leaves '' doubled)
• governing_root::unquote_scalar: one matched pair, no '' decoding
• pnpm_layout::unquote_yaml_scalar: '' + JSON escapes
• .modules.yaml read three ways (npm_crawler, formats/pnpm/version.rs,
  redirect/upstream/npm.rs): first key vs last key, comment kept vs stripped
• hosted/guidance.rs trustLockfile (first key, trim_matches) and the nested
  registries: / yarn npmScopes readers (trim_matches), found by
  /code-review high

Fix

• formats::pnpm::workspace::top_level_scalar(text, key): last top-level key,
  value decoded by yaml_scalar (the file's own single/double-quote decoders);
  empty is Some(""), a block-scalar header (|, >-) is None.
  yaml_top_level_value is that with empty read as unset.
• double_quoted now decodes YAML 1.2 escapes (\x, \u incl. surrogate pairs,
  \U, \b, ...) and refuses malformed ones; strip_comment honours \" so an
  escaped quote before # no longer truncates the value.
• New formats::pnpm::modules_yaml::field: the one .modules.yaml reader (JSON on
  pnpm 10+, the shared scalar reader before) used by the crawler's
  virtualStoreDir, the version gate and the redirect upstream.
• governing_root, pnpm_layout, guidance (trustLockfile), yaml_block_value
  and yaml_path_in route through the shared decoder; the three private unquote
  helpers and pnpm_spec_version's re-trim are gone.

Not in this PR

The issue's later audit comment also points at the pnpm lockfile grammar's
one-pair unquotes (grammar::unquote, lines::unquote_value) and the two
pnpm@x version parsers. Those read lock entries and version specs, not settings
scalars, and have many callers; folding them is a separate change.

Tests (red → green)

Written first on 0295a9d, all four failed on main:

Reader	Test	Before
yaml_top_level_value	workspace::tests::yaml_top_level_value_decodes_yaml_quoting	it''s
governing_root lockfileDir	workspace_lockfile_dir_reads_the_top_level_key	../it''s
.modules.yaml packageManager (version gate)	version::tests::modules_yaml_pin_reads_the_last_key	first key (9.0.0)
.modules.yaml virtualStoreDir (crawler)	npm_crawler::tests::modules_yaml_virtual_store_dir_uses_the_shared_reader	.pnpm # default

Plus workspace::tests::top_level_scalar_table (plain, comment, escapes, surrogates,
'', empty, unpaired quotes, malformed escape, block scalar, BOM, CRLF),
modules_yaml::tests::reads_json_and_yaml_records_alike,
guidance::tests::workspace_trust_plan_reads_the_last_key_as_yaml,
upstream::npm::tests::nested_settings_values_decode_yaml_quoting.

Local gates:

• cargo fmt --all -- --check: ok
• cargo clippy --workspace --all-features -- -D warnings: ok
• cargo test --workspace --all-features --no-fail-fast: all pass except tests that
  fail identically on main in this container (it runs as uid 0, so chmod-based
  write-failure tests can't fail writes, and the live patch API is unreachable):
  core copy_tree/vlt_heal/pypi_poetry/pypi_requirements write-failure tests,
  in_process_redirect x3, repair x2, e2e_bun_lockb x2 (live blob fetch, see Fix bun.lockb nightly e2e reaching the live blob API #1905).
  Checked on origin/main for the core and in_process_redirect ones.
• e2e_safety_pnpm --ignored (4 ok), e2e_redirect_pnpm_build --ignored --skip pnpm_pinned_matrix (11 ok, pnpm 10.28), scan_pnpm_relocated_store_cwd_e2e (41 ok),
  in_process_redirect_pnpm (23 ok).
• No wrapper (npm/, pypi/, gem/) change needed: they only dispatch to the binary.

/code-review high

9 findings. Fixed: escaped-quote comment truncation, missing escapes / surrogate
pairs, registries:/yarn nested trim, trustLockfile first-key read, \u+041
accepted, block-scalar header read as value, pnpm_spec_version re-trim, the
version.rs one-line wrapper. Not fixed: ".modules.yaml parsed into a full JSON
tree per call": the callers already did that before this PR, so it's not a
regression, and caching the record is a separate perf change.

https://claude.ai/code/session_011f9aeRTcq7ezZiUt7xTkrb

---

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

[Mikola Lysenko (mikolalysenko)]

Ready for review at fab9b081.

• CI: 32/32 ran green, 50 path-skipped; no main-wide failures.
• Bugbot: reviewed fab9b081, no findings.
• Mergeable, no CHANGELOG.md changes, no unresolved review threads.
• Reviewer focus: the shared formats::pnpm::workspace::top_level_scalar unquote rule now used by all six pnpm YAML readers ('' decoding, last-key-wins).

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does
Makes every pnpm/yarn YAML settings read (pnpm-workspace.yaml lockfileDir/modulesDir/trustLockfile, .yarnrc.yml, pre-pnpm-10 YAML .modules.yaml) go through one reader: last top-level key wins, value decoded with real YAML quoting ('' → ', double-quoted escapes). Before, five-plus private unquote helpers disagreed on quoting and on first vs last duplicate key.

Risk: low. Read-only parsing of settings files, no writes changed. The behaviour shifts only for odd inputs (quoted values with ''/escapes, duplicate keys, unpaired quotes), and each moves toward what pnpm's own YAML parser reads. Well covered by table tests.

Look here

• workspace.rs:66 top_level_scalar and :85 yaml_scalar: the one shared reader/decoder
• workspace.rs:363 double_quoted: full YAML 1.2 escapes, malformed escape now None. This decoder also backs quoted keys and glob_scalar, so an invalid escape like "pkgs\*" is now a parse refusal rather than kept verbatim (pnpm itself rejects such a file, so this is correct)
• workspace.rs:337 strip_comment: \" no longer closes a double-quoted scalar; shared with pnpm_build_approvals and pnpm_lock
• modules_yaml.rs:12 field:`` one .modules.yaml reader for crawler, version gate and redirect upstream
• guidance.rs:409: trustLockfile now reads the last key (was first)

Verified

• Read the full diff (9 files, +293/−112) and every caller of the changed helpers (strip_comment, double_quoted, pnpm_spec_version): no caller passed still-quoted text that relied on the removed trim_matches
• cargo test -p socket-patch-core --lib for workspace, modules_yaml, version, pnpm_layout, governing_root, guidance, upstream::npm: 131 passed
• cargo clippy -p socket-patch-core --all-targets: nothing in touched files
• CHANGELOG.md untouched; CI 32/32 green (50 path-skipped); Bugbot reviewed fab9b081, no findings; no unresolved threads

Changes I made: none

Open questions: none. The lockfile-grammar unquotes are deliberately left for a follow-up, as the description says

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

---

Generated by Claude Code

[Wenxin Jiang (Wenxin-Jiang)]

[agent] Reviewed fab9b0815f88ab5e2ccf9aa2dff270f992ae1729. Checked shared YAML scalars, escapes, duplicate-key precedence and pnpm version/store/config readers. No major correctness issues found. Validation: source and regression-test review plus the available exact-head CI checks; no local test pass is claimed.