Repository navigation
Agent-mode apply reports already_patched / applied: 0 when it actually patched an unpatched pnpm peer-variant copy (the store-copy pass's writes are never reported) #756
Description
[agent] Found by the scheduled pnpm bug-hunt routine (ledger #303).
Summary
apply_package_patch re-runs the patch over every pnpm peer-variant copy of the package (find_store_peer_variant_copies, for example .pnpm/react-dom@18.2.0_react@18.2.0 and …_react@18.3.1). The copies' results are folded in for success/error only, and the event is classified from the primary copy's per-file records. When the primary is already patched and a twin is not, apply writes the twin, but the run reports that package as skipped / already_patched ("All files already match afterHash"). The JSON summary says applied: 0, and the human summary says 0 of 1 targeted patch applied, 1 already patched.
So the output says the run changed nothing when it actually fixed a vulnerable copy on disk. The same order-dependence decides whether the human Patched packages: block shows via blob at all. If the unpatched copy happens to be the first one visited, the run correctly reports applied: 1.
Impact
This is a reporting bug only: the bytes end up correct. But it breaks automation that keys on --json (summary.applied, events[].action), for example CI that treats applied > 0 after a postinstall as drift (node_modules was reset or a new peer variant appeared) and alerts or invalidates caches. In that case the drift is silently swallowed. It also contradicts CLI_CONTRACT's meaning of already_patched (every file already matched afterHash before the run).
Repro (Linux, pnpm 12.8.1, main 045d7ec)
mkdir -p ws/a ws/b && cd ws
echo '{"name":"root","version":"1.0.0","private":true}' > package.json
printf 'packages:\n - a\n - b\n' > pnpm-workspace.yaml
echo '{"name":"a","version":"1.0.0","dependencies":{"react":"18.2.0","react-dom":"18.2.0"}}' > a/package.json
echo '{"name":"b","version":"1.0.0","dependencies":{"react":"18.3.1","react-dom":"18.2.0"}}' > b/package.json
pnpm install # -> .pnpm/react-dom@18.2.0_react@18.2.0 and .pnpm/react-dom@18.2.0_react@18.3.1
socket-patch scan --mode agent --yes # a react-dom@18.2.0 patch (mock API); both copies patched
# simulate a re-created twin: put upstream bytes back into ONLY the react@18.3.1 copy
X=node_modules/.pnpm/react-dom@18.2.0_react@18.3.1/node_modules/react-dom/index.js
# (replace $X with its upstream content as a new file)
socket-patch apply --offline --json
# summary: {"applied": 0, "skipped": 4, ...}; every event: skipped / already_patched
head -c 19 "$X" # now patched: apply DID write itResults for the three starting states (human output):
| unpatched before the run | apply's report | on disk after |
|---|---|---|
_react@18.2.0 copy (visited first) |
1 of 1 applied, that copy via blob |
both patched |
_react@18.3.1 copy only |
0 of 1 applied, 1 already patched, all four lines already patched |
both patched |
| both | 1 of 1 applied; the 18.3.1 copy is listed already patched |
both patched |
Each row reproduced at least twice. Four lines appear for two copies because each workspace-member link is also visited (#633). That's a separate bug.
Expected vs actual
- Expected: a run that patched files on disk reports the package as
applied(with the files it wrote, or at leastsummary.applied: 1).already_patchedis reserved for "nothing needed doing". - Actual: the classification follows only the primary copy's file records, so the twin's patch is invisible.
Versions
| pnpm 12.8.1 | |
|---|---|
main 045d7ec |
fail |
| release 4.0.0 (npm) | fail (same output: 0/1 targeted patches applied, 4 already patched, twin patched) |
Not a regression. Other pnpm versions and OSes aren't tested; the code path is version-independent. vlt ~peer copies go through the same fold, by code reading.
Suspect code
crates/socket-patch-core/src/patch/apply.rs:742-753(apply_package_patch): copies are applied, thenfold_copy_result(:765) keeps only success/error. The doc comment says "The primary's per-file records are what the returnedApplyResultcarries".crates/socket-patch-cli/src/commands/apply.rs:727(result_to_event) and:1493(tally_results) then classify fromfiles_verified/files_patchedof the primary only.
Probe runs: none (Linux reproduction only).
Activity
mikolalysenko commentedon Oct 4, 2026
[agent]
rollbackhas the same blind spot (crates/socket-patch-core/src/patch/rollback.rs:356runs the same store-copy pass). Same workspace on pnpm 12.8.1, main045d7ec, with both copies patched. I put upstream bytes back into only the primary_react@18.2.0copy, then ransocket-patch rollback --offline --json --yes:On disk, the
_react@18.3.1copy went from patched to upstream during that run. So rollback reverted a patched copy and reported that nothing was rolled back.Generated by Claude Code
pnpm: run 16 ledger (filed #754, #756; #636 comment)
mikolalysenko commentedon Oct 4, 2026
[agent] Structural root cause: apply and rollback each keep a private copy of the store-copy fan-out and
fold_copy_result(which have already drifted on the advisory rule), and both drop the copy's per-file records. #772 proposes one shared fan-out and fold that merges those records for both directions, under tracking #771. A fix here can land first; #772 then becomes the mechanical merge and keeps this issue's regression tests.Generated by Claude Code
mikolalysenko commentedon Oct 4, 2026
[agent] Shares root cause with #772: the duplicated store-copy
fold_copy_resultinpatch/apply.rsandpatch/rollback.rsdrops each copy's per-file records. Will be fixed together, covering both the apply and the rollback misreport noted above.Generated by Claude Code
mikolalysenko commentedon Oct 4, 2026
[agent] Claiming this issue (with #772; shared root cause: apply and rollback each fold pnpm/vlt store copies with a private
fold_copy_resultthat drops the copy's per-file records). Branch: agent/fix-store-copy-fold-records. Claim-ID: 2026-10-04T10:20:57Z-20f5dfGenerated by Claude Code
mikolalysenko commentedon Oct 4, 2026
[agent] Draft PR: #774
Generated by Claude Code
Report writes to pnpm/vlt store twin copies
Test CLI reporting of store twin writes