fix: avoid workspace state parse crashes#12094
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (2)pacquet/**/Cargo.toml📄 CodeRabbit inference engine (pacquet/AGENTS.md)
Files:
pacquet/**/*.rs📄 CodeRabbit inference engine (pacquet/AGENTS.md)
Files:
🧠 Learnings (3)📚 Learning: 2026-05-20T19:40:55.051ZApplied to files:
📚 Learning: 2026-05-22T00:08:44.646ZApplied to files:
📚 Learning: 2026-05-20T23:07:58.444ZApplied to files:
🔇 Additional comments (3)
📝 WalkthroughWalkthroughUses 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. ChangesWorkspace State Cache Stability
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Review Summary by QodoFix workspace state crashes and race conditions
WalkthroughsDescription• 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 Diagramflowchart 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
File Changes1. workspace/state/src/loadWorkspaceState.ts
|
Integrated-Benchmark Report (Linux)Scenario: Isolated linker: fresh restore, cold cache + cold store
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
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
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
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
]
}
]
} |
|
| Branch | pr/12094 |
| Testbed | pacquet |
Click to view all benchmark results
| Benchmark | Latency | Benchmark 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%) |
8d3cea8 to
4f88c92
Compare
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 Report❌ Patch coverage is
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. 🚀 New features to boost your workflow:
|
|
Congrats on merging your first pull request! 🎉🎉🎉 |
Fixes #12020
Summary
loadWorkspaceState.write-file-atomicso concurrent readers do not observe partial direct writes.pnpm/@pnpm/workspace.state.Verification
node --input-type=module -e ...for a partial.pnpm-workspace-state-v1.jsonrepro: before fixSyntaxError: Unexpected end of JSON input; after fixundefined.npm --force exec --yes pnpm@11.4.0 -- --filter @pnpm/workspace.state run test test/loadWorkspaceState.test.tsnpm --force exec --yes pnpm@11.4.0 -- --filter @pnpm/workspace.state testcompile-only, lint, spelling, meta-updater, Rust fmt, and Rust docs. Optionalcargo-dylintandtaplochecks were skipped because those tools are not installed locally.Notes
pacquet/crates/workspace-statenow writesupdate_workspace_stateatomically (temp file + rename, mirroringwrite-file-atomic). pacquet'sinstall/add/update/removewrite the shared.pnpm-workspace-state-v1.json, so a non-atomic write there can still leave a torn file that a concurrentpnpm runreads and crashes on. The read side needs no change: pacquet's only caller ofload_workspace_statealready maps a parse error to a cache miss, so the runtime behavior already matches pnpm's newundefinedreturn. Verified withcargo nextest run -p pacquet-workspace-state(9/9),cargo clippy --deny warnings,cargo fmt --check, andtaplo 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
Tests