Repository navigation
Fix repair/prune deleting active patches' restore blobs (#893) - #1316
Merged
Mikola Lysenko (mikolalysenko) merged 5 commits intoOct 9, 2026
Merged
Conversation
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
`repair` (alias `gc`) and `scan --prune` kept only the afterHash blobs of patches still in the manifest, so the first repair deleted the originals `get` stored. A later `rollback --offline` of a still active patch then failed and told the user to run `repair`, which only downloads afterHash blobs and can never bring the original back. Give ArtifactReferences one policy for a manifest's patches, `active`: afterHash and beforeHash blobs plus the diff archive of every patch. repair and scan --prune use it, and remove/rollback's `after_removal` builds on it. Delete the dead cleanup_unused_blobs, cleanup_unused_archives and format_cleanup_result. The rollback missing-blob remedy now says to re-run without --offline (or once the patch API is reachable) instead of naming repair. Fixes #893 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko)
marked this pull request as ready for review
October 9, 2026 17:08
Collaborator
Author
|
BugBot review |
Mikola Lysenko (mikolalysenko)
enabled auto-merge
October 9, 2026 17:08
The human repair output tests assumed repair sweeps the active patch's beforeHash blob. It now keeps it (#893), so the no-orphan case checks both blobs in use and the orphan case removes only the orphan. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Tanmay Singla (Tanmay182003)
approved these changes
Oct 9, 2026
Collaborator
Author
|
bugbot run 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 37bd179. Configure here.
Mikola Lysenko (mikolalysenko)
disabled auto-merge
October 9, 2026 19:55
Resolve conflicts with #1049 (diff download path removed): - ArtifactReferences::active keeps the afterHash and beforeHash blobs of every active patch (#1316); the patch_uuids/diff-archive retention is dropped because #1049 sweeps every diff and package archive as obsolete. - after_removal builds on active() plus the originals of removed-but-not-installed patches. - cleanup_unused_blobs / format_cleanup_result / cleanup_archives stay removed (no callers left); the archive-retention unit tests go with them. - CLI_CONTRACT.md: repair row and scan --prune paragraph describe the combined policy. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko)
enabled auto-merge
October 9, 2026 22:06
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Oct 9, 2026
Mikola Lysenko (mikolalysenko)
deleted the
agent/v5-gc-keep-before-blobs
branch
October 9, 2026 23:08
Mikola Lysenko (mikolalysenko)
added a commit
that referenced
this pull request
Oct 10, 2026
Resolve against main's removal of the diff download path (#1049) and the restore-blob GC fix (#1316): repair drops the created-file blob pass and keeps the GcReport carrier; remove keeps the archive noun loop; the contract keeps "update" and drops the removed paidRequired status; the envelope contract test uses AppliedVia::Blob. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
LLM Description written by Claude Code:claude-opus-5-5
Fixes #893
Summary
repair/gcandscan --pruneno longer delete the beforeHash blobs of patches that are still in the manifest. An offlinerollbackafter a repair works again, and its missing-blob error no longer sends users torepair, which can't download originals.Root cause
ArtifactReferenceshad two retention policies for an unchanged manifest:after_removal(used byremoveandrollback) kept the beforeHash blobs of every remaining patch.for_apply(used byrepairandscan --prune) kept only the afterHash blobs.So the first
repairdeleted the originals thatgetstored.Changes
ArtifactReferences::active(manifest)replacesfor_applyand is the one policy. It keeps the afterHash and beforeHash blobs and the diff archive of every manifest patch.repairandscan --prunecall it, andafter_removalbuilds on it.cleanup_unused_blobs,cleanup_unused_archivesandformat_cleanup_result. Their unit tests now run the samecleanup_dirmechanics through theactivepolicy.Re-run without --offline to download the original blobs.; failed download →Re-run once the patch API is reachable….repairrow and thescan --pruneparagraph describe the shared retention policy.Per-issue checklist
repair --offlineon a project with an active patch and both blobs,rollback --offlineexits 0 and restores the file:repair_invariants::repair_keeps_active_patch_before_blob_so_offline_rollback_still_works(red on main:summary.removedswept the before blob).scan --mode agent --pruneagainst a mock API keeps the active patch's beforeHash blob:scan_paths_e2e::prune_keeps_before_blobs_of_active_patches(red on main).cargo test -p socket-patch-core --lib manifest::passes.repair:rollback_invariantsassertions updated.cli::output_modes_e2e::repair_non_json_*updated: repair now reports the active patch's beforeHash blob as in use (CI caught the stale expectation).Commands run
cargo test -p socket-patch-core --lib manifest::(104 passed)cargo test -p socket-patch-cli --test repair --test scan --test rollback --test remove --test covgap_commands_rollback --test remove_rollback_api_overrides --test diff_created_file_e2e(all green)cargo clippy --workspace --all-features -- -D warnings,cargo fmt --all -- --check(the one remaining diff is main'sredirect/upstream/mod.rs, which this PR doesn't touch)Overlap
PR #1273 (GC JSON shape) and #1279 (crate cleanup) also edit
repair.rs,scan/gc.rsandrollback.rs. Here those files change only on the one-line policy call and the remedy strings, so a rebase in either direction should be trivial.🤖 Generated with Claude Code
Generated by Claude Code
Note
Medium Risk
Changes blob GC semantics for active patches and rollback error guidance; incorrect retention could still break offline rollback or leak disk, but behavior is narrowly scoped and heavily tested.
Overview
repairandscan --prunenow keepbeforeHashblobs for every patch still in the manifest, aligning GC withremove/rollbackretention. PreviouslyArtifactReferences::for_applyswept originals on cleanup, which broke offlinerollbackafter a repair (andrepaironly re-fetchesafterHashblobs).ArtifactReferences::active(manifest)replacesfor_applyas the shared policy: retain each manifest patch’s afterHash, beforeHash, and diff archive; only unreferenced blobs/archives are removed.repairandscan/gccallactive;after_removalis built on top of it. Standalonecleanup_unused_blobs/cleanup_unused_archiveshelpers are removed in favor of the sweep path.Rollback missing-blob messaging no longer points users at
socket-patch repair; it tells them to re-run without--offlineor once the patch API is reachable.CLI_CONTRACT.mddocuments the unified retention rules forrepairandscan --prune. Tests cover offline rollback-after-repair, prune retention, and updated repair stdout expectations.Reviewed by Cursor Bugbot for commit 37bd179. Configure here.