fix(stash): quiet-flag stash target resolution + working-log merge dedup + stash cycle regression tests#1722
Open
svarlamov wants to merge 1 commit into
Open
fix(stash): quiet-flag stash target resolution + working-log merge dedup + stash cycle regression tests#1722svarlamov wants to merge 1 commit into
svarlamov wants to merge 1 commit into
Conversation
…op cycles Stash push copied the working log into the stash dir but only cleared INITIAL, never checkpoints.jsonl; pop then raw-appended the copy back. Each push/pop cycle doubled the file (20 cycles ~= 10^6x), and since append_checkpoint rewrites the whole file per checkpoint, a multi-GB file turned into multi-GB daemon allocations (20GB+ RSS reports). - clean_working_log_for_stash now truncates the live checkpoints.jsonl on full stash (without reading it, so push on an already-bloated log is an O(1) self-heal) and complement-filters it on pathspec'd stash, mirroring filter_stash_checkpoints_to_pathspecs on the stash copy. - restore_stash_attributions dedups byte-identical lines on append, so repeated `stash apply` and conflict-tolerant re-restores are idempotent. - Stash attribution dir is versioned (stashes -> stashes_v2) and the legacy dir is deleted on any stash op, so pre-fix multi-GB stash worklogs can never be re-appended or loaded into memory again. - merge_working_log_dirs dedups checkpoints by (kind, diff, timestamp, author) so HEAD-move merges can't duplicate either. - Fix `stash pop -q` / `apply -q` never restoring attribution: enrich_stash treated the first arg after the subcommand as the stash target, so boolean flags broke stash-sha resolution and the restore silently no-op'd. All changes live in the async side-effect layer; nothing was added to the ingestion path and no new git spawns. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Collaborator
|
See: #1721 |
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
2 participants
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 (which fixed the stash checkpoint storage growth). While independently root-causing the same 18GB
checkpoints.jsonl/ 20GB+ daemon RSS explosion, end-to-end verification with a real daemon surfaced two additional bugs that #1719 did not cover, plus a broader regression-test suite for the stash/pop explosion.1.
git stash pop -q/apply -qnever restored attribution (src/daemon/ref_cursor.rs)enrich_stashtreated the first argv token after the stash subcommand as the stash target:stash_args.get(1). Forgit stash pop -qthat's-q, so stash-sha resolution failed and the attribution restore silently no-op'd — the stash entry was consumed by git but its attribution was dropped, and on pop thestashes_v2entry was leaked (never cleaned up). Coding agents typically pass-q, so this is exactly the flow from the original bug report.Fixed with
stash_positional_arg, which skips flags when locating the positional target (and the branch-name/target positions forstash branch). Repro before fix:git stash push -q && git stash pop -q→ attribution gone; without-q→ restored.2.
merge_working_log_dirsduplicated identical checkpoints (src/git/repo_storage.rs)When a working log merges into an existing one on HEAD moves (checkout/switch), checkpoints were concatenated with plain
Vec::extend— a log merged forward and back accumulated duplicates, the same compounding shape as the stash bug at lower frequency. Now dedups by(kind, diff, timestamp, author)(diffis a content hash, so distinct checkpoints never collide).3. Regression tests for the stash explosion (tests/integration/stash_attribution.rs)
TDD-written tests (each watched fail on the pre-#1719 code) covering the full bug surface, adapted to the compact
stashes_v2snapshot design:test_stash_pop_cycles_do_not_grow_checkpoints— 5 push/pop cycles, live line count must not grow (was 32× pre-fix), no leaked stash entriestest_stash_push_clears_live_checkpoints_and_pop_restorestest_stash_apply_repeated_is_idempotent— incl. the conflict-tolerant re-restore after a failed second applytest_stash_push_pathspec_keeps_unstashed_checkpoints_in_live_log— live-log complement of the stash-side pathspec filtertest_stash_pop_after_head_move_stays_bounded— shifted-pop pathtest_legacy_stashes_dir_is_removedtest_stash_pop_quiet_flag_restores_attribution/test_stash_apply_quiet_flag_restores_attribution— the ref_cursor fix; fail without ittest_merge_working_log_dirs_dedups_identical_checkpoints(unit) — the repo_storage fix; fails without itVerification
End-to-end with a real installed daemon (
task dev): 10 files × 300 lines with AI checkpoints, then the reported repro — 20 manualstash push -q/pop -qcycles.checkpoints.jsonlbyte-identical to baseline (7,931 bytes), daemon RSS flat (~3.5MB), zero leaked stash entries, legacystashes/dir removed, andgit-ai blameshows full AI attribution after commit (broken before the ref_cursor fix).Full stash suite (114 tests), ref_cursor/checkout/switch suites,
task lint,task fmtall green locally.🤖 Generated with Claude Code