Skip to content

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

Description

[agent] Filed by the scheduled architecture audit (audit-core). Register: A44.

Kind: refactor. Register ID: A44.

Problem

"Last top-level key: of a YAML settings file, unquoted" is written three times on top of the shared formats::pnpm::workspace::top_level_key:

  • formats::pnpm::workspace::yaml_top_level_value uses trim_matches(['"', '\'']), which strips unpaired and repeated quotes and leaves '' doubled, and drops empty values (workspace.rs L59-L68);``
  • hosted::governing_root::workspace_lockfile_dir + unquote_scalar strips one matched pair only (governing_root.rs L1246-L1265);```
  • crawlers::pnpm_layout::workspace_modules_dir + unquote_yaml_scalar handles '' and JSON escapes, and keeps an empty value so it beats .npmrc (pnpm_layout.rs L145-L170).````

npm_crawler (L1438) carries a fourth unquote copied from pnpm_layout::unquote_yaml_scalar, and .modules.yaml is parsed twice (npm_crawler::parse_modules_yaml_virtual_store_dir, formats/pnpm/version.rs::modules_yaml_package_manager). So modulesDir: 'it''s' and lockfileDir: "a'" read differently depending on the caller. Unexecuted.

Proposed change

  1. One formats::pnpm::workspace::top_level_scalar(text, key) -> Option<String> using unquote_yaml_scalar's rules (moved there), returning an empty string as Some(""); callers that want "empty = unset" filter.
  2. Delete unquote_scalar, yaml_top_level_value's trim_matches, workspace_modules_dir's copy and the npm_crawler unquote.
  3. Move the .modules.yaml reader to formats/pnpm and have npm_crawler call it.

Size

4–5 files, ~-60 lines net.

Acceptance criteria

  • Behavior unchanged for plain and matched-quote values (existing pnpm workspace, governing_root and pnpm_layout tests pass).
  • New table test of the shared reader: plain, "…" with escapes, '…''…', empty, unpaired quote.

Activity

  1. added
    arch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)
    refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code
    on Nov 1, 2026
  2. mikolalysenko commented on Nov 1, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Triage: priority:p1 (pnpm). Confirmed on main @ cbbbdb9: yaml_top_level_value, governing_root::unquote_scalar and pnpm_layout::unquote_yaml_scalar are separate unquote rules. Not a duplicate (#1347 unified the modules-dir lookup, not the scalar reader).


    Generated by Claude Code

  3. mikolalysenko commented on Nov 1, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] The audit-core run of 2026-11-01 found that .modules.yaml has a third reader, and the readers have drifted.

    The three readers:

    • formats/pnpm/version.rs L103-L112 (modules_yaml_package_manager)
    • patch/redirect/upstream/npm.rs L1597-L1615``` (modules_yaml_pnpm_major, via `yaml_top_level_value`)
    • npm_crawler's, already listed in this issue

    Executed in a scratch test on cbbbdb9, with the duplicate key packageManager: pnpm@9.0.0\npackageManager: pnpm@11.0.0:

    • root_only_workspace_breaks_add reads the first key: Some("pnpm@9.0.0 …").
    • modules_yaml_pnpm_major reads the last key: Some(11).

    The "pnpm@x" version parsers also differ: pinned_version + version_bounds on one side, pnpm_spec_version/pnpm_version on the other.

    Separately, workspace::yaml_top_level_value (workspace.rs L62-L68) unquotes with trim_matches. That reads 'it''s' as it''s and strips an unbalanced 'a" to a, even though the same file has proper single_quoted/double_quoted decoders.

    The one-pair unquote is also written 3× identically: grammar.rs L74-L79,````` lines.rs L193-L203 and governing_root::unquote_scalar. Suggest folding all of these into this issue's one reader.


    Generated by Claude Code

  4. mikolalysenko commented on Nov 2, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Claiming this issue (shared root cause: pnpm YAML settings files are read by several hand-written top-level-scalar and .modules.yaml readers with different unquote rules). Branch: agent/fix-pnpm-yaml-scalar-reader. Claim-ID: 2026-11-02T06:23:21Z-d6e508


    Generated by Claude Code

  5. mikolalysenko commented on Nov 2, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Fix in progress: #1973


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:triagedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)priority:p1refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions