feat(settings): add automated remote branch cleanup on task archive/delete#1442
feat(settings): add automated remote branch cleanup on task archive/delete#1442singhvibhanshu wants to merge 3 commits intogeneralaction:mainfrom
Conversation
|
@singhvibhanshu is attempting to deploy a commit to the General Action Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR introduces a configurable Key points:
Confidence Score: 2/5
|
| Filename | Overview |
|---|---|
| src/main/services/RemoteBranchService.ts | New service for remote branch deletion — well-structured, but the overly broad /not found/i pattern can mask real network errors as "already absent", and isBranchStale treats unresolvable branches as stale which can trigger silent deletions. |
| src/renderer/hooks/useTaskManagement.ts | Remote branch cleanup logic integrated cleanly before optimistic mutations; window.confirm for 'ask' mode is inconsistent with the app's modal system and fragile in some Electron configurations. |
| src/renderer/components/RemoteBranchCleanupCard.tsx | Clean settings UI; uses defaultValue (uncontrolled) for the days-threshold input, which can show a stale value if settings update while the component is mounted. |
| src/shared/remoteBranchCleanup.ts | Well-defined shared types, constants, and validation helpers for the cleanup modes; clampCleanupDays correctly handles non-finite and out-of-range inputs. |
| src/main/ipc/gitIpc.ts | Two new IPC handlers (git:delete-remote-branch, git:evaluate-branch-cleanup) added correctly with input validation and graceful error returns. |
| src/main/settings.ts | New remoteBranchCleanup and remoteBranchCleanupDaysThreshold settings correctly added with validation, defaults, and normalization in normalizeSettings. |
| src/main/services/WorktreeService.ts | Remote branch deletion in removeWorktree is gated on an explicit deleteRemoteBranch option flag and only executes when requested, keeping existing behavior unchanged by default. |
| src/main/preload.ts | New deleteRemoteBranch and evaluateBranchCleanup methods correctly exposed via contextBridge. |
Sequence Diagram
sequenceDiagram
participant UI as Renderer (useTaskManagement)
participant IPC as IPC Bridge (preload)
participant GitIPC as gitIpc.ts
participant RBS as RemoteBranchService
participant WS as WorktreeService
UI->>UI: shouldDeleteRemoteBranch(project, task)
UI->>IPC: evaluateBranchCleanup({projectPath, branch, mode, daysThreshold})
IPC->>GitIPC: git:evaluate-branch-cleanup
GitIPC->>RBS: evaluateCleanupAction(...)
RBS-->>GitIPC: 'delete' | 'skip' | 'ask'
GitIPC-->>IPC: { success, action }
IPC-->>UI: { success, action }
alt action === 'ask'
UI->>UI: window.confirm(...)
end
alt Archive task
UI->>UI: archiveTaskMutation.mutateAsync({deleteRemoteBranch})
UI->>IPC: deleteRemoteBranch({projectPath, branch})
IPC->>GitIPC: git:delete-remote-branch
GitIPC->>RBS: deleteRemoteBranch(projectPath, branch)
RBS-->>GitIPC: RemoteBranchDeletionResult
else Delete task
UI->>UI: deleteTaskMutation.mutateAsync({deleteRemoteBranch})
UI->>IPC: worktreeRemove({..., deleteRemoteBranch})
IPC->>WS: removeWorktree(..., {deleteRemoteBranch})
WS->>RBS: deleteRemoteBranch(projectPath, branch)
RBS-->>WS: RemoteBranchDeletionResult
end
Last reviewed commit: d88cadc
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /error: unable to delete '[^']*': remote ref does not exist/i, | ||
| ]; |
There was a problem hiding this comment.
Overly broad not found pattern can mask network errors
The pattern /not found/i is too generic. Git error messages for network failures (e.g., Could not resolve hostname 'github.com': Name or service not found, SSL_connect: Connection not found) also contain "not found". When this pattern matches a transient network error, the code returns { success: true, alreadyAbsent: true } — silently reporting success and masking what was actually a failed deletion.
Consider tightening the pattern to specifically match git branch-not-found output:
| /error: unable to delete '[^']*': remote ref does not exist/i, | |
| ]; | |
| const ALREADY_ABSENT_PATTERNS = [ | |
| /remote ref does not exist/i, | |
| /unknown revision/i, | |
| /error: unable to delete '[^']*': remote ref does not exist/i, | |
| ]; |
The fourth pattern (error: unable to delete '...') is already a precise superset of the first for the branch-absent case; and unknown revision covers the remaining scenario, so dropping the broad /not found/i loses no real coverage while eliminating false positives.
There was a problem hiding this comment.
Implemented!
| const dateStr = await getLastCommitDate(projectPath, branch); | ||
| if (!dateStr) { | ||
| // Cannot determine — treat as stale so users aren't surprised by | ||
| // branches that silently escape cleanup. | ||
| return true; |
There was a problem hiding this comment.
Treating unknown commit date as stale can silently delete live branches
When getLastCommitDate returns null (branch doesn't exist locally, git failed, etc.), isBranchStale returns true, which causes evaluateCleanupAction to return 'delete'. This means any branch that can't be resolved locally — even a branch pushed very recently from another machine — will be unconditionally deleted in auto mode.
The safer conservative choice is to return false (not stale) when the age cannot be determined, preventing silent deletions of branches that cannot be verified:
| const dateStr = await getLastCommitDate(projectPath, branch); | |
| if (!dateStr) { | |
| // Cannot determine — treat as stale so users aren't surprised by | |
| // branches that silently escape cleanup. | |
| return true; | |
| if (!dateStr) { | |
| // Cannot determine age — skip deletion to avoid removing branches | |
| // that may still be active but lack a local tracking ref. | |
| return false; | |
| } |
The existing tests (returns true when date cannot be determined) explicitly assert the current "fail open" behaviour; those tests would need updating if this is changed.
| case 'delete': | ||
| return true; | ||
| case 'ask': | ||
| return window.confirm(`Also delete remote branch "${task.branch}" on origin?`); |
There was a problem hiding this comment.
window.confirm is inconsistent with the app's modal system
window.confirm renders a plain browser-style dialog, bypassing the project's custom modal infrastructure (useModalContext / showModal). In certain Electron security configurations (e.g., when allowRunningInsecureContent or contextIsolation settings are tightened), window.confirm can be suppressed or return false silently, which would cause the 'ask' mode to always skip deletion without any user prompt.
Consider invoking the existing modal system (or an electronAPI IPC call to dialog.showMessageBox) to keep UX consistent and avoid platform-specific surprises:
| return window.confirm(`Also delete remote branch "${task.branch}" on origin?`); | |
| return new Promise<boolean>((resolve) => { | |
| showModal('confirmDialog', { | |
| title: 'Delete remote branch?', | |
| description: `Also delete remote branch "${task.branch}" on origin?`, | |
| onConfirm: () => resolve(true), | |
| onCancel: () => resolve(false), | |
| }); | |
| }); |
(Adjust to match the actual modal API shape in your codebase.)
| type="number" | ||
| min={1} | ||
| max={365} | ||
| defaultValue={currentDays} |
There was a problem hiding this comment.
Uncontrolled input can show a stale value
defaultValue only sets the initial DOM value; subsequent changes to currentDays (e.g., after another settings write that triggers a re-render while the auto block remains mounted) will not update the displayed number. Using a controlled value + onChange pattern keeps the input in sync with the persisted setting:
| defaultValue={currentDays} | |
| value={currentDays} | |
| onChange={(e) => { | |
| const raw = parseInt(e.target.value, 10); | |
| if (!Number.isFinite(raw)) return; | |
| const clamped = Math.min(365, Math.max(1, raw)); | |
| updateSettings({ | |
| repository: { remoteBranchCleanupDaysThreshold: clamped }, | |
| }); | |
| }} |
You can remove the onBlur handler once onChange is in place (or keep both for debouncing purposes).
|
All flagged issues have been addressed in the latest commit |
|
Merge conflicts resolved. |
1 participant
Summary
Adds a configurable setting to automatically delete remote branches when a task is archived or deleted.
Previously, stale remote branches accumulated on GitHub, requiring manual cleanup. This PR introduces a
Remote Branch Cleanupsetting with four modes:window.confirmbefore deletion.Implementation Details:
RemoteBranchServicein the Node backend to executegit push --deletesafely.git:delete-remote-branch,git:evaluate-branch-cleanup) to connect the React UI with the Git service.Fixes
Fixes #1430
Type of change
Mandatory Tasks
Checklist
pnpm run format)pnpm run lint)