GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Fix pnpm YAML settings read by drifted readers (#1918) by mikolalysenko · Pull Request #1973 · SocketDev/socket-patch

Conversation

Mikola Lysenko (mikolalysenko) commented Nov 2, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Start fix for #1918

75313f3

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) mentioned this pull request

Nov 2, 2026

Read pnpm YAML settings through one formats::pnpm scalar reader: three top-level-value readers drifted on quotes and empty values, and .modules.yaml is parsed twice #1918

Closed

Claude (claude) added 3 commits

November 2, 2026 06:31


          Add regression tests for pnpm YAML scalar reads

0295a9d

The pnpm settings readers decode quoted values and duplicate keys
differently depending on the caller (#1918). These tests pin the
pnpm YAML reading: '' is one quote, an unpaired quote is kept, the
last top-level key wins and a trailing comment is not the value.

Assisted-by: Claude Code:claude-opus-5-5


          Read pnpm YAML settings through one reader

e913c96

A quoted value in pnpm-workspace.yaml, .yarnrc.yml or a YAML
.modules.yaml was decoded differently depending on which feature
read it: modulesDir: 'it''s' or lockfileDir: "a'" could resolve to
different paths, and a duplicated packageManager key in
.modules.yaml was read as the first value by one reader and the last
by another (#1918).

Every top-level settings scalar now goes through
formats::pnpm::workspace::top_level_scalar, which decodes the value
with the file's own YAML single/double-quote decoders (last key wins,
unpaired quotes kept, empty kept as empty). .modules.yaml is read
once, by formats::pnpm::modules_yaml::field, for the crawler's
virtualStoreDir, the version gate and the redirect upstream. The
three private unquote helpers are gone.

Assisted-by: Claude Code:claude-opus-5-5


          Route nested pnpm settings through the decoder

fab9b08

Review follow-ups for #1918: a double-quoted value with an escaped
quote before " #" is no longer cut at the quote, YAML escapes
(surrogate pairs, \x, \U, \b) decode as pnpm reads them and a
malformed one is not guessed, a block scalar header is not read as
the value, and the trustLockfile, registries and yarn nested readers
use the same decoder instead of trimming quote characters.

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) marked this pull request as ready for review

November 2, 2026 07:08

Mikola Lysenko (mikolalysenko) commented Nov 2, 2026

Copy link

Copy Markdown

Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot] reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cursor Bot reviewed

Nov 2, 2026

View reviewed changes

cursor Bot left a comment

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label

Nov 2, 2026

Mikola Lysenko (mikolalysenko) commented Nov 2, 2026

Copy link

Copy Markdown

Collaborator Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) commented Nov 2, 2026

Copy link

Copy Markdown

Collaborator Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) enabled auto-merge

November 2, 2026 09:05

Wenxin Jiang (Wenxin-Jiang) approved these changes

Nov 2, 2026

View reviewed changes

Wenxin Jiang (Wenxin-Jiang) left a comment

Copy link

Copy Markdown

Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Mikola Lysenko (mikolalysenko) added this pull request to the merge queue

Nov 2, 2026

Merged via the queue into main with commit 7178662

Nov 2, 2026

82 checks passed

Uh oh!

There was an error while loading. Please reload this page.

Mikola Lysenko (mikolalysenko) deleted the agent/fix-pnpm-yaml-scalar-reader branch

November 2, 2026 16:49

Reviewers

cursor[bot]

cursor[bot] left review comments

Wenxin Jiang (Wenxin-Jiang)

Wenxin-Jiang approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

Ready for review

Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Read pnpm YAML settings through one formats::pnpm scalar reader: three top-level-value readers drifted on quotes and empty values, and .modules.yaml is parsed twice

Uh oh!

There was an error while loading. Please reload this page.

3 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

Mikola Lysenko (mikolalysenko) commented Nov 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

What changes for users

Root cause

Fix

Not in this PR

Tests (red → green)

/code-review high

Uh oh!

Mikola Lysenko (mikolalysenko) commented Nov 2, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Nov 2, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Nov 2, 2026

Final review brief

Uh oh!

Wenxin Jiang (Wenxin-Jiang) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mikola Lysenko (mikolalysenko) commented Nov 2, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.