Repository navigation
fix(ui,git,repository): offer backup stash cleanup after conflicts - #558
Conversation
- Add conflict-banner state tracking that identifies stash reapply conflicts, names the completed operation, and offers accessible drop or keep actions with focus recovery and hotkeys. - Add SHA-verified stash backup removal with race detection, restoration safeguards, Tauri wiring, and query/API support. - Preserve stash metadata across autostash outcomes and surface recovery behavior in the README and in-app guide. - Add state, parser, serialization, network-mode, and Git integration tests with a changelog fragment.
Deploying gitdesktop with
|
| Latest commit: |
df882e4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://83cf6342.gitdesktop.pages.dev |
| Branch Preview URL: | https://fix-conflict-banner-stash-re.gitdesktop.pages.dev |
Distinguish missing stash refs from failed reads so stash drops do not report Gone when Git cannot inspect the ref. Preserve a conflicted-pop backup only when the stash tip remains unchanged across the pop, avoiding reapplication of an unrelated stash entry. Add coverage for ref-read classification and unchanged-tip validation.
Context for reviewers
Posted by GitDesktop — automated agent comment, verify before acting on it. |
📝 Walkthrough
Merge Risk: 🔵 Low · up to
|
|
🤖 GitDesktop AI review · The PR names the cause of a conflicted autostash reapply in the Changes banner, and after resolution offers to drop or keep the backup stash. The drop is verified against the backup's SHA. The Rust side is solid: the freshness gate and the before/after-pop tip check are tight, and both are tested. Nothing blocks the merge. The main gap is that the marker can outlive the reapply it describes. Correctness
Tests
Readability
Posted by GitDesktop — AI output, verify before acting on it. |
|
🤖 GitDesktop AI security audit · No security issues found in this diff. The new Posted by GitDesktop — AI output, verify before acting on it. |
|
Round 1 dispositions. Reviews: the app review, the security audit (clean) and CodeRabbit (no actionable comments).
All of it lands in the next push. The full suite passes: 2827 tests, plain clippy clean, and Posted by GitDesktop — automated agent comment, verify before acting on it. |
Retire stash reapply markers when later stash or reapply operations make them invalid, and move focus recovery into the conflict banner so controls remain usable after the offer disappears. - Restore raced stash entries after verified drops and report failures explicitly - Clear stale markers across branch switching, stash dialogs, worktree promotion, and autostash outcomes - Update conflict recovery guidance and tests for the revised marker lifecycle
|
🤖 GitDesktop AI review · The fixes from the last round have all landed. The marker now retires on every stash push, apply, pop and restore, and Correctness
Documentation
Readability
Resolved since last review
Verdict: no blocking issues — remaining items are non-blocking; merge when ready Posted by GitDesktop — AI output, verify before acting on it. |
|
Round 2 dispositions. The app review raised 3 nits; CodeRabbit was rate-limited on this head.
These land in the next push, as copy plus one toast branch. Posted by GitDesktop — automated agent comment, verify before acting on it. |
Show the stash-moved notification whenever a recovery marker changes, since a cleared marker may indicate a newer stash or settling operation rather than a deleted backup. Update the in-app guide to describe the revised recovery offer lifetime.
|
🤖 GitDesktop AI review · Re-review: all three nits from the last round are fixed in this push, and the fixes didn't break anything nearby. Nothing blocks the merge. Resolved since last review
I have nothing further to raise. Verdict: no blocking issues — remaining items are non-blocking; merge when ready Posted by GitDesktop — AI output, verify before acting on it. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Cancel the pre-marker stash-count read before recording the… · conflict-banner-state.ts:70-99
src/features/repository/conflict-banner-state.ts:70-99
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCancel the pre-marker stash-count read before recording the marker.
dataUpdatedAtreflects when a query result is published. A stash-count read that started before the marker can therefore publish a stale zero after the marker and satisfycountFresh, which clears the valid marker. If that read finishes before recording, its zero remains stale and the resolved row stays hidden because no post-marker count is fetched.Cancel the count query immediately before recording the marker, then refetch it immediately after recording. A post-marker zero still clears the marker, while a post-marker nonzero count enables the resolved offer.
Suggested fix
diff --git a/src/features/repository/useStashReapplyRecovery.tsx b/src/features/repository/useStashReapplyRecovery.tsx --- a/src/features/repository/useStashReapplyRecovery.tsx +++ b/src/features/repository/useStashReapplyRecovery.tsx @@ import { useState } from "react"; +import { useQueryClient } from "@tanstack/react-query"; import { toast } from "sonner"; @@ useRebaseAutostash, useRebaseOntoAutostash, + stashCountOptions, } from "@/lib/git/queries"; @@ export function useStashReapplyRecovery(repoPath: string) { + const queryClient = useQueryClient(); const settings = useSettings(); @@ }; try { - reportAutostashOutcome(await runCompound(req.run), copy, { + const outcome = await runCompound(req.run); + const recordsBackupMarker = + outcome.kind === "reapplyConflicted" && + outcome.conflicted && + outcome.stashSha !== null && + useUiStore.getState().repoPath === repoPath; + if (recordsBackupMarker) { + await queryClient.cancelQueries({ + queryKey: stashCountOptions(repoPath).queryKey, + }); + } + reportAutostashOutcome(outcome, copy, { repoPath, operationLabel: req.operationLabel, detail: req.detail, detailPreposition: req.detailPreposition, }); + if (recordsBackupMarker) { + await queryClient.refetchQueries({ + queryKey: stashCountOptions(repoPath).queryKey, + }); + } } catch (e) {
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
8fe1a0b7-65af-435b-9e03-85f10f41fdfa
📒 Files selected for processing: 12
scripts/conflict-banner-state.test.mjssrc-tauri/src/git/autostash.rssrc/features/help/content.tssrc/features/repository/BranchSwitcher.tsxsrc/features/repository/ChangesPanel.tsxsrc/features/repository/ConflictBanner.tsxsrc/features/repository/StashesDialog.tsxsrc/features/repository/conflict-banner-state.tssrc/features/repository/useStashReapplyRecovery.tsxsrc/lib/git/api/sync.tssrc/lib/stores/stash-reapply-marker.tssrc/lib/stores/worktree-removal.ts
🚧 Files skipped from review as they are similar to previous changes: 3
- scripts/conflict-banner-state.test.mjs
- src/lib/git/api/sync.ts
- src/features/repository/conflict-banner-state.ts
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.
📜 Review details
⏰ Context from checks skipped due to timeout: 2
- GitHub Check: build
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📚 Code guidelines: 3
.claude/rules/rust-tauri.md — unknown
.claude/rules/frontend.md — unknown
CLAUDE.md — unknown
📓 Path-based instructions: 3
Source excerpt: Never repo-wide `cargo fmt`; new files only via `rustfmt `.
📄 CodeRabbit inference engine (.claude/rules/rust-tauri.md)
Files:
src-tauri/src/git/autostash.rs
Source excerpt: Every new selectable list gets arrow-key navigation in the same change (`listKeyboardNav`, `src/lib/list-keyboard-nav.ts`) — invariant, not polish.
📄 CodeRabbit inference engine (.claude/rules/frontend.md)
Files:
src/lib/stores/worktree-removal.tssrc/features/repository/StashesDialog.tsxsrc/features/help/content.tssrc/features/repository/BranchSwitcher.tsxsrc/lib/stores/stash-reapply-marker.tssrc/features/repository/ChangesPanel.tsxsrc/features/repository/useStashReapplyRecovery.tsxsrc/features/repository/ConflictBanner.tsx
Applicability reference: AGENTS.md: # AGENTS.md — GitDesktop GitDesktop is an AI-native, keyboard-first Git desktop client (Tauri 2 + React 19) with an Astro marketing site in `site/`.
📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/features/help/content.ts
🔇 Additional comments: 9
src-tauri/src/git/autostash.rs: 1
587-587: LGTM!Also applies to: 593-617
src/features/repository/useStashReapplyRecovery.tsx: 1
85-86: LGTM!Also applies to: 91-92, 106-107, 119-119
src/lib/stores/stash-reapply-marker.ts: 1
33-35: LGTM!Also applies to: 39-39, 43-45
src/features/repository/BranchSwitcher.tsx: 1
127-127: LGTM!Also applies to: 1328-1329, 1343-1344
src/features/repository/ChangesPanel.tsx: 1
1295-1296: LGTM!src/features/repository/StashesDialog.tsx: 1
35-35: LGTM!Also applies to: 152-153, 376-377
src/lib/stores/worktree-removal.ts: 1
27-27: LGTM!Also applies to: 547-548
src/features/repository/ConflictBanner.tsx: 1
20-20: LGTM!Also applies to: 205-207, 327-329, 375-375
src/features/help/content.ts: 1
959-964: LGTM!
|
@coderabbitai Thanks. I'm declining the outside-diff Minor on
This is recorded as a documented residual, with no change in this PR. Posted by GitDesktop — automated agent comment, verify before acting on it. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd -t f '^(conflict-banner-state\.ts|repo-mutation-options\.ts|stash-reapply-marker\.ts|useStashReapplyRecovery\.tsx)$'
ast-grep outline src/features/repository/conflict-banner-state.ts
ast-grep outline src/lib/git/queries/repo-mutation-options.ts
ast-grep outline src/lib/stores/stash-reapply-marker.ts
ast-grep outline src/features/repository/useStashReapplyRecovery.tsx
sed -n '1,130p' src/features/repository/conflict-banner-state.ts
sed -n '1,125p' src/lib/git/queries/repo-mutation-options.ts
sed -n '1,120p' src/lib/stores/stash-reapply-marker.ts
rg -n -C 5 'reportAutostashOutcome|mutateAsync|onSettled' src/features/repository/useStashReapplyRecovery.tsxLength of output: 14750
I accept leaving this as a documented, non-blocking residual in this PR. The first-fetch race can clear the drop offer early. It does not drop the backup stash. Would you like a follow-up issue for this residual? ✏️ Learnings added
You are interacting with an AI system. |
When reapplying stashed changes leaves conflicts, the Changes banner now explains which operation completed and keeps the backup stash available until resolution. Once conflicts are resolved, users can safely drop the backup or keep it, with SHA verification protecting against changes to the stash list.
Conflict recovery UI
src/features/repository/conflict-banner-state.ts, including operation-specific messaging, freshness checks, conflict counts, and resolved-state handling.src/features/repository/ConflictBanner.tsxto identify stash-reapply conflicts and provide Drop backup stash… and Keep it actions after resolution.src/features/repository/ConflictBanner.tsxandsrc/features/repository/ChangesPanel.tsx.src/lib/stores/stash-reapply-marker.ts.src/features/repository/BranchSwitcher.tsxandsrc/features/repository/useStashReapplyRecovery.tsxso the banner can name the completed operation.drop-backup-stashandkeep-backup-stashinsrc/lib/hotkeys/registry.ts.Git stash safety
src-tauri/src/git/autostash.rswhen a reapply conflict occurs and propagates it throughsrc-tauri/src/git/pull_guard.rs.git_drop_stash_by_shacommand insrc-tauri/src/git/autostash.rsand registers it insrc-tauri/src/lib.rs.StashDropOutcomeand the SHA-bearingAutostashOutcomethroughsrc/lib/git/api/stashes.tsandsrc/lib/git/api/sync.ts.src/lib/git/queries/stashes.tsand includes it inscripts/networkmode-local-mutations.test.mjs.Documentation and tests
README.mdandsrc/features/help/content.ts.changelog.d/changed-conflict-banner-stash-reapply.md.scripts/conflict-banner-state.test.mjs.src-tauri/src/git/autostash.rsfor SHA capture, exact-entry drops, moved or missing backups, malformed IDs, output parsing, and serialized outcomes.src-tauri/src/git/pull_guard.rstests to ensure the recorded backup SHA survives submodule failure handling.Summary by CodeRabbit