Skip to content

Fix pnpm-workspace.yaml key detection and appends (#400, #402) - #414

Open
Mikola Lysenko (mikolalysenko) wants to merge 8 commits into
mainfrom
agent/fix-pnpm-workspace-yaml-keys
Open

Fix pnpm-workspace.yaml key detection and appends (#400, #402)#414
Mikola Lysenko (mikolalysenko) wants to merge 8 commits into
mainfrom
agent/fix-pnpm-workspace-yaml-keys

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

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

Fixes #400
Fixes #402

Summary

After a successful-looking scan --mode hosted or vendor, a project's pnpm-workspace.yaml could stop loading, and every later pnpm install failed. This happened in two cases:

With this change, both editors read the file the way pnpm's YAML parser does. Existing keys are recognised in every spelling, new keys go inside the document (before a ... marker), and shapes a line edit can't safely extend are left untouched with a clear reason.

Root cause

socket-patch edits pnpm-workspace.yaml in two places: the hosted trustLockfile: true write (plan_workspace_trust in hosted/guidance.rs) and the vendored overrides: mirror (check_workspace_override / apply_workspace_override / revert in vendor/pnpm_lock.rs). Both were line splices that:

  1. found an existing top-level key only by a literal line prefix (trustLockfile:, overrides:), and
  2. appended block-style lines after the last non-empty line, without checking that the document is a single block mapping.

Fix

crates/socket-patch-core/src/formats/pnpm/workspace.rs is a new shared helper:

  • top_level_key: parses a column-0 mapping key that is plain, single-quoted ('' escapes) or double-quoted (backslash escapes), allowing blanks before the colon. It returns the key and its inline value with any trailing comment stripped.
  • block_insert_point: finds where a new top-level key belongs (after the last content line, before a ... end marker). It returns an error for a flow-style root, a root on the --- line, an indented or sequence root, or a second document.
  • block_section_bounds: finds a block section under any key spelling. Column-0 # comments and stray \r lines stay inside the section.

Callers:

  • Hosted trust (plan_workspace_trust): uses both helpers. The new TrustPlan::Unsupported(reason) writes nothing and emits the redirect_pnpm_trust_lockfile warning with both manual recoveries (pnpm install --trust-lockfile, or add the key yourself) and the don't-rebuild caution (pnpm_trust_workspace_unsupported_detail).
  • Vendored overrides: the pre-flight check refuses unspliceable shapes before any write, and refuses a quoted or spaced inline overrides mapping. The section lookup used by the check, the edit, the revert and the empty-section prune accepts every key spelling, and takes the entries' indent from the first non-comment line. The append is placed by block_insert_point.
  • Hosted rollback: the pnpm_trust_lockfile_left check (upstream/npm.rs) recognises the key in any spelling.
  • CLI_CONTRACT.md: documents this behaviour.

No wrapper changes are needed. npm/, pypi/ and gem/ only dispatch to the binary.

Test evidence

Red on main (commit d3fb884, tests only), green with the fix:

Issue Variant Test
#402 hosted "trustLockfile": false, trustLockfile : false, 'trustLockfile': false # c → respected (UserSet) plan_workspace_trust_reads_quoted_and_spaced_keys (cli lib)
#402 hosted 'trustLockfile': true, "trustLockfile" : "true", trustLockfile: true # c → no-op same
#402 vendored "overrides":, 'overrides':, overrides :, overrides: # c → entry inserted inside the section quoted_or_spaced_overrides_key_is_edited_in_place
#402 vendored conflict inside a quoted section is refused quoted_overrides_section_conflict_is_refused
#402 quoted or spaced inline mapping is refused quoted_inline_overrides_mapping_is_refused
#402 revert removes our key from a quoted section revert_removes_our_key_from_a_quoted_section
#400 hosted ...-terminated file → key inserted before ...; flow root, --- {…}, multi-document → not appended, warning names the reason plan_workspace_trust_respects_the_document_shape
#400 vendored ... → section before the marker; leading --- is fine document_end_marker_keeps_the_section_in_the_document
#400 vendored flow, multi-line flow, --- {…}, second document, indented root → refused before writing unspliceable_document_shapes_are_refused
#400 / #402 end to end: real hosted scan over a quoted opt-out, a spaced opt-out, a flow file and a ... file; the lock is redirected and the workspace file is untouched (or gains the key inside the document) in_process_redirect_pnpm::hosted_trust_edit_reads_the_workspace_yaml_shape
Bugbot 1 a column-0 comment inside overrides: doesn't hide later entries (check, edit, revert) column_zero_comment_inside_the_section_keeps_later_entries
Bugbot 2 a leading comment doesn't set the entries' indent (4-space section) leading_comment_does_not_set_the_entry_indent
helper key spellings, insert points and section bounds formats::pnpm::workspace::tests::*

Red→green: on main, all 8 issue tests in the cli lib and core lib fail (6 vendored + 2 hosted). Each Bugbot test fails without its fix. All pass with the fix. The in-process test was added after the fix and wasn't run against main.

Local commands (Linux, Node 22, pnpm 10.28.0, toolchain 1.93.1):

  • cargo fmt --check: the files this PR changes are fmt-clean. main itself isn't fmt-clean under the pinned toolchain (in about 120 files), and CI doesn't run cargo fmt, so this PR doesn't touch those files.
  • cargo clippy --workspace --all-features -- -D warnings: ok
  • cargo test --workspace --all-features --no-fail-fast: everything passes except tests that rely on chmod to force a write failure (covgap_commands_vendor state-write ×3, in_process_redirect write-failure ×3, repair ×2, core copy_tree / vlt_heal / pypi_poetry / pypi_requirements write-failure ×4). This sandbox runs as uid 0, which ignores read-only modes. None of these tests touch pnpm-workspace.yaml.
  • --test e2e_vendor_pnpm_build: 15 passed. --test e2e_redirect_pnpm_build -- --ignored --skip pnpm_pinned_matrix: 8 passed. --test in_process_redirect_pnpm: 11 passed.
  • e2e_safety_pnpm --ignored can't run here: the proxy refuses patches-api.socket.dev. CI covers it.

CI on 3c5f8f8: every compatibility workflow is green (npm, pnpm, Bun, vlt, Composer, Go, Poetry, PDM, Pipenv). The Pipenv 2022.12.19 extras and PDM 2.20.1 static-urls cells failed once, in code this PR doesn't touch, and passed on their single re-run. Bugbot found no new issues on 3c5f8f8.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Kmie1ncmqvPzYfYc2ustqm

Generated by Claude Code

@claude
Start fix for #400, #402
e7a34f2 
Assisted-by: Claude Code:claude-opus-5-5
This was referenced Oct 1, 2026
Open
Open
@claude
Add failing tests for pnpm-workspace.yaml edits
d3fb884 
Quoted or space-before-colon top-level keys (#402) and flow-style,
'...'-terminated or multi-document files (#400) are mishandled by both
the hosted trustLockfile write and the vendored overrides write. These
tests pin the expected behaviour before the fix.

Assisted-by: Claude Code:claude-opus-5-5
@claude
Read pnpm-workspace.yaml keys as YAML does
19963f2 
The hosted trustLockfile write and the vendored overrides write found
an existing key only by a literal line prefix and appended new keys at
end of file. Quoted or space-before-colon keys were then duplicated,
and flow-style or '...'-terminated files were corrupted, so every
later pnpm install failed while the command reported success.

Both editors now share one helper that recognises every key spelling
pnpm reads and finds where a new key belongs. Keys go before a '...'
marker. Shapes a line edit cannot extend are refused before anything
is written, with the manual recoveries in the warning.

Fixes #400, #402.

Assisted-by: Claude Code:claude-opus-5-5
@claude
Test the hosted trust edit on real scans
7e8508e 
Runs scan --mode hosted over a quoted opt-out, a spaced opt-out, a
flow-style file and a '...'-terminated file, and checks the workspace
file is left as is (or gains the key inside the document) while the
lock is still redirected.

Assisted-by: Claude Code:claude-opus-5-5
@claude
Nest workspace YAML tests in the test module
ffb82ed 
Keeps the new regression tests inside the existing test module so the
crate stays clean under clippy's items-after-test-module lint.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 1, 2026 04:16
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot]
cursor Bot reviewed Oct 1, 2026
View reviewed changes

@cursor cursor Bot left a comment •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Comment thread crates/socket-patch-core/src/formats/pnpm/workspace.rs
@claude
Keep comment lines inside workspace sections
9e1814e 
A '#' comment at column 0 does not end a YAML block mapping, but the
overrides section lookup treated it as the next key. Entries after
such a comment were then invisible to the vendored conflict check,
the edit and the revert. The lookup now skips comments and stray
carriage returns.

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

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot]
cursor Bot reviewed Oct 1, 2026
View reviewed changes

@cursor cursor Bot left a comment •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Comment thread crates/socket-patch-core/src/formats/pnpm/workspace.rs
@claude
Drop unrelated formatting changes
94de5f6 
An earlier commit ran rustfmt across the whole workspace and swept
import reordering and line wrapping in 120+ files that this fix does
not touch. Restore those files to main so the change is reviewable;
only the pnpm-workspace.yaml fix and its tests remain.

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

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot]
cursor Bot reviewed Oct 1, 2026
View reviewed changes

@cursor cursor Bot left a comment •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

@claude
Ignore comments when reading override indent
3c5f8f8 
The overrides section keeps column-0 comments now, but the entry
indent was still taken from the first non-empty line. A leading
comment then forced the 2-space default, so a 4-space section was
skipped by the conflict check, the edit and the revert, and a
duplicate key could be added. Comment lines are now skipped.

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

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

BugBot review

Generated by Claude Code

cursor[bot]
cursor Bot reviewed Oct 1, 2026
View reviewed changes

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 3c5f8f8. Configure here.

@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

[agent] One failing check on 3c5f8f8: Pipenv compatibility / matrix (ubuntu-latest, 2022.12.19 2026.8.0, crlf marker-excluded extras category).

25 of 26 cells pass. The one failure is Pipenv 2022.12.19 / extras / hosted, where rescanIdempotent fails because the harness's pipenv lock relock step exited 1 (relock exit 1). The same cell passes on Pipenv 2026.8.0 and in vendored mode.

This isn't caused by this PR:

  • The PR touches no Python or Pipenv code, only pnpm-workspace.yaml handling.
  • The same workflow passed on this branch's previous two heads, 9e1814e (run 36815304905) and 94de5f6 (run 36815751498).
  • The only change since 94de5f6 is a pnpm-only edit to how the overrides indent is read (ws_overrides_section).
  • A relock that exits 1 against live PyPI on the oldest Pipenv points to resolution against the live index, not to socket-patch.

I don't have a fix to port, because none is needed in this PR. I'll re-run the failed job once when the run completes. If it fails again, I'll treat it as real and dig in.

Generated by Claude Code

@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

[agent] A second failing check on 3c5f8f8: PDM patch compatibility / native (ubuntu-latest, 2.20.1).

1 of 47 cells fails: PDM 2.20.1 / static-urls / vendored. The other 46 are PASS or the expected REF/UNS.

This isn't caused by this PR:

I don't have a fix to port, because none is needed in this PR. Like the Pipenv job above, I'll re-run the failed job once when its run completes. If it fails again, I'll treat it as real and pull the cell's result.json.

Generated by Claude Code

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

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator Author

[burn-down agent] Ready for review on 3c5f8f8: mergeable, 0 commits behind main.

  • CI: 97/97 completed checks green on 3c5f8f8 (3 skipped by workflow conditions).
  • Bugbot: reviewed 3c5f8f8, no new issues; 0 unresolved review threads.
  • Reviewer note: Touches both pnpm-workspace.yaml editors (hosted guidance + vendored overrides); new parser in formats/pnpm/workspace.rs is the core to review.

Generated by Claude Code

This branch has not been deployed

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

Reviewers

@cursor cursor[bot] cursor[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

2 participants

@mikolalysenko @claude