Repository navigation
Fix vendor --check artifact failure message (#1467) - #1468
Conversation
Assisted-by: Claude Code:claude-opus-5-5
When a committed vendored artifact was missing or changed, vendor
--check printed the Rust Debug form of an internal state, such as
`Corrupt { reason: "vendor_workspace_artifact_missing" }`, with no
path and no next step. CI logs gave users nothing to act on.
The reason now says what is wrong, names the committed file (for a
Bun bun.lockb workspace, the member copy that is off, and that the
root artifact is intact), and gives the fix that matches what repair
does: `socket-patch repair` then commit, `socket-patch vendor` for a
pending re-vendor, fix state.json for an entry that can't be
verified, or upgrade for an unknown npm flavor. The stable tag stays
in parentheses so logs remain searchable.
Fixes #1467
Assisted-by: Claude Code:claude-opus-5-5
Follow-up from review of the new vendor --check wording: - Only call the root artifact intact for a Bun member-copy failure when the ledger records its sha256; without one it was never checked. - An entry that can't be verified no longer always blames state.json: an unreadable bun.lockb says so, and other cases point at the ledger, the manifest or a re-vendor, and say repair won't rebuild. - A pending re-vendor says the ledger and manifest disagree. - A JVM entry also counts as missing when only its tree marker is gone, so the message names both. - vendor --check and repair now share one check for a Bun member copy failure. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
vendor --check told users to run `socket-patch repair` for any missing or damaged artifact. When the ledger records no archive SHA-256 (file artifacts) or no file inventory (directory artifacts), repair has nothing to verify a download against and refuses, so that advice left the check red. The message now gives repair's own refusal and its remedy instead. When a Bun workspace's copies can't be listed, the message used to guess at an unreadable bun.lockb. It now carries the real cause (unreadable lock, unsafe path, symlinked member copy, ...). restore() and vendor --check share one check for when repair refuses. Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
There was a problem hiding this comment.
✅ 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 c8cec59. Configure here.
|
Labeled Ready for review by the burn-down agent.
Generated by Claude Code |
Final review briefWhat it does: Risk: low. Only the text of failure messages changes; pass/fail decisions and exit codes don't. The one refactor on a live path ( Look here:
Verified:
Changes I made: none. Auto-merge is armed, so approving sends this straight to the merge queue. Generated by Claude Code |
LLM Description written by Claude Code:claude-opus-5-5
Fixes #1467
Summary
When a committed vendored artifact is missing or changed,
vendor --checknow says what is wrong, names the file, and gives the step that fixes it.
Before, it printed the Rust Debug form of an internal state, with no path
and no remedy:
Now (Bun
bun.lockbworkspace, member copy deleted):The same text goes into the
--jsonevent'sreason. The stable tagstays in parentheses so logs remain searchable.
Root cause
run_checkincrates/socket-patch-cli/src/commands/vendor.rsturned everynon-healthy
ArtifactHealthintoformat!("artifact verification failed: {health:?}").That string is shared by every vendored ecosystem, so the root-artifact
rows in the issue (
Missing,Corrupt { reason: "vendor_sha256_mismatch" })were affected as well as the Bun member-copy rows.
Fix
vendor::describe_artifact_failurein core. It turns each healthstate into a cause, the committed path, and the remedy that matches what
repairdoes for that state:socket-patch repair, then commit. When repairwould refuse because the ledger has no archive SHA-256 (file artifact)
or no file inventory (directory artifact), the message gives repair's
own refusal and remedy instead (shared
redownload::ledger_blocks_restore).For a JVM entry, "missing" also covers a missing tree marker.
bun_workspace::failing_mirror).It calls the root artifact intact only when the ledger records its
sha256.
vendor_uuid_mismatch):socket-patch vendor.copies can't be listed, the message carries the real cause; other cases
point at the ledger, the manifest, or a re-vendor.
ArtifactHealth::is_workspace_copy_issue()replaces repair's privatecopy of the same check, so
vendor --checkandrepairagree on whatcounts as a member-copy failure.
npm/,pypi/, andgem/onlydispatch to the binary and never parse this text.
Tests (red → green)
bun.lockbworkspace)vendor::verify::tests::binary_workspace_health_attests_every_installable_tarball(asserts the exact text, including the member path)Missing/vendor_sha256_mismatch)in_process_vendor::vendor_check_names_damaged_artifact_and_repair_remedyvendor::verify::tests::describe_artifact_failure_names_path_and_remedyWithout the fix (the CLI change reverted), the CLI test fails:
With the fix, it passes. Commands run locally:
cargo fmt --all -- --check: cleancargo clippy --workspace --all-features -- -D warnings: cleancargo test -p socket-patch-core --all-features --lib -- vendor::: everything passes except two chmod-based write-failure tests (pypi_poetry::wire_write_failure_*,pypi_requirements::wire_failure_rolls_back_*). Those tests can't make a write fail when run as root, which is how this sandbox runs. They are unrelated to this change, and CI runs as a non-root user.cargo test --workspace --all-features: this sandbox's disk allowance cannot hold every test binary (it fails with ENOSPC while linking), so I ran the suites this change touches instead.in_process_vendor: 133/133 pass.repair: 126 pass; the 2 failures are chmod-based tests (*_unremovable,repair_cleanup_failure_*).covgap_commands_vendor: 54 pass; the 3 failures are chmod-based*_state_write_failure_*tests. A root user ignores those permissions, which is why they fail here. CI runs the full workspace.Bugbot
Two findings on c8c77bf. Both were real, both are fixed in c8cec59, and
both threads are answered and resolved: repair advice where repair would
refuse, and the wrong cause given for
vendor_workspace_artifact_invalid.Code review (
/code-review high)Fixed: the "intact" claim without a recorded sha256, the one-size remedy
for unverifiable entries, the StaleUuid wording, the JVM marker case, the
duplicated workspace-copy check, and the test doc.
Not fixed:
failing_mirrorre-reads the mirrors to find the path, which costs I/Oand allows a re-check race. It runs only after a check has already
failed, and
vendor --checkis a read-only audit. Carrying the path inArtifactHealth::Corruptwould change a public enum that about tenmatch sites use.
output is out of scope here.
Generated by Claude Code