Force push (vibe-kanban) (#1350)
* Excellent! All tasks completed successfully. Here's a summary of the changes:
I've successfully added typed error handling to the push method following the same pattern as `CreatePrError`. Here's what was implemented:
1. **Created `PushError` enum** in `crates/server/src/routes/task_attempts.rs:608-613`
- Single variant: `ForcePushRequired`
- Uses proper derives for TypeScript generation
2. **Updated push handler signature** in `crates/server/src/routes/task_attempts.rs:596`
- Changed return type to `Result<ResponseJson<ApiResponse<(), PushError>>, ApiError>`
3. **Registered error type** in `crates/server/src/bin/generate_types.rs:140`
- Added `PushError::decl()` to the type declarations
4. **Generated TypeScript type** in `shared/types.ts:301`
- Type: `export type PushError = { "type": "force_push_required" }`
5. **Updated frontend API** in `frontend/src/lib/api.ts`
- Imported `PushError` type
- Changed `push` method to return `Result<void, PushError>`
- Uses `handleApiResponseAsResult` for proper error handling
6. **Updated `usePush` hook** in `frontend/src/hooks/usePush.ts`
- Created custom `PushErrorWithData` class to properly pass error data
- Updated signature to pass `errorData` to `onError` callback
7. **Updated component error handling** in `frontend/src/hooks/useGitOperations.ts:47-52`
- Added handling for `force_push_required` error type
- Displays user-friendly message: "Force push required. The remote branch has diverged."
All checks pass successfully! The implementation is ready for future force push functionality.
Cleanup script changes for task attempt 982f21cc-f0a3-455d-8b30-3a9911c2889f
Excellent! All tasks completed successfully. Here's a summary of the force push implementation:
I've successfully implemented a complete force push feature with user confirmation. Here's what was done:
1. **Updated Git CLI** (`crates/services/src/services/git/cli.rs:318`)
- Added `force: bool` parameter to `push()` method
- Uses `+refs/heads/{branch}` refspec when force is true
2. **Updated Git Service** (`crates/services/src/services/git.rs:1615`)
- Added `force: bool` parameter to `push_to_github()` method
- Passes force flag to Git CLI
3. **Added Force Push Endpoint** (`crates/server/src/routes/task_attempts.rs:608`)
- New handler: `force_push_task_attempt_branch`
- Calls `push_to_github()` with `force: true`
- Regular push endpoint now passes `force: false`
4. **Added Route** (`crates/server/src/routes/task_attempts.rs:1581`)
- Route: `POST /api/task-attempts/:id/push/force`
5. **API Client** (`frontend/src/lib/api.ts:561`)
- Added `forcePush()` method
- Returns `Result<void, PushError>`
6. **Created useForcePush Hook** (`frontend/src/hooks/useForcePush.ts`)
- React Query mutation for force push
- Handles typed errors
- Invalidates branch status on success
7. **Created ForcePushDialog** (`frontend/src/components/dialogs/git/ForcePushDialog.tsx`)
- Destructive variant with warning icon
- Clear warning about overwriting remote changes
- Buttons: "Cancel" and "Force Push"
8. **Integrated Dialog Flow** (`frontend/src/hooks/useGitOperations.ts:58-85`)
- When regular push fails with `force_push_required` error:
- Shows ForcePushDialog to confirm
- If user confirms, executes force push
- Handles errors from force push operation
1. User attempts to push
2. If remote has diverged, backend returns `force_push_required` error
3. Frontend shows confirmation dialog explaining the consequences
4. User can cancel or confirm force push
5. If confirmed, force push is executed
6. Success/error feedback is shown
All checks pass successfully! The feature is ready to use.
Cleanup script changes for task attempt 982f21cc-f0a3-455d-8b30-3a9911c2889f
Perfect! The fix is complete and all checks pass. Here's what was changed:
**Problem:** The `push_task_attempt_branch` endpoint was using the `?` operator which converted `GitServiceError::GitCLI(GitCliError::PushRejected(_))` into a generic `ApiError`, preventing the frontend from receiving the typed `PushError::ForcePushRequired` error.
**Solution:** Updated the endpoint in `crates/server/src/routes/task_attempts.rs:602-613` to use a `match` statement that:
1. Returns success on `Ok(_)`
2. Detects `GitServiceError::GitCLI(GitCliError::PushRejected(_))` and returns the typed error `ApiResponse::error_with_data(PushError::ForcePushRequired)`
3. Returns generic `ApiError::GitService(e)` for other errors
Now when a push is rejected due to non-fast-forward, the backend will return:
```json
{
"success": false,
"error_data": {
"type": "force_push_required"
}
}
```
This will trigger the force push confirmation dialog in the frontend, allowing the user to choose whether to force push or cancel.
Cleanup script changes for task attempt 982f21cc-f0a3-455d-8b30-3a9911c2889f
Perfect! The fix is complete and all checks pass. Here's what was changed:
**Problem:** The ForcePushDialog was not closing after the user clicked "Force Push" or "Cancel" because it only called `modal.resolve()` without calling `modal.hide()`.
**Solution:** Added `modal.hide()` calls to both handlers in `frontend/src/components/dialogs/git/ForcePushDialog.tsx`:
- **Line 24:** Added `modal.hide()` after `modal.resolve('confirmed')` in `handleConfirm`
- **Line 29:** Added `modal.hide()` after `modal.resolve('canceled')` in `handleCancel`
Now the dialog will properly close when the user clicks either button. This follows the correct NiceModal pattern where you need both:
1. `modal.resolve()` - to resolve the promise with a result
2. `modal.hide()` - to actually hide the dialog
The force push feature should now work end-to-end: push rejection → confirmation dialog → force push → dialog closes.
Perfect! All checks pass. Here's what was fixed:
The force push dialog now follows the same pattern as all other async dialogs in the codebase.
**Added:**
- `attemptId` prop (required) to identify which task attempt to force push
- `useForcePush` hook usage internally
- Loading state with `forcePush.isPending`
- Error state displayed in the dialog
- Loader spinner and "Force Pushing..." button text during operation
- Disabled buttons during operation
- Dialog stays open until operation completes
**Behavior:**
- User clicks "Force Push" → button shows spinner and "Force Pushing..."
- On success → dialog closes automatically
- On error → error shown in dialog, stays open, user can retry or cancel
**Changed:**
- Removed the manual force push call after dialog confirmation
- Now passes `attemptId` to the dialog: `ForcePushDialog.show({ attemptId })`
- Dialog handles the entire force push operation internally
1. User attempts push
2. Push rejected with `force_push_required` error
3. Dialog appears with warning
4. User clicks "Force Push"
5. **Button shows spinner and "Force Pushing..."** ← New!
6. **Buttons disabled during operation** ← New!
7. On success: dialog closes
8. On error: error displayed in dialog, user can retry
This matches the pattern used by CreatePRDialog, GhCliSetupDialog, and other async dialogs in the codebase.
* Force push i18n (vibe-kanban 5519a7db)
Run @scripts/check-i18n.sh until it passes. Make sure to check the script and set GITHUB_BASE_REF to vk/607c-add-pre-flight-c
Force push i18n (vibe-kanban 5519a7db)
Run @scripts/check-i18n.sh until it passes. Make sure to check the script and set GITHUB_BASE_REF to vk/607c-add-pre-flight-c
* fix tests
This commit is contained in:
112
frontend/src/components/dialogs/git/ForcePushDialog.tsx
Normal file
112
frontend/src/components/dialogs/git/ForcePushDialog.tsx
Normal file
@@ -0,0 +1,112 @@
|
||||
import {
|
||||
Dialog,
|
||||
DialogContent,
|
||||
DialogDescription,
|
||||
DialogFooter,
|
||||
DialogHeader,
|
||||
DialogTitle,
|
||||
} from '@/components/ui/dialog';
|
||||
import { Button } from '@/components/ui/button';
|
||||
import NiceModal, { useModal } from '@ebay/nice-modal-react';
|
||||
import { AlertTriangle, Loader2 } from 'lucide-react';
|
||||
import { defineModal } from '@/lib/modals';
|
||||
import { useForcePush } from '@/hooks/useForcePush';
|
||||
import { useState } from 'react';
|
||||
import { Alert, AlertDescription } from '@/components/ui/alert';
|
||||
import { useTranslation } from 'react-i18next';
|
||||
|
||||
export interface ForcePushDialogProps {
|
||||
attemptId: string;
|
||||
branchName?: string;
|
||||
}
|
||||
|
||||
const ForcePushDialogImpl = NiceModal.create<ForcePushDialogProps>((props) => {
|
||||
const modal = useModal();
|
||||
const { attemptId, branchName } = props;
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
const { t } = useTranslation(['tasks', 'common']);
|
||||
const branchLabel = branchName ? ` "${branchName}"` : '';
|
||||
|
||||
const forcePush = useForcePush(
|
||||
attemptId,
|
||||
() => {
|
||||
// Success - close dialog
|
||||
modal.resolve('success');
|
||||
modal.hide();
|
||||
},
|
||||
(err: unknown) => {
|
||||
// Error - show in dialog and keep open
|
||||
const message =
|
||||
err && typeof err === 'object' && 'message' in err
|
||||
? String(err.message)
|
||||
: t('tasks:git.forcePushDialog.error');
|
||||
setError(message);
|
||||
}
|
||||
);
|
||||
|
||||
const handleConfirm = async () => {
|
||||
setError(null);
|
||||
try {
|
||||
await forcePush.mutateAsync();
|
||||
} catch {
|
||||
// Error already handled by onError callback
|
||||
}
|
||||
};
|
||||
|
||||
const handleCancel = () => {
|
||||
modal.resolve('canceled');
|
||||
modal.hide();
|
||||
};
|
||||
|
||||
const isProcessing = forcePush.isPending;
|
||||
|
||||
return (
|
||||
<Dialog open={modal.visible} onOpenChange={handleCancel}>
|
||||
<DialogContent className="sm:max-w-[500px]">
|
||||
<DialogHeader>
|
||||
<div className="flex items-center gap-3">
|
||||
<AlertTriangle className="h-6 w-6 text-destructive" />
|
||||
<DialogTitle>{t('tasks:git.forcePushDialog.title')}</DialogTitle>
|
||||
</div>
|
||||
<DialogDescription className="text-left pt-2 space-y-2">
|
||||
<p>{t('tasks:git.forcePushDialog.description', { branchLabel })}</p>
|
||||
<p className="font-medium">
|
||||
{t('tasks:git.forcePushDialog.warning')}
|
||||
</p>
|
||||
<p className="text-sm text-muted-foreground">
|
||||
{t('tasks:git.forcePushDialog.note')}
|
||||
</p>
|
||||
</DialogDescription>
|
||||
</DialogHeader>
|
||||
{error && (
|
||||
<Alert variant="destructive">
|
||||
<AlertDescription>{error}</AlertDescription>
|
||||
</Alert>
|
||||
)}
|
||||
<DialogFooter className="gap-2">
|
||||
<Button
|
||||
variant="outline"
|
||||
onClick={handleCancel}
|
||||
disabled={isProcessing}
|
||||
>
|
||||
{t('common:buttons.cancel')}
|
||||
</Button>
|
||||
<Button
|
||||
variant="destructive"
|
||||
onClick={handleConfirm}
|
||||
disabled={isProcessing}
|
||||
>
|
||||
{isProcessing && <Loader2 className="mr-2 h-4 w-4 animate-spin" />}
|
||||
{isProcessing
|
||||
? t('tasks:git.states.forcePushing')
|
||||
: t('tasks:git.states.forcePush')}
|
||||
</Button>
|
||||
</DialogFooter>
|
||||
</DialogContent>
|
||||
</Dialog>
|
||||
);
|
||||
});
|
||||
|
||||
export const ForcePushDialog = defineModal<ForcePushDialogProps, string>(
|
||||
ForcePushDialogImpl
|
||||
);
|
||||
45
frontend/src/hooks/useForcePush.ts
Normal file
45
frontend/src/hooks/useForcePush.ts
Normal file
@@ -0,0 +1,45 @@
|
||||
import { useMutation, useQueryClient } from '@tanstack/react-query';
|
||||
import { attemptsApi } from '@/lib/api';
|
||||
import type { PushError } from 'shared/types';
|
||||
|
||||
class ForcePushErrorWithData extends Error {
|
||||
constructor(
|
||||
message: string,
|
||||
public errorData?: PushError
|
||||
) {
|
||||
super(message);
|
||||
this.name = 'ForcePushErrorWithData';
|
||||
}
|
||||
}
|
||||
|
||||
export function useForcePush(
|
||||
attemptId?: string,
|
||||
onSuccess?: () => void,
|
||||
onError?: (err: unknown, errorData?: PushError) => void
|
||||
) {
|
||||
const queryClient = useQueryClient();
|
||||
|
||||
return useMutation({
|
||||
mutationFn: async () => {
|
||||
if (!attemptId) return;
|
||||
const result = await attemptsApi.forcePush(attemptId);
|
||||
if (!result.success) {
|
||||
throw new ForcePushErrorWithData(
|
||||
result.message || 'Force push failed',
|
||||
result.error
|
||||
);
|
||||
}
|
||||
},
|
||||
onSuccess: () => {
|
||||
// A force push affects remote status; invalidate the same branchStatus
|
||||
queryClient.invalidateQueries({ queryKey: ['branchStatus', attemptId] });
|
||||
onSuccess?.();
|
||||
},
|
||||
onError: (err) => {
|
||||
console.error('Failed to force push:', err);
|
||||
const errorData =
|
||||
err instanceof ForcePushErrorWithData ? err.errorData : undefined;
|
||||
onError?.(err, errorData);
|
||||
},
|
||||
});
|
||||
}
|
||||
@@ -1,10 +1,12 @@
|
||||
import { useRebase } from './useRebase';
|
||||
import { useMerge } from './useMerge';
|
||||
import { usePush } from './usePush';
|
||||
import { useForcePush } from './useForcePush';
|
||||
import { useChangeTargetBranch } from './useChangeTargetBranch';
|
||||
import { useGitOperationsError } from '@/contexts/GitOperationsContext';
|
||||
import { Result } from '@/lib/api';
|
||||
import type { GitOperationError } from 'shared/types';
|
||||
import { ForcePushDialog } from '@/components/dialogs/git/ForcePushDialog';
|
||||
|
||||
export function useGitOperations(
|
||||
attemptId: string | undefined,
|
||||
@@ -41,10 +43,31 @@ export function useGitOperations(
|
||||
}
|
||||
);
|
||||
|
||||
const push = usePush(
|
||||
const forcePush = useForcePush(
|
||||
attemptId,
|
||||
() => setError(null),
|
||||
(err: unknown) => {
|
||||
const message =
|
||||
err && typeof err === 'object' && 'message' in err
|
||||
? String(err.message)
|
||||
: 'Failed to force push';
|
||||
setError(message);
|
||||
}
|
||||
);
|
||||
|
||||
const push = usePush(
|
||||
attemptId,
|
||||
() => setError(null),
|
||||
async (err: unknown, errorData) => {
|
||||
// Handle typed push errors
|
||||
if (errorData?.type === 'force_push_required') {
|
||||
// Show confirmation dialog - dialog handles the force push internally
|
||||
if (attemptId) {
|
||||
await ForcePushDialog.show({ attemptId });
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
const message =
|
||||
err && typeof err === 'object' && 'message' in err
|
||||
? String(err.message)
|
||||
@@ -70,6 +93,7 @@ export function useGitOperations(
|
||||
rebase.isPending ||
|
||||
merge.isPending ||
|
||||
push.isPending ||
|
||||
forcePush.isPending ||
|
||||
changeTargetBranch.isPending;
|
||||
|
||||
return {
|
||||
@@ -77,6 +101,7 @@ export function useGitOperations(
|
||||
rebase: rebase.mutateAsync,
|
||||
merge: merge.mutateAsync,
|
||||
push: push.mutateAsync,
|
||||
forcePush: forcePush.mutateAsync,
|
||||
changeTargetBranch: changeTargetBranch.mutateAsync,
|
||||
},
|
||||
isAnyLoading,
|
||||
@@ -84,6 +109,7 @@ export function useGitOperations(
|
||||
rebasePending: rebase.isPending,
|
||||
mergePending: merge.isPending,
|
||||
pushPending: push.isPending,
|
||||
forcePushPending: forcePush.isPending,
|
||||
changeTargetBranchPending: changeTargetBranch.isPending,
|
||||
},
|
||||
};
|
||||
|
||||
@@ -1,17 +1,34 @@
|
||||
import { useMutation, useQueryClient } from '@tanstack/react-query';
|
||||
import { attemptsApi } from '@/lib/api';
|
||||
import type { PushError } from 'shared/types';
|
||||
|
||||
class PushErrorWithData extends Error {
|
||||
constructor(
|
||||
message: string,
|
||||
public errorData?: PushError
|
||||
) {
|
||||
super(message);
|
||||
this.name = 'PushErrorWithData';
|
||||
}
|
||||
}
|
||||
|
||||
export function usePush(
|
||||
attemptId?: string,
|
||||
onSuccess?: () => void,
|
||||
onError?: (err: unknown) => void
|
||||
onError?: (err: unknown, errorData?: PushError) => void
|
||||
) {
|
||||
const queryClient = useQueryClient();
|
||||
|
||||
return useMutation({
|
||||
mutationFn: () => {
|
||||
if (!attemptId) return Promise.resolve();
|
||||
return attemptsApi.push(attemptId);
|
||||
mutationFn: async () => {
|
||||
if (!attemptId) return;
|
||||
const result = await attemptsApi.push(attemptId);
|
||||
if (!result.success) {
|
||||
throw new PushErrorWithData(
|
||||
result.message || 'Push failed',
|
||||
result.error
|
||||
);
|
||||
}
|
||||
},
|
||||
onSuccess: () => {
|
||||
// A push only affects remote status; invalidate the same branchStatus
|
||||
@@ -20,7 +37,9 @@ export function usePush(
|
||||
},
|
||||
onError: (err) => {
|
||||
console.error('Failed to push:', err);
|
||||
onError?.(err);
|
||||
const errorData =
|
||||
err instanceof PushErrorWithData ? err.errorData : undefined;
|
||||
onError?.(err, errorData);
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
@@ -197,6 +197,13 @@
|
||||
"branch": {
|
||||
"current": "current"
|
||||
},
|
||||
"forcePushDialog": {
|
||||
"title": "Force Push Required",
|
||||
"description": "The remote branch{{branchLabel}} has diverged from your local branch. A regular push was rejected.",
|
||||
"warning": "Force pushing will overwrite the remote changes with your local changes. This action cannot be undone.",
|
||||
"note": "Only proceed if you're certain you want to replace the remote branch history.",
|
||||
"error": "Failed to force push"
|
||||
},
|
||||
"status": {
|
||||
"commits_one": "commit",
|
||||
"commits_other": "commits",
|
||||
@@ -214,6 +221,8 @@
|
||||
"pushed": "Pushed!",
|
||||
"pushing": "Pushing...",
|
||||
"push": "Push",
|
||||
"forcePush": "Force Push",
|
||||
"forcePushing": "Force Pushing...",
|
||||
"creating": "Creating...",
|
||||
"createPr": "Create PR"
|
||||
},
|
||||
|
||||
@@ -186,6 +186,13 @@
|
||||
"branch": {
|
||||
"current": "actual"
|
||||
},
|
||||
"forcePushDialog": {
|
||||
"title": "Se requiere push forzado",
|
||||
"description": "La rama remota{{branchLabel}} se ha desviado de tu rama local. Se rechazó un push normal.",
|
||||
"warning": "El push forzado sobrescribirá los cambios remotos con tus cambios locales. Esta acción no se puede deshacer.",
|
||||
"note": "Solo continúa si estás seguro de que deseas reemplazar el historial remoto de la rama.",
|
||||
"error": "No se pudo hacer el push forzado"
|
||||
},
|
||||
"errors": {
|
||||
"changeTargetBranch": "Error al cambiar rama de destino",
|
||||
"mergeChanges": "Error al fusionar cambios",
|
||||
@@ -210,6 +217,8 @@
|
||||
},
|
||||
"states": {
|
||||
"createPr": "Crear PR",
|
||||
"forcePush": "Push forzado",
|
||||
"forcePushing": "Push forzado en curso...",
|
||||
"creating": "Creando...",
|
||||
"merge": "Fusionar",
|
||||
"merged": "¡Fusionado!",
|
||||
|
||||
@@ -186,6 +186,13 @@
|
||||
"branch": {
|
||||
"current": "現在"
|
||||
},
|
||||
"forcePushDialog": {
|
||||
"title": "強制プッシュが必要です",
|
||||
"description": "リモートブランチ{{branchLabel}}がローカルブランチと乖離しています。通常のプッシュは拒否されました。",
|
||||
"warning": "強制プッシュはリモートの変更をローカルの変更で上書きします。この操作は元に戻せません。",
|
||||
"note": "リモートのブランチ履歴を置き換えてもよいと確信できる場合のみ続行してください。",
|
||||
"error": "強制プッシュに失敗しました"
|
||||
},
|
||||
"errors": {
|
||||
"changeTargetBranch": "ターゲットブランチの変更に失敗しました",
|
||||
"mergeChanges": "変更のマージに失敗しました",
|
||||
@@ -217,6 +224,8 @@
|
||||
"push": "プッシュ",
|
||||
"pushed": "プッシュ完了!",
|
||||
"pushing": "プッシュ中...",
|
||||
"forcePush": "強制プッシュ",
|
||||
"forcePushing": "強制プッシュ中...",
|
||||
"rebase": "リベース",
|
||||
"rebasing": "リベース中..."
|
||||
},
|
||||
|
||||
@@ -186,6 +186,13 @@
|
||||
"branch": {
|
||||
"current": "현재"
|
||||
},
|
||||
"forcePushDialog": {
|
||||
"title": "강제 푸시가 필요합니다",
|
||||
"description": "원격 브랜치{{branchLabel}}가 로컬 브랜치와 분기되었습니다. 일반 푸시가 거부되었습니다.",
|
||||
"warning": "강제 푸시는 로컬 변경 사항으로 원격 변경을 덮어씁니다. 이 동작은 되돌릴 수 없습니다.",
|
||||
"note": "원격 브랜치 기록을 대체해도 확실한 경우에만 계속하세요.",
|
||||
"error": "강제 푸시에 실패했습니다"
|
||||
},
|
||||
"errors": {
|
||||
"changeTargetBranch": "대상 브랜치를 변경하지 못했습니다",
|
||||
"mergeChanges": "변경사항을 병합하지 못했습니다",
|
||||
@@ -217,6 +224,8 @@
|
||||
"push": "푸시",
|
||||
"pushed": "푸시됨!",
|
||||
"pushing": "푸시 중...",
|
||||
"forcePush": "강제 푸시",
|
||||
"forcePushing": "강제 푸시 중...",
|
||||
"rebase": "리베이스",
|
||||
"rebasing": "리베이스 중..."
|
||||
},
|
||||
|
||||
@@ -71,6 +71,7 @@ import {
|
||||
OpenEditorResponse,
|
||||
OpenEditorRequest,
|
||||
CreatePrError,
|
||||
PushError,
|
||||
} from 'shared/types';
|
||||
|
||||
// Re-export types for convenience
|
||||
@@ -550,11 +551,21 @@ export const attemptsApi = {
|
||||
return handleApiResponse<void>(response);
|
||||
},
|
||||
|
||||
push: async (attemptId: string): Promise<void> => {
|
||||
push: async (attemptId: string): Promise<Result<void, PushError>> => {
|
||||
const response = await makeRequest(`/api/task-attempts/${attemptId}/push`, {
|
||||
method: 'POST',
|
||||
});
|
||||
return handleApiResponse<void>(response);
|
||||
return handleApiResponseAsResult<void, PushError>(response);
|
||||
},
|
||||
|
||||
forcePush: async (attemptId: string): Promise<Result<void, PushError>> => {
|
||||
const response = await makeRequest(
|
||||
`/api/task-attempts/${attemptId}/push/force`,
|
||||
{
|
||||
method: 'POST',
|
||||
}
|
||||
);
|
||||
return handleApiResponseAsResult<void, PushError>(response);
|
||||
},
|
||||
|
||||
rebase: async (
|
||||
|
||||
Reference in New Issue
Block a user