Skip to content

fix: avoid workspace state parse crashes#12094

Merged
zkochan merged 2 commits into
pnpm:mainfrom
qybaihe:fix-12020-workspace-state-race
Jun 1, 2026
Merged

fix: avoid workspace state parse crashes#12094
zkochan merged 2 commits into
pnpm:mainfrom
qybaihe:fix-12020-workspace-state-race

Conversation

@qybaihe

@qybaihe qybaihe commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Fixes #12020

Summary

  • Treat malformed workspace state JSON as a cache miss instead of crashing loadWorkspaceState.
  • Write workspace state through write-file-atomic so concurrent readers do not observe partial direct writes.
  • Add a regression test for partial JSON and a changeset for pnpm / @pnpm/workspace.state.

Verification

  • node --input-type=module -e ... for a partial .pnpm-workspace-state-v1.json repro: before fix SyntaxError: Unexpected end of JSON input; after fix undefined.
  • npm --force exec --yes pnpm@11.4.0 -- --filter @pnpm/workspace.state run test test/loadWorkspaceState.test.ts
  • npm --force exec --yes pnpm@11.4.0 -- --filter @pnpm/workspace.state test
  • Pre-push hook completed compile-only, lint, spelling, meta-updater, Rust fmt, and Rust docs. Optional cargo-dylint and taplo checks were skipped because those tools are not installed locally.

Notes

  • The on-disk workspace state format is unchanged.
  • pacquet port: pacquet/crates/workspace-state now writes update_workspace_state atomically (temp file + rename, mirroring write-file-atomic). pacquet's install/add/update/remove write the shared .pnpm-workspace-state-v1.json, so a non-atomic write there can still leave a torn file that a concurrent pnpm run reads and crashes on. The read side needs no change: pacquet's only caller of load_workspace_state already maps a parse error to a cache miss, so the runtime behavior already matches pnpm's new undefined return. Verified with cargo nextest run -p pacquet-workspace-state (9/9), cargo clippy --deny warnings, cargo fmt --check, and taplo format --check.

Written by an agent (Codex, GPT-5).
pacquet port added by an agent (Claude Code, claude-opus-4-8).

Summary by CodeRabbit

  • Bug Fixes

    • Workspace state cache now treats partially written or corrupted cache files as missing instead of crashing, improving stability.
    • Workspace state persistence now uses atomic file writes to avoid readers observing partially written files.
  • Tests

    • Added a test verifying graceful handling of truncated or malformed workspace state cache files.

@qybaihe qybaihe requested a review from zkochan as a code owner June 1, 2026 04:48
@coderabbitai

coderabbitai Bot commented Jun 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6c0241ee-5e0c-405c-b974-867f18ce9ba0

📥 Commits

Reviewing files that changed from the base of the PR and between 4f88c92 and 5cff168.

📒 Files selected for processing (2)
  • pacquet/crates/workspace-state/Cargo.toml
  • pacquet/crates/workspace-state/src/lib.rs
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (2)
pacquet/**/Cargo.toml

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/Cargo.toml: Check whether the workspace already depends on something suitable in [workspace.dependencies] in the root Cargo.toml before adding a new dependency
Keep dependencies at the right level — add a new dependency to the specific crate that needs it, not to the workspace root or shared crate unless multiple crates depend on it

Files:

  • pacquet/crates/workspace-state/Cargo.toml
pacquet/**/*.rs

📄 CodeRabbit inference engine (pacquet/AGENTS.md)

pacquet/**/*.rs: Log emissions are part of matching pnpm — when porting a function that fires pnpm:<channel> events through globalLogger, logger.debug(...), or streamParser.write(...), mirror the call site, payload, and ordering so @pnpm/cli.default-reporter parses pacquet's NDJSON the same way
Declare a newtype wrapper for branded string types instead of collapsing the brand into a plain String or &str in Rust
If upstream TypeScript always validates before construction of a branded string, validate in the Rust wrapper too via TryFrom<String> and/or FromStr and do not provide an infallible public constructor
If upstream TypeScript never validates a branded string, just brand for type-safety in Rust by exposing an infallible From<String> constructor
If upstream TypeScript occasionally constructs a branded string without validation, expose from_str_unchecked in Rust as an escape hatch alongside the validating constructor
Match upstream serde behavior for branded strings crossing JSON, YAML, or INI boundaries by using #[serde(try_from = "String")] for deserialization and #[serde(into = "String")] for serialization
Derive simple conversions for branded strings using #[derive(derive_more::From)] and #[derive(derive_more::Into)] instead of handwriting impl blocks; use manual impl only when conversion needs custom logic
Model TypeScript string literal unions (like 'auto' | 'always' | 'never') as Rust enums instead of newtype wrappers, since the set of valid values is closed
Treat TypeScript string template literal types (like `${string}@${string}`) the same as branded string types in Rust, using a newtype wrapper with validation
Follow the code style guide in CODE_STYLE_GUIDE.md — imports, modules, naming, ownership and borrowing, parameter type selection, trait bounds, pattern matching, pipe-trait, error handling, test layout, and cloning of Arc and Rc
Choose owned vs. borrowed parameters to minimize copies; widen to t...

Files:

  • pacquet/crates/workspace-state/src/lib.rs
🧠 Learnings (3)
📚 Learning: 2026-05-20T19:40:55.051Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11774
File: pacquet/crates/resolving-deps-resolver/src/resolve_peers.rs:0-0
Timestamp: 2026-05-20T19:40:55.051Z
Learning: In the pacquet Rust code, ensure the semver implementation uses the `node-semver` crate (not `nodejs-semver`). `node-semver`’s public API does not include a `satisfies_with_prerelease`-style method; prerelease-tolerant matching should be implemented inline by first calling `Range::satisfies`, and when it rejects a prerelease version, retry matching against a stripped `MAJOR.MINOR.PATCH` base of the prerelease version.

Applied to files:

  • pacquet/crates/workspace-state/src/lib.rs
📚 Learning: 2026-05-22T00:08:44.646Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11837
File: pacquet/crates/resolving-npm-resolver/src/pick_package.rs:33-51
Timestamp: 2026-05-22T00:08:44.646Z
Learning: In the pnpm/pnpm repo’s pacquet Rust crates, do not flag Unicode ellipsis characters (U+2026, `…`) in Rust doc comments (`///` / `/** */`) as a lint violation. The pacquet crate’s `dylint.toml` only enables `perfectionist::derive_ordering`, and the Dylint `unicode-ellipsis` rule is not enabled for this project—so `…` in doc comments is an intentional, repo-consistent style.

Applied to files:

  • pacquet/crates/workspace-state/src/lib.rs
📚 Learning: 2026-05-20T23:07:58.444Z
Learnt from: zkochan
Repo: pnpm/pnpm PR: 11784
File: pacquet/crates/resolving-deps-resolver/src/hoist_peers.rs:120-133
Timestamp: 2026-05-20T23:07:58.444Z
Learning: When reviewing code in this pacquet Rust port, follow the upstream pnpm compatibility rule: only match pnpm’s behavior exactly. Do not propose review changes that intentionally deviate from pnpm’s documented/observed behavior, even if pnpm appears buggy. If you identify a real bug in pnpm behavior, the review should prioritize fixing it upstream in pnpm first, and avoid implementing a pnpm-behavior workaround here unless the same fix has already landed upstream.

Applied to files:

  • pacquet/crates/workspace-state/src/lib.rs
🔇 Additional comments (3)
pacquet/crates/workspace-state/Cargo.toml (1)

20-20: LGTM!

pacquet/crates/workspace-state/src/lib.rs (2)

20-25: LGTM!


182-192: LGTM!

Also applies to: 206-216


📝 Walkthrough

Walkthrough

Uses atomic writes for workspace-state (JS and Rust), treats malformed/truncated JSON as a cache miss instead of throwing, adds a test for truncated cache files, and includes a changeset entry for patch releases.

Changes

Workspace State Cache Stability

Layer / File(s) Summary
JS: atomic writes and dependency
workspace/state/package.json, workspace/state/src/updateWorkspaceState.ts
Adds write-file-atomic (and its types) and replaces fs.promises.writeFile with atomic writes to prevent readers observing partially written workspace state files.
Rust: atomic tempfile write
pacquet/crates/workspace-state/Cargo.toml, pacquet/crates/workspace-state/src/lib.rs
Moves tempfile to regular dependencies and updates update_workspace_state to write JSON to a same-directory temp file and persist/rename it atomically.
Graceful JSON parse error handling
workspace/state/src/loadWorkspaceState.ts, workspace/state/test/loadWorkspaceState.test.ts
Wraps JSON.parse to return undefined on SyntaxError (treat as missing cache) and adds a Jest test that verifies truncated/malformed cache files are handled without throwing.
Release metadata
.changeset/fix-workspace-state-race.md
Adds a changeset declaring patch releases for @pnpm/workspace.state and pnpm with a release note describing the crash-avoidance change.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • pnpm/pnpm#11665: Related workspace-state writer changes; both modify how .pnpm-workspace-state-v1.json is produced/updated.

Suggested reviewers

  • zkochan
  • anonrig

Poem

🐰 I hopped where JSON used to fray,
Partial writes would spoil the day.
Now temp files dance and then replace,
Broken parses vanish without a trace.
A quiet burrow, stable space.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: avoid workspace state parse crashes' directly and clearly summarizes the main change: adding parse-error handling to prevent loadWorkspaceState from crashing on malformed JSON.
Linked Issues check ✅ Passed The PR fully addresses all coding requirements from #12020: graceful handling of malformed workspace state JSON, atomic writes via write-file-atomic in pnpm and tempfile in pacquet, and a regression test for partial JSON.
Out of Scope Changes check ✅ Passed All changes directly support the stated objectives: parse-error handling in loadWorkspaceState, atomic writes in updateWorkspaceState (pnpm) and update_workspace_state (pacquet), dependency updates, a test, and a changeset entry.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Review Summary by Qodo

Fix workspace state crashes and race conditions

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Handle malformed workspace state JSON gracefully by treating parse errors as cache misses
• Replace direct file writes with atomic writes to prevent partial state observations
• Add regression test for partial JSON parsing scenarios
• Update dependencies to include write-file-atomic package
Diagram
flowchart LR
  A["Malformed JSON"] -->|Previously crashed| B["SyntaxError"]
  A -->|Now handled| C["Return undefined"]
  D["Concurrent writes"] -->|Previously risky| E["Direct fs.writeFile"]
  D -->|Now safe| F["writeFileAtomic"]
  C --> G["Cache miss treated safely"]
  F --> G

Loading

Grey Divider

File Changes

1. workspace/state/src/loadWorkspaceState.ts 🐞 Bug fix +8/-1

Add error handling for malformed JSON parsing

• Wrapped JSON.parse in try-catch block to handle SyntaxError
• Returns undefined for malformed JSON instead of crashing
• Preserves existing error handling for file read failures

workspace/state/src/loadWorkspaceState.ts


2. workspace/state/src/updateWorkspaceState.ts 🐞 Bug fix +2/-1

Use atomic writes for workspace state updates

• Added import for write-file-atomic package
• Replaced fs.promises.writeFile with writeFileAtomic for atomic writes
• Prevents concurrent readers from observing partial writes

workspace/state/src/updateWorkspaceState.ts


3. workspace/state/test/loadWorkspaceState.test.ts 🧪 Tests +12/-0

Add regression test for partial JSON

• Added regression test for partial JSON content
• Test verifies loadWorkspaceState returns undefined for malformed JSON
• Ensures logger debug calls are still made during error handling

workspace/state/test/loadWorkspaceState.test.ts


View more (3)
4. .changeset/fix-workspace-state-race.md 📝 Documentation +6/-0

Add changeset for workspace state fix

• Created changeset documenting the fix
• Marks patch version bump for @pnpm/workspace.state and pnpm
• Describes fix for partial or malformed workspace state cache

.changeset/fix-workspace-state-race.md


5. workspace/state/package.json Dependencies +4/-2

Add write-file-atomic dependency

• Added write-file-atomic as production dependency
• Added @types/write-file-atomic as dev dependency
• Updated lock file with new dependency versions

workspace/state/package.json


6. pnpm-lock.yaml Dependencies +6/-0

Update lock file with new dependencies

• Updated lock file with write-file-atomic@7.0.1 dependency
• Added @types/write-file-atomic@4.0.3 type definitions
• Reflects new dependencies in workspace/state package

pnpm-lock.yaml


Grey Divider

Qodo Logo

@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Integrated-Benchmark Report (Linux)

Scenario: Isolated linker: fresh restore, cold cache + cold store

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 2.116 ± 0.097 1.998 2.311 1.02 ± 0.05
pacquet@main 2.076 ± 0.036 1.999 2.122 1.00
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 2.1160877187600002,
      "stddev": 0.09671268395599487,
      "median": 2.0867196540600004,
      "user": 2.6549692,
      "system": 3.40489148,
      "min": 1.9983646210600001,
      "max": 2.31106516706,
      "times": [
        2.1473933150600004,
        2.08177989806,
        2.04415923206,
        2.0615349970600003,
        2.0453537980600003,
        2.13717038506,
        2.24239636406,
        2.31106516706,
        1.9983646210600001,
        2.09165941006
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 2.0762816237600004,
      "stddev": 0.03598708320630822,
      "median": 2.0732923000600003,
      "user": 2.6725193,
      "system": 3.39263288,
      "min": 1.9991157590600002,
      "max": 2.1221836570600003,
      "times": [
        2.0650172860600002,
        2.08944251706,
        2.07057920106,
        2.0493684730600004,
        2.07600539906,
        2.1221836570600003,
        2.0692112050600002,
        1.9991157590600002,
        2.1044742000600003,
        2.11741854006
      ]
    }
  ]
}

Scenario: Isolated linker: fresh restore, hot cache + hot store

Command Mean [ms] Min [ms] Max [ms] Relative
pacquet@HEAD 697.4 ± 35.4 653.1 786.0 1.04 ± 0.09
pacquet@main 670.7 ± 47.8 640.8 802.6 1.00
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 0.6974392395400001,
      "stddev": 0.03538335804070127,
      "median": 0.68936932094,
      "user": 0.3551984799999999,
      "system": 1.3305154399999999,
      "min": 0.65313642344,
      "max": 0.78597189544,
      "times": [
        0.78597189544,
        0.6925746984400001,
        0.65313642344,
        0.6852802624400001,
        0.69168329344,
        0.7073998074400001,
        0.67507498844,
        0.71475626644,
        0.68145941144,
        0.68705534844
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 0.6707445538400001,
      "stddev": 0.04777933500922883,
      "median": 0.6542131089400001,
      "user": 0.35172548,
      "system": 1.32101704,
      "min": 0.64083630644,
      "max": 0.8025915734400001,
      "times": [
        0.8025915734400001,
        0.64795601944,
        0.68207219844,
        0.6461480794400001,
        0.64083630644,
        0.65338779944,
        0.66786790244,
        0.65369831844,
        0.65815944144,
        0.6547278994400001
      ]
    }
  ]
}

Scenario: Isolated linker: fresh install, cold cache + cold store

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 2.403 ± 0.021 2.370 2.441 1.00 ± 0.01
pacquet@main 2.397 ± 0.027 2.349 2.442 1.00
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 2.40328800894,
      "stddev": 0.02051929609041609,
      "median": 2.39975368384,
      "user": 3.800398899999999,
      "system": 3.1654736399999996,
      "min": 2.3697636023400004,
      "max": 2.44132939734,
      "times": [
        2.44132939734,
        2.39711363234,
        2.4307466313400004,
        2.40951310234,
        2.3697636023400004,
        2.40239373534,
        2.39534266334,
        2.4052023193400003,
        2.38917759134,
        2.39229741434
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 2.39740984864,
      "stddev": 0.027492865381042526,
      "median": 2.3995669893400002,
      "user": 3.819565,
      "system": 3.1665209399999994,
      "min": 2.34877730634,
      "max": 2.4421948113400003,
      "times": [
        2.39935366534,
        2.40278159534,
        2.39978031334,
        2.34877730634,
        2.3938975253400003,
        2.40706233734,
        2.38536433934,
        2.4421948113400003,
        2.43017741534,
        2.36470917734
      ]
    }
  ]
}

Scenario: Isolated linker: fresh install, hot cache + hot store

Command Mean [s] Min [s] Max [s] Relative
pacquet@HEAD 1.556 ± 0.041 1.504 1.643 1.03 ± 0.04
pacquet@main 1.517 ± 0.039 1.461 1.599 1.00
BENCHMARK_REPORT.json
{
  "results": [
    {
      "command": "pacquet@HEAD",
      "mean": 1.55565462414,
      "stddev": 0.04144432933951576,
      "median": 1.5515969586400002,
      "user": 1.7332989399999998,
      "system": 1.8572315199999998,
      "min": 1.5040836006400002,
      "max": 1.6431386816400002,
      "times": [
        1.52480231264,
        1.57690542164,
        1.5483746766400002,
        1.5219726126400002,
        1.5973855036400002,
        1.5578317426400001,
        1.5040836006400002,
        1.5272324486400002,
        1.6431386816400002,
        1.55481924064
      ]
    },
    {
      "command": "pacquet@main",
      "mean": 1.5167276123400002,
      "stddev": 0.03888937574873275,
      "median": 1.5146137291400001,
      "user": 1.72637024,
      "system": 1.8285125199999999,
      "min": 1.46108172664,
      "max": 1.5986843756400002,
      "times": [
        1.4865947296400002,
        1.5986843756400002,
        1.5186412946400003,
        1.49882550464,
        1.5151309066400003,
        1.4850438786400002,
        1.51409655164,
        1.46108172664,
        1.54485343964,
        1.5443237156400003
      ]
    }
  ]
}

@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

🐰 Bencher Report

Branchpr/12094
Testbedpacquet
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
milliseconds (ms)
(Result Δ%)
Upper Boundary
milliseconds (ms)
(Limit %)
isolated-linker.fresh-install.cold-cache.cold-store📈 view plot
🚷 view threshold
2,403.29 ms
(+0.38%)Baseline: 2,394.13 ms
2,872.95 ms
(83.65%)
isolated-linker.fresh-install.hot-cache.hot-store📈 view plot
🚷 view threshold
1,555.65 ms
(+2.34%)Baseline: 1,520.05 ms
1,824.05 ms
(85.29%)
isolated-linker.fresh-restore.cold-cache.cold-store📈 view plot
🚷 view threshold
2,116.09 ms
(-0.21%)Baseline: 2,120.44 ms
2,544.53 ms
(83.16%)
isolated-linker.fresh-restore.hot-cache.hot-store📈 view plot
🚷 view threshold
697.44 ms
(+2.97%)Baseline: 677.29 ms
812.75 ms
(85.81%)
🐰 View full continuous benchmarking report in Bencher

@zkochan zkochan force-pushed the fix-12020-workspace-state-race branch from 8d3cea8 to 4f88c92 Compare June 1, 2026 13:13
Port the atomic-write half of the pnpm fix for pnpm#12020 to pacquet.
pacquet's install/add/update/remove all call update_workspace_state,
and the on-disk file is shared with the pnpm CLI, so a non-atomic
fs::write could leave a torn file that a concurrent `pnpm run` reads
and crashes on. Write to a temp file in the same directory and rename
it into place, mirroring pnpm's switch to write-file-atomic.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 36.36364% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.02%. Comparing base (0a4d665) to head (5cff168).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pacquet/crates/workspace-state/src/lib.rs 36.36% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12094      +/-   ##
==========================================
- Coverage   87.04%   87.02%   -0.03%     
==========================================
  Files         252      252              
  Lines       28146    28155       +9     
==========================================
+ Hits        24499    24501       +2     
- Misses       3647     3654       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zkochan zkochan merged commit 37669c2 into pnpm:main Jun 1, 2026
23 checks passed
@welcome

welcome Bot commented Jun 1, 2026

Copy link
Copy Markdown

Congrats on merging your first pull request! 🎉🎉🎉

@zkochan zkochan mentioned this pull request Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

loadWorkspaceState crashes with "Unexpected end of JSON input" under concurrent pnpm run (non-atomic write, no parse-error handling)

3 participants