Guard git quick actions for diverged or stale branch status#47
Guard git quick actions for diverged or stale branch status#47juliusmarminge merged 2 commits intomainfrom
Conversation
WalkthroughAdds branch divergence detection that disables the "Sync branch" quick action when a branch is both ahead and behind upstream. Introduces a null-safe git status proxy in the UI while resynchronizing and starts upstream refresh in the background on checkout; tests updated to handle asynchronous refresh/fetch behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Web UI (GitActionsControl)
participant API as Server API (checkout)
participant Core as GitCoreService
UI->>API: request checkoutBranch
API->>Core: perform checkout
Core-->>API: checkout result
API-->>UI: return checkout result (immediate)
API->>Core: start refreshCheckedOutBranchUpstream (async)
Note right of Core: background fetch may be delayed/long-running
Core-->>API: (background) fetch complete
API->>UI: subsequent status invalidation/notification
UI->>UI: set gitStatusForActions = null while resyncing
UI-->>UI: show "Refreshing git status..."
UI->>API: refetch git status
API->>Core: query status
Core-->>API: latest status
API->>UI: deliver updated git status (gitStatusForActions populated)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Comment |
Guard GitActionsControl quick actions for diverged or stale git status and run
|
Greptile SummaryImproved git status synchronization and diverged branch handling by preventing quick actions when branches have both ahead and behind commits. The PR adds stale status detection that triggers automatic query refresh when Key changes:
Critical Issue: The Confidence Score: 3/5
Important Files Changed
Flowchartflowchart TD
A[Git Status Query] --> B{isGitStatusOutOfSync?}
B -->|Yes| C[Invalidate Git Queries]
B -->|No| D[Use gitStatus]
B -->|Yes| E[Set gitStatusForActions = null]
D --> F{resolveQuickAction}
E --> F
F --> G{gitStatus null?}
G -->|Yes| H[Show unavailable hint]
G -->|No| I{isBusy?}
I -->|Yes| J[Show busy hint]
I -->|No| K{hasBranch?}
K -->|No| L[Show branch hint]
K -->|Yes| M{hasChanges?}
M -->|Yes| N[Return commit action]
M -->|No| O{isDiverged?}
O -->|Yes| P[Show sync disabled hint]
O -->|No| Q{isBehind?}
Q -->|Yes| R[Return pull action]
Q -->|No| S{isAhead?}
S -->|Yes| T[Return push/PR action]
S -->|No| U[Show up-to-date hint]
Last reviewed commit: 92b0dca |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| {gitStatusForActions && | ||
| gitStatusForActions.branch !== null && | ||
| !gitStatusForActions.hasWorkingTreeChanges && | ||
| gitStatusForActions.behindCount > 0 && | ||
| gitStatusForActions.aheadCount === 0 && ( |
There was a problem hiding this comment.
Consider adding a clearer message for diverged branches here instead of only showing "Behind upstream" for non-diverged cases. Users might be confused why the warning disappears when they make local commits on an already-behind branch.
Additional Comments (1)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/GitActionsControl.tsx (1)
188-346:⚠️ Potential issue | 🟠 MajorBlock actions while git status is out-of-sync.
IfgitStatusForActionsis null (out-of-sync), an already-open dialog can still execute actions, which bypasses the new gating and can skip default-branch confirmation. Add a guard to prevent running actions until the status is synchronized.🛡️ Suggested guard to enforce out-of-sync gating
}) => { + if (isGitStatusOutOfSync || !gitStatusForActions) { + toastManager.add({ + type: "info", + title: "Refreshing git status...", + description: "Please retry once the status is synchronized.", + }); + return; + } const confirmed = await maybeConfirmPushToDefaultBranch(action); if (!confirmed) return; onConfirmed?.(); @@ [ api, + isGitStatusOutOfSync, gitStatusForActions?.branch, gitStatusForActions?.hasWorkingTreeChanges, gitStatusForActions?.openPr?.url,
- Disable the quick sync action when local and upstream branches have diverged - Treat mismatched branch/status data as out-of-sync and refresh before enabling actions - Update GitActionsControl logic tests for diverged branch behavior Co-authored-by: codex <codex@users.noreply.github.com>
Co-authored-by: codex <codex@users.noreply.github.com>
juliusmarminge
force-pushed
the
fix/too-aggresive-pull-action
branch
from
26c2e84 to
724ecbe
Compare
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| !requiresDefaultBranchConfirmation(action, isDefaultBranch) || | ||
| !gitStatusForActions?.branch | ||
| ) { | ||
| return true; |
There was a problem hiding this comment.
🟢 Low
components/GitActionsControl.tsx:202 When gitStatusForActions is null (during sync), maybeConfirmPushToDefaultBranch returns true immediately, bypassing the confirmation prompt. Consider returning false when gitStatusForActions?.branch is falsy to block the action until status is available.
| return true; | |
| return false; |
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/web/src/components/GitActionsControl.tsx around line 202:
When `gitStatusForActions` is `null` (during sync), `maybeConfirmPushToDefaultBranch` returns `true` immediately, bypassing the confirmation prompt. Consider returning `false` when `gitStatusForActions?.branch` is falsy to block the action until status is available.
Port upstream commits ac3f22c..268016d: - Inline branch picker filtering and drop PR-only shortcut (pingdotgg#46) - Fix PR thread upstream tracking for fork branches (pingdotgg#47) Removes filterBranchPickerItems shared function in favor of inline filtering, adds URL-derived fork identity resolution, ensures upstream tracking is restored on local checkouts and reused worktrees, and simulates real git checkout in fake GH CLI for integration tests.
1 participant
Summary
aheadandbehind), and show a disabled hint to rebase/merge first.gitStatus.branchis out of sync with the current branch list and temporarily gate actions until queries refresh.Testing
apps/web/src/components/GitActionsControl.logic.test.ts: added test asserting diverged status resolves to disabledSync branchhint.Summary by CodeRabbit
New Features
Bug Fixes
Tests