Skip to content

Fix repair leaving npm vendor dirs uncommittable (#1581) - #1656

Open
Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
mainfrom
agent/fix-repair-npm-uuid-metadata
Open

Mikola Lysenko (mikolalysenko) wants to merge 4 commits into
mainfrom
agent/fix-repair-npm-uuid-metadata

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

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

Fixes #1581

Summary

socket-patch repair now restores the .socket/vendor/npm/<uuid>/.gitignore
and .gitattributes for every npm-family vendored entry (npm, pnpm, yarn,
bun text, bun.lockb, vlt), both when the artifact is healthy and after a
redownload. It then checks that git would actually commit the artifact. A
rule that the uuid .gitignore can't override (such as .socket/) now
fails with vendor_artifact_gitignored, the same refusal vendor gives,
on every run until the rule is fixed.

Root cause

repair only rewrote the uuid-dir metadata for vlt and JVM entries
(commands/vendored_backend/repair.rs). The npm tarball flavors only got
it from a vendored scan (npm_common.rs keep_pack_committable). So:

  1. A project vendored before Vendored Bun tarballs go into Git LFS when .gitattributes tracks *.tgz with LFS, so a checkout without LFS fails bun install --frozen-lockfile #1541 kept the legacy * -text
    .gitattributes. A repository *.tgz filter=lfs rule still applied,
    and the repaired tarball went back into LFS on the next commit.
  2. When the whole <uuid>/ dir was lost, repair restored only the .tgz.
    With a stock *.tgz ignore rule, git ignored the redownloaded tarball,
    so "commit it" committed nothing (the second symptom reported on repair leaves a pre-#1541 root <uuid>/.gitattributes as * -text, so the repaired Bun tarball goes back into Git LFS on the next commit #1581).

Changes

Test evidence

Red → green. The new CLI tests were run against origin/main with only
the test file changed, in a separate worktree:

cargo test -p socket-patch-cli --all-features --test in_process_vendor repair_
main:   repair_of_a_lost_uuid_dir_under_an_ignored_socket_dir_fails ... FAILED
        repair_upgrades_a_legacy_uuid_gitattributes ... FAILED
        repair_of_a_lost_uuid_dir_restores_its_gitignore_and_gitattributes ... FAILED
branch: 4 passed; 0 failed

Per-issue checklist for #1581:

  • Legacy * -text survives repair (healthy artifact, and an LFS
    pointer redownload) → repair_upgrades_a_legacy_uuid_gitattributes
    (also asserts git check-attr filter = unset under a repo
    *.tgz filter=lfs rule).
  • Lost uuid dir comes back without .gitignore/.gitattributes, so
    the tarball is ignored by *.tgz →
    repair_of_a_lost_uuid_dir_restores_its_gitignore_and_gitattributes
    (asserts git check-ignore is empty).
  • Unoverridable .socket/ rule reports success →
    repair_of_a_lost_uuid_dir_under_an_ignored_socket_dir_fails, which
    fails with vendor_artifact_gitignored on both the first and the
    second run.
  • Core units in redownload.rs:
    npm_redownload_restores_the_uuid_metadata,
    npm_uuid_metadata_upgrades_a_legacy_gitattributes,
    npm_commit_check_refuses_only_an_unoverridable_rule.

Local runs:

  • cargo fmt --all -- --check and
    cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo test --workspace --all-features --no-fail-fast: everything
    passes except tests that can't pass in this sandbox. Those fail the same
    way independent of this diff:
    • chmod-based write-failure tests, because the sandbox runs as uid 0
      (repair_invariants::repair_cleanup_failure_is_reported_in_json_and_silent_modes,
      repair_render::repair_exits_zero_and_stays_quiet_when_lock_file_unremovable,
      in_process_redirect write-failure tests, core
      copy_tree/vlt_heal/pypi_* write-failure tests).
    • e2e_bun_lockb::binary_shared_bundled_record_hosted_pin_is_managed,
      which reaches the real patches-api.socket.dev through a blocked proxy.
  • No wrapper changes are needed: npm/, pypi/ and gem/ only
    dispatch to the binary.

/code-review high

The review found 8 candidates. Fixed here:

  • symlinked vendor dir
  • check only on the rewrite run
  • workspace-mirror continue skipping the metadata step
  • error wording
  • vlt dir probe

Not fixed:

Follow-ups

  • socket-patch.vendor.json (the informational marker) isn't restored
    when the uuid dir is lost. It is never a trust input.

https://claude.ai/code/session_01WMCn4ztc3DCiDDSBZDtjHd


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
`socket-patch repair` redownloaded a broken npm, pnpm, yarn or bun
vendored tarball but never rewrote the `<uuid>/.gitignore` and
`.gitattributes` beside it; only vlt and JVM entries got them back.
A project vendored before #1541 kept the old `* -text` attributes, so
a repository `*.tgz filter=lfs` rule put the repaired tarball back
into Git LFS, and a lost uuid dir came back without its `!*`
.gitignore, so a stock `*.tgz` ignore rule left the tarball out of
the commit. Both reported a successful repair while fresh clones
kept failing.

Repair now rewrites both files for every npm-family entry, on the
healthy path and after a redownload, then asks git whether it would
commit the artifact. A rule the uuid .gitignore cannot override
(such as `.socket/`) fails with `vendor_artifact_gitignored`, the
same refusal `vendor` gives.

Fixes #1581

Assisted-by: Claude Code:claude-opus-5-5
A rewritten npm uuid .gitignore or .gitattributes is not an
artifact rebuild, as the vlt repair contract already says, so
repair reports no rebuilt event for it.

Assisted-by: Claude Code:claude-opus-5-5
Repair now asks git whether it would commit a healthy npm-family
artifact on every run, not only when it rewrote the uuid metadata,
so a .socket/ ignore rule keeps failing until it is fixed. It also
restores the metadata before repairing bun workspace mirrors, skips
a vendor dir reached through a symlink, and probes a vlt package
dir as a directory.

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

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@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 1d7c6ad. Configure here.

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.

repair leaves a pre-#1541 root <uuid>/.gitattributes as * -text, so the repaired Bun tarball goes back into Git LFS on the next commit

2 participants