Fix compact stash recovery edge cases#1721
Closed
Siddhant-K-code wants to merge 6 commits into
Closed
Conversation
Collaborator
Author
|
It seems #1515 broke some windows test 🤔 |
Siddhant-K-code
marked this pull request as ready for review
July 2, 2026 09:08
…codex/fix-compact-stash-followups # Conflicts: # src/authorship/rewrite_stash.rs # tests/integration/stash_attribution.rs
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.
Summary
Follow-up to #1719 for the compact stash storage path.
stash pop -q/stash apply -qrestore attribution correctlyVirtualAttributionsreads them, so duplicated checkpoint logs do not get reintroduced through compact stash restoreRoot Cause
#1719 moved stash attribution into compact
INITIALsnapshots, which is the right storage shape, but two edge cases remained:RefCursor::enrich_stashtreated the first arg afterpop/applyas the stash target, so boolean flags like-qcould break stash SHA resolution and skip attribution restore.save_stash_attributionscompacted directly from the live working log, so an already-duplicatedcheckpoints.jsonlcould still be read in full before the cleanup path had a chance to truncate or replace it.Validation
Passed:
task fmtgit diff --checktask buildtask test TEST_FILTER=test_stash_pop_quiet_flag_restores_attributiontask test TEST_FILTER=test_stash_apply_quiet_flag_restores_attributiontask test TEST_FILTER=test_stash_compaction_recovers_from_duplicated_checkpoint_logtask test TEST_FILTER=test_repeated_stash_pop_does_not_duplicate_checkpointstask test TEST_FILTER=test_stash_push_pathspec_excludes_unstashed_file_from_stash_logtask test TEST_FILTER=test_partial_stash_trims_unstashed_initial_metadataLocal caveat:
task lintwas run, but local Rust/Clippy is1.96.0and failed on an untouched existingsrc/daemon.rsclippy::collapsible-matchwarning. The repo instructions call out Rust1.93.0; this branch does not modify that file.