diff --git a/apps/backend/internal/orchestrator/executor/executor_mocks_test.go b/apps/backend/internal/orchestrator/executor/executor_mocks_test.go index 0888049fb0..124561cff1 100644 --- a/apps/backend/internal/orchestrator/executor/executor_mocks_test.go +++ b/apps/backend/internal/orchestrator/executor/executor_mocks_test.go @@ -515,6 +515,9 @@ func (m *mockRepository) HasActiveTaskSessionsByEnvironment(ctx context.Context, func (m *mockRepository) HasActiveTaskSessionsByRepository(ctx context.Context, repositoryID string) (bool, error) { return false, nil } +func (m *mockRepository) CountActiveTaskSessionsByRepository(ctx context.Context, repositoryID string) (int, error) { + return 0, nil +} func (m *mockRepository) DeleteEphemeralTasksByAgentProfile(ctx context.Context, agentProfileID string) (int64, error) { return 0, nil } diff --git a/apps/backend/internal/task/dto/requests.go b/apps/backend/internal/task/dto/requests.go index 2bada497b4..1951654ff7 100644 --- a/apps/backend/internal/task/dto/requests.go +++ b/apps/backend/internal/task/dto/requests.go @@ -335,3 +335,7 @@ type BulkMoveTasksResponse struct { type TaskCountResponse struct { TaskCount int `json:"task_count"` } + +type RepositoryActiveSessionCountResponse struct { + ActiveSessionCount int `json:"active_session_count"` +} diff --git a/apps/backend/internal/task/handlers/process_handlers_test.go b/apps/backend/internal/task/handlers/process_handlers_test.go index e2b43ef0e2..6089681013 100644 --- a/apps/backend/internal/task/handlers/process_handlers_test.go +++ b/apps/backend/internal/task/handlers/process_handlers_test.go @@ -261,6 +261,9 @@ func (m *mockRepository) HasActiveTaskSessionsByEnvironment(ctx context.Context, func (m *mockRepository) HasActiveTaskSessionsByRepository(ctx context.Context, repositoryID string) (bool, error) { return false, nil } +func (m *mockRepository) CountActiveTaskSessionsByRepository(ctx context.Context, repositoryID string) (int, error) { + return 0, nil +} func (m *mockRepository) DeleteEphemeralTasksByAgentProfile(ctx context.Context, agentProfileID string) (int64, error) { return 0, nil } diff --git a/apps/backend/internal/task/handlers/repository_handlers.go b/apps/backend/internal/task/handlers/repository_handlers.go index 3dff3231bb..2c8b4052cf 100644 --- a/apps/backend/internal/task/handlers/repository_handlers.go +++ b/apps/backend/internal/task/handlers/repository_handlers.go @@ -51,6 +51,7 @@ func (h *RepositoryHandlers) registerHTTP(router *gin.Engine) { api.GET("/fs/list-dir", h.httpListDirectory) api.GET("/repositories/:id", h.httpGetRepository) api.GET("/repositories/:id/branches", h.httpListRepositoryBranches) + api.GET("/repositories/:id/active-session-count", h.httpGetRepositoryActiveSessionCount) api.PATCH("/repositories/:id", h.httpUpdateRepository) api.DELETE("/repositories/:id", h.httpDeleteRepository) api.GET("/repositories/:id/scripts", h.httpListRepositoryScripts) @@ -447,6 +448,15 @@ func (h *RepositoryHandlers) httpDeleteRepository(c *gin.Context) { c.Status(http.StatusNoContent) } +func (h *RepositoryHandlers) httpGetRepositoryActiveSessionCount(c *gin.Context) { + count, err := h.service.CountActiveSessionsByRepository(c.Request.Context(), c.Param("id")) + if err != nil { + handleNotFound(c, h.logger, err, "repository not found") + return + } + c.JSON(http.StatusOK, dto.RepositoryActiveSessionCountResponse{ActiveSessionCount: count}) +} + // WS handlers func reposToListResponse(repositories []*models.Repository) dto.ListRepositoriesResponse { diff --git a/apps/backend/internal/task/repository/interface.go b/apps/backend/internal/task/repository/interface.go index 988c32b059..73a390f660 100644 --- a/apps/backend/internal/task/repository/interface.go +++ b/apps/backend/internal/task/repository/interface.go @@ -126,6 +126,7 @@ type SessionRepository interface { HasActiveTaskSessionsByExecutor(ctx context.Context, executorID string) (bool, error) HasActiveTaskSessionsByEnvironment(ctx context.Context, environmentID string) (bool, error) HasActiveTaskSessionsByRepository(ctx context.Context, repositoryID string) (bool, error) + CountActiveTaskSessionsByRepository(ctx context.Context, repositoryID string) (int, error) DeleteEphemeralTasksByAgentProfile(ctx context.Context, agentProfileID string) (int64, error) DeleteTaskSession(ctx context.Context, id string) error GetPrimarySessionByTaskID(ctx context.Context, taskID string) (*models.TaskSession, error) diff --git a/apps/backend/internal/task/repository/sqlite/session.go b/apps/backend/internal/task/repository/sqlite/session.go index b84db3bd59..2c51c1f6a5 100644 --- a/apps/backend/internal/task/repository/sqlite/session.go +++ b/apps/backend/internal/task/repository/sqlite/session.go @@ -742,6 +742,21 @@ func (r *Repository) HasActiveTaskSessionsByRepository(ctx context.Context, repo return err == nil, err } +func (r *Repository) CountActiveTaskSessionsByRepository(ctx context.Context, repositoryID string) (int, error) { + var count int + err := r.ro.QueryRowContext(ctx, r.ro.Rebind(` + SELECT COUNT(*) + FROM task_sessions s + INNER JOIN task_repositories tr ON tr.task_id = s.task_id + WHERE s.state IN ('CREATED', 'STARTING', 'RUNNING', 'WAITING_FOR_INPUT') + AND tr.repository_id = ? + `), repositoryID).Scan(&count) + if err != nil { + return 0, err + } + return count, nil +} + // DeleteEphemeralTasksByAgentProfile deletes all ephemeral tasks (and their sessions) // that are using the specified agent profile. This is used during profile deletion // to clean up transient quick chat / config chat tasks. diff --git a/apps/backend/internal/task/repository/sqlite/session_test.go b/apps/backend/internal/task/repository/sqlite/session_test.go index e44b820e16..3d916e6c41 100644 --- a/apps/backend/internal/task/repository/sqlite/session_test.go +++ b/apps/backend/internal/task/repository/sqlite/session_test.go @@ -191,3 +191,119 @@ func TestIncrementTaskSessionUsage_EmptySessionIDNoOp(t *testing.T) { t.Errorf("empty session id should be a no-op, got %v", err) } } + +// seedRepoLink wires up a workspace, repository, task, task_repositories link +// row, and a task_session row in the given state. Used to exercise the join +// in CountActiveTaskSessionsByRepository. +func seedRepoLink(t *testing.T, repo *Repository, workspaceID, repositoryID, taskID, sessionID, state string) { + t.Helper() + now := time.Now().UTC() + _, err := repo.db.Exec(repo.db.Rebind(` + INSERT OR IGNORE INTO workspaces (id, name, created_at, updated_at) + VALUES (?, 'ws', ?, ?) + `), workspaceID, now, now) + if err != nil { + t.Fatalf("seed workspace: %v", err) + } + _, err = repo.db.Exec(repo.db.Rebind(` + INSERT OR IGNORE INTO repositories (id, workspace_id, name, created_at, updated_at) + VALUES (?, ?, 'repo', ?, ?) + `), repositoryID, workspaceID, now, now) + if err != nil { + t.Fatalf("seed repository: %v", err) + } + _, err = repo.db.Exec(repo.db.Rebind(` + INSERT OR IGNORE INTO tasks (id, workspace_id, title, created_at, updated_at) + VALUES (?, ?, 'test task', ?, ?) + `), taskID, workspaceID, now, now) + if err != nil { + t.Fatalf("seed task: %v", err) + } + _, err = repo.db.Exec(repo.db.Rebind(` + INSERT INTO task_repositories (id, task_id, repository_id, created_at, updated_at) + VALUES (?, ?, ?, ?, ?) + `), "tr-"+taskID+"-"+repositoryID, taskID, repositoryID, now, now) + if err != nil { + t.Fatalf("seed task_repositories: %v", err) + } + _, err = repo.db.Exec(repo.db.Rebind(` + INSERT INTO task_sessions (id, task_id, state, started_at, updated_at) + VALUES (?, ?, ?, ?, ?) + `), sessionID, taskID, state, now, now) + if err != nil { + t.Fatalf("seed task_session: %v", err) + } +} + +// TestCountActiveTaskSessionsByRepository_NoSessions verifies the count is +// zero when no sessions reference the repository at all. +func TestCountActiveTaskSessionsByRepository_NoSessions(t *testing.T) { + repo := newRepoForSessionTests(t) + count, err := repo.CountActiveTaskSessionsByRepository(context.Background(), "repo-empty") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if count != 0 { + t.Errorf("expected 0, got %d", count) + } +} + +// TestCountActiveTaskSessionsByRepository_CountsActiveOnly verifies the join +// counts sessions in active states (CREATED, STARTING, RUNNING, +// WAITING_FOR_INPUT) and excludes sessions in terminal states. +func TestCountActiveTaskSessionsByRepository_CountsActiveOnly(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + + // Two active sessions across two tasks linked to the repo. + seedRepoLink(t, repo, "ws-a", "repo-a", "task-a1", "sess-a1", "RUNNING") + seedRepoLink(t, repo, "ws-a", "repo-a", "task-a2", "sess-a2", "WAITING_FOR_INPUT") + // Terminal-state session linked to the repo — must NOT count. + seedRepoLink(t, repo, "ws-a", "repo-a", "task-a3", "sess-a3", "COMPLETED") + // Active session linked to a different repo — must NOT count. + seedRepoLink(t, repo, "ws-a", "repo-b", "task-b1", "sess-b1", "RUNNING") + + count, err := repo.CountActiveTaskSessionsByRepository(ctx, "repo-a") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if count != 2 { + t.Errorf("expected 2 active sessions, got %d", count) + } +} + +// TestCountActiveTaskSessionsByRepository_RequiresJoinRow verifies that a +// session whose task is NOT linked via task_repositories is not counted, even +// if the session is active. This guards against accidentally widening the +// query to use task_sessions.repository_id. +func TestCountActiveTaskSessionsByRepository_RequiresJoinRow(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + + // Seed workspace, repo, and a task with the link. + seedRepoLink(t, repo, "ws-j", "repo-j", "task-j1", "sess-j1", "RUNNING") + + // Seed a second task in the same workspace with an active session, but + // without inserting a task_repositories row pointing at repo-j. + now := time.Now().UTC() + if _, err := repo.db.Exec(repo.db.Rebind(` + INSERT INTO tasks (id, workspace_id, title, created_at, updated_at) + VALUES ('task-j2', 'ws-j', 'orphan', ?, ?) + `), now, now); err != nil { + t.Fatalf("seed orphan task: %v", err) + } + if _, err := repo.db.Exec(repo.db.Rebind(` + INSERT INTO task_sessions (id, task_id, state, started_at, updated_at) + VALUES ('sess-j2', 'task-j2', 'RUNNING', ?, ?) + `), now, now); err != nil { + t.Fatalf("seed orphan session: %v", err) + } + + count, err := repo.CountActiveTaskSessionsByRepository(ctx, "repo-j") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if count != 1 { + t.Errorf("expected only the linked session to be counted, got %d", count) + } +} diff --git a/apps/backend/internal/task/service/service_resources.go b/apps/backend/internal/task/service/service_resources.go index b5456ceb35..cc68a358b7 100644 --- a/apps/backend/internal/task/service/service_resources.go +++ b/apps/backend/internal/task/service/service_resources.go @@ -441,6 +441,18 @@ func (s *Service) ListRepositories(ctx context.Context, workspaceID string) ([]* return s.repoEntities.ListRepositories(ctx, workspaceID) } +// CountActiveSessionsByRepository returns the number of agent sessions in an +// active state (CREATED / STARTING / RUNNING / WAITING_FOR_INPUT) that are +// attached to the given repository. Used by the UI to warn users before they +// attempt to delete a repository that would otherwise be blocked by +// DeleteRepository's ErrActiveTaskSessions sentinel. +func (s *Service) CountActiveSessionsByRepository(ctx context.Context, id string) (int, error) { + if _, err := s.repoEntities.GetRepository(ctx, id); err != nil { + return 0, err + } + return s.sessions.CountActiveTaskSessionsByRepository(ctx, id) +} + // Repository script operations func (s *Service) CreateRepositoryScript(ctx context.Context, req *CreateRepositoryScriptRequest) (*models.RepositoryScript, error) { diff --git a/apps/web/app/actions/workspaces.ts b/apps/web/app/actions/workspaces.ts index 992b8e70d4..bcfca9b3cc 100644 --- a/apps/web/app/actions/workspaces.ts +++ b/apps/web/app/actions/workspaces.ts @@ -36,13 +36,22 @@ async function fetchJson(url: string, options?: RequestInit): Promise { ...(options?.headers ?? {}), }, }); + const text = response.status === 204 ? "" : await response.text(); if (!response.ok) { - throw new Error(`Request failed: ${response.status} ${response.statusText}`); + let message = `Request failed: ${response.status} ${response.statusText}`; + if (text) { + try { + const body = JSON.parse(text) as { error?: string; message?: string }; + const detail = body.error ?? body.message; + if (detail) { + message = detail; + } + } catch { + // body was not JSON, fall back to status text + } + } + throw new Error(message); } - if (response.status === 204) { - return undefined as T; - } - const text = await response.text(); if (!text) { return undefined as T; } @@ -500,6 +509,14 @@ export async function getWorkflowTaskCount(workflowId: string): Promise<{ task_c ); } +export async function getRepositoryActiveSessionCountAction( + repositoryId: string, +): Promise<{ active_session_count: number }> { + return fetchJson<{ active_session_count: number }>( + `${apiBaseUrl}/api/v1/repositories/${repositoryId}/active-session-count`, + ); +} + export async function getStepTaskCount(stepId: string): Promise<{ task_count: number }> { return fetchJson<{ task_count: number }>( `${apiBaseUrl}/api/v1/workflow/steps/${stepId}/task-count`, diff --git a/apps/web/components/diff/diff-error-boundary.test.tsx b/apps/web/components/diff/diff-error-boundary.test.tsx new file mode 100644 index 0000000000..ba39b2bb67 --- /dev/null +++ b/apps/web/components/diff/diff-error-boundary.test.tsx @@ -0,0 +1,54 @@ +import { describe, it, expect, vi, afterEach, beforeEach } from "vitest"; +import { render, screen, cleanup } from "@testing-library/react"; + +import { DiffErrorBoundary } from "./diff-error-boundary"; + +function Boom({ message = "boom" }: { message?: string }): never { + throw new Error(message); +} + +afterEach(() => { + cleanup(); + vi.restoreAllMocks(); +}); + +beforeEach(() => { + // React logs caught errors to console.error — silence so test output stays clean. + vi.spyOn(console, "error").mockImplementation(() => undefined); +}); + +describe("DiffErrorBoundary", () => { + it("renders children when no error is thrown", () => { + render( + +
child content
+ , + ); + expect(screen.getByTestId("child")).toBeTruthy(); + }); + + it("renders fallback when a child throws during render", () => { + render( + + + , + ); + expect(screen.getByText(/Unable to render diff/i)).toBeTruthy(); + }); + + it("resets fallback when filePath changes", () => { + const { rerender } = render( + + + , + ); + expect(screen.getByText(/Unable to render diff/i)).toBeTruthy(); + + rerender( + +
recovered
+ , + ); + expect(screen.getByTestId("child")).toBeTruthy(); + }); +}); diff --git a/apps/web/components/diff/diff-error-boundary.tsx b/apps/web/components/diff/diff-error-boundary.tsx new file mode 100644 index 0000000000..30107dde3b --- /dev/null +++ b/apps/web/components/diff/diff-error-boundary.tsx @@ -0,0 +1,52 @@ +"use client"; + +import { Component, type ErrorInfo, type ReactNode } from "react"; +import { IconAlertTriangle } from "@tabler/icons-react"; + +type DiffErrorBoundaryProps = { + filePath: string; + children: ReactNode; +}; + +type DiffErrorBoundaryState = { + error: Error | null; +}; + +/** + * Catches throws from @pierre/diffs renderers so a single malformed diff + * (e.g. stale cached hunk header vs live working-tree content for an + * untracked file that was edited after the snapshot) cannot tear down + * the whole review list. Renders a small per-file fallback instead. + */ +export class DiffErrorBoundary extends Component { + state: DiffErrorBoundaryState = { error: null }; + + static getDerivedStateFromError(error: Error): DiffErrorBoundaryState { + return { error }; + } + + componentDidCatch(error: Error, info: ErrorInfo): void { + console.error("[DiffErrorBoundary]", this.props.filePath, error, info.componentStack); + } + + componentDidUpdate(prevProps: DiffErrorBoundaryProps): void { + if (prevProps.filePath !== this.props.filePath && this.state.error) { + this.setState({ error: null }); + } + } + + render(): ReactNode { + if (this.state.error) { + return ( +
+ + + Unable to render diff for this file. The cached diff may be out of sync with the working + tree — refresh to retry. + +
+ ); + } + return this.props.children; + } +} diff --git a/apps/web/components/diff/index.ts b/apps/web/components/diff/index.ts index d5b95c3367..41da86ead5 100644 --- a/apps/web/components/diff/index.ts +++ b/apps/web/components/diff/index.ts @@ -2,6 +2,7 @@ export { DiffViewerResolved as DiffViewer } from "./diff-viewer-resolver"; export { DiffViewInlineResolved as DiffViewInline } from "./diff-viewer-resolver"; export { FileDiffViewer } from "./file-diff-viewer"; +export { DiffErrorBoundary } from "./diff-error-boundary"; export type { RevertBlockInfo } from "./diff-viewer-resolver"; export { CommentForm } from "./comment-form"; export { CommentDisplay } from "./comment-display"; diff --git a/apps/web/components/quick-chat/quick-chat-content.tsx b/apps/web/components/quick-chat/quick-chat-content.tsx index 2e166b48e3..ec70a510ed 100644 --- a/apps/web/components/quick-chat/quick-chat-content.tsx +++ b/apps/web/components/quick-chat/quick-chat-content.tsx @@ -106,7 +106,7 @@ export const QuickChatContent = memo(function QuickChatContent({
({ FileDiffViewer: ({ filePath }: { filePath: string }) => (
{filePath}
), + DiffErrorBoundary: ({ children }: { children: ReactNode }) => <>{children}, })); vi.mock("@/components/editors/file-actions-dropdown", () => ({ diff --git a/apps/web/components/review/review-diff-list.tsx b/apps/web/components/review/review-diff-list.tsx index f051bd321f..91fafeb515 100644 --- a/apps/web/components/review/review-diff-list.tsx +++ b/apps/web/components/review/review-diff-list.tsx @@ -3,7 +3,7 @@ import { memo, useEffect, useMemo, useRef, useState, useCallback } from "react"; import { IconAlertTriangle, IconChevronDown, IconChevronRight } from "@tabler/icons-react"; import { Checkbox } from "@kandev/ui/checkbox"; -import { FileDiffViewer } from "@/components/diff"; +import { FileDiffViewer, DiffErrorBoundary } from "@/components/diff"; import type { RevertBlockInfo } from "@/components/diff"; import { getWebSocketClient } from "@/lib/ws/connection"; import { requestFileContent, updateFileContent } from "@/lib/ws/workspace-files"; @@ -375,26 +375,35 @@ function renderDiffContent(opts: { // both PR and local changes, the wrong content gets paired with the PR // patch and @pierre/diffs 1.1.x renders nothing/errors. Disable expansion // for PR-sourced rows; uncommitted/committed rows still get it. - const enableExpansion = file.source !== "pr"; + // + // Also disable expansion for untracked files: the backend synthesizes a + // single all-additions hunk against /dev/null, so there is no real + // context to expand. Re-parsing with the live working-tree content + // races with concurrent edits — if the file shrinks after the snapshot, + // the cached hunk header advertises more lines than the fetched content + // has and @pierre/diffs' DiffHunksRenderer throws. + const enableExpansion = file.source !== "pr" && file.status !== "untracked"; return ( <> - + + + {file.diff_skip_reason === "truncated" && (
Diff truncated — showing first 256 KB diff --git a/apps/web/components/settings/repository-card.tsx b/apps/web/components/settings/repository-card.tsx index e323287ce6..8a46eb5de2 100644 --- a/apps/web/components/settings/repository-card.tsx +++ b/apps/web/components/settings/repository-card.tsx @@ -9,18 +9,12 @@ import { Checkbox } from "@kandev/ui/checkbox"; import { Input } from "@kandev/ui/input"; import { Label } from "@kandev/ui/label"; import { Textarea } from "@kandev/ui/textarea"; -import { - Dialog, - DialogContent, - DialogDescription, - DialogFooter, - DialogHeader, - DialogTitle, -} from "@kandev/ui/dialog"; import { useRequest } from "@/lib/http/use-request"; import { useToast } from "@/components/toast-provider"; import { UnsavedChangesBadge, UnsavedSaveButton } from "@/components/settings/unsaved-indicator"; import { EditableCard } from "@/components/settings/editable-card"; +import { DeleteRepositoryDialog } from "@/components/settings/repository-delete-dialog"; +import { getRepositoryActiveSessionCountAction } from "@/app/actions/workspaces"; import type { Repository, RepositoryScript } from "@/lib/types/http"; type RepositoryWithScripts = Repository & { scripts: RepositoryScript[] }; @@ -474,35 +468,6 @@ function RepositoryPreview({ ); } -type DeleteRepositoryDialogProps = { - open: boolean; - onOpenChange: (open: boolean) => void; - onDelete: () => void; -}; - -function DeleteRepositoryDialog({ open, onOpenChange, onDelete }: DeleteRepositoryDialogProps) { - return ( - - - - Delete repository - - This will remove the repository and its scripts. This action cannot be undone. - - - - - - - - - ); -} - type RepositoryCardProps = { repository: RepositoryWithScripts; isRepositoryDirty: boolean; @@ -516,6 +481,69 @@ type RepositoryCardProps = { onDelete: (repoId: string) => Promise | void; }; +function useRepositoryDelete( + repositoryId: string, + onDelete: (repoId: string) => Promise | void, + onDeleted: () => void, +) { + const { toast } = useToast(); + const [deleteOpen, setDeleteOpen] = useState(false); + const [activeSessionCount, setActiveSessionCount] = useState(0); + const [checkingCount, setCheckingCount] = useState(false); + const deleteRequest = useRequest(async () => { + await onDelete(repositoryId); + }); + + const handleOpenDelete = async () => { + // Reset count up-front so a stale value from a previous open can't flash + // the destructive button between dialog mount and the async fetch + // resolving with the fresh count. + setActiveSessionCount(0); + if (repositoryId.startsWith("temp-repo-")) { + setDeleteOpen(true); + return; + } + setCheckingCount(true); + try { + const { active_session_count } = await getRepositoryActiveSessionCountAction(repositoryId); + setActiveSessionCount(active_session_count); + setDeleteOpen(true); + } catch (error) { + toast({ + title: "Failed to check repository sessions", + description: error instanceof Error ? error.message : "Request failed", + variant: "error", + }); + } finally { + setCheckingCount(false); + } + }; + + const handleDelete = async () => { + try { + await deleteRequest.run(); + setDeleteOpen(false); + onDeleted(); + } catch (error) { + toast({ + title: "Failed to delete repository", + description: error instanceof Error ? error.message : "Request failed", + variant: "error", + }); + } + }; + + return { + deleteOpen, + setDeleteOpen, + activeSessionCount, + handleOpenDelete, + handleDelete, + buttonLoading: deleteRequest.isLoading || checkingCount, + dialogDeleteLoading: deleteRequest.isLoading, + }; +} + export function RepositoryCard({ repository, isRepositoryDirty, @@ -529,13 +557,10 @@ export function RepositoryCard({ onDelete, }: RepositoryCardProps) { const { toast } = useToast(); - const [deleteOpen, setDeleteOpen] = useState(false); const [isEditing, setIsEditing] = useState(() => autoOpen); const saveRequest = useRequest(() => onSave(repository.id)); - const deleteRequest = useRequest(async () => { - await onDelete(repository.id); - }); const isDirty = isRepositoryDirty || areScriptsDirty; + const deleteState = useRepositoryDelete(repository.id, onDelete, () => setIsEditing(false)); const handleSave = async (close: () => void) => { try { @@ -550,20 +575,6 @@ export function RepositoryCard({ } }; - const handleDelete = async () => { - try { - await deleteRequest.run(); - setDeleteOpen(false); - setIsEditing(false); - } catch (error) { - toast({ - title: "Failed to delete repository", - description: error instanceof Error ? error.message : "Request failed", - variant: "error", - }); - } - }; - return ( <> setDeleteOpen(true)} - deleteLoading={deleteRequest.isLoading} + onOpenDelete={deleteState.handleOpenDelete} + deleteLoading={deleteState.buttonLoading} close={close} /> )} @@ -591,16 +602,18 @@ export function RepositoryCard({ setDeleteOpen(true)} + deleteLoading={deleteState.buttonLoading} + onOpenDelete={deleteState.handleOpenDelete} open={open} /> )} /> ); diff --git a/apps/web/components/settings/repository-delete-dialog.tsx b/apps/web/components/settings/repository-delete-dialog.tsx new file mode 100644 index 0000000000..4ac4494fd4 --- /dev/null +++ b/apps/web/components/settings/repository-delete-dialog.tsx @@ -0,0 +1,66 @@ +"use client"; + +import { Button } from "@kandev/ui/button"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@kandev/ui/dialog"; + +type DeleteRepositoryDialogProps = { + open: boolean; + onOpenChange: (open: boolean) => void; + onDelete: () => void; + activeSessionCount: number; + deleteLoading: boolean; +}; + +export function DeleteRepositoryDialog({ + open, + onOpenChange, + onDelete, + activeSessionCount, + deleteLoading, +}: DeleteRepositoryDialogProps) { + const hasActiveSessions = activeSessionCount > 0; + const isOne = activeSessionCount === 1; + const sessionWord = isOne ? "session" : "sessions"; + const pronoun = isOne ? "it" : "them"; + const description = hasActiveSessions + ? `This repository is used by ${activeSessionCount} active agent ${sessionWord}. Stop or finish ${pronoun} before deleting the repository.` + : "This will remove the repository and its scripts. This action cannot be undone."; + return ( + + + + Delete repository + {description} + + + + {!hasActiveSessions && ( + + )} + + + + ); +} diff --git a/apps/web/components/task/chat/clarification-input-overlay.tsx b/apps/web/components/task/chat/clarification-input-overlay.tsx index 34d63af7fc..4d4777e787 100644 --- a/apps/web/components/task/chat/clarification-input-overlay.tsx +++ b/apps/web/components/task/chat/clarification-input-overlay.tsx @@ -1,7 +1,8 @@ "use client"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; -import { IconX, IconMessageQuestion, IconInfoCircle } from "@tabler/icons-react"; +import { IconX, IconMessageQuestion, IconInfoCircle, IconCheck } from "@tabler/icons-react"; +import { cn } from "@/lib/utils"; import ReactMarkdown from "react-markdown"; import { markdownComponents, remarkPlugins } from "@/components/shared/markdown-components"; import type { @@ -239,6 +240,9 @@ type CarouselBodyProps = { setActiveIndex: (idx: number) => void; customDrafts: Record; setCustomDrafts: React.Dispatch>>; + allAnswered: boolean; + isSubmitting: boolean; + onSubmit: () => void; }; type QuestionHandlerCtx = { @@ -340,6 +344,9 @@ function ClarificationCarouselBody({ setActiveIndex, customDrafts, setCustomDrafts, + allAnswered, + isSubmitting, + onSubmit, }: CarouselBodyProps) { const total = sortedMessages.length; const activeMessage = sortedMessages[Math.min(activeIndex, total - 1)] ?? null; @@ -347,22 +354,8 @@ function ClarificationCarouselBody({ const showAgentDisconnectedAtTop = sortedMessages.some( (m) => (m.metadata as ClarificationRequestMetadata | undefined)?.agent_disconnected === true, ); - const isSubmitting = group.submitState === "submitting"; const isSingleQuestion = total === 1; - const allAnswered = sortedMessages.every((m) => { - const id = readSingleQuestionMeta(m)?.questionId; - return id ? Boolean(group.answers[id]) : false; - }); - - // group is a fresh object every render, but its submitCollected callback is - // memoised by the hook — depend on the function only so this useCallback - // doesn't churn on every keystroke (via the live-record path). - const submitCollected = group.submitCollected; - const handleSubmit = useCallback(() => { - if (allAnswered) void submitCollected(); - }, [allAnswered, submitCollected]); - if (!meta) return null; const { selectedOption, customCommittedText, draft, customActive } = deriveSelectionState( @@ -396,7 +389,7 @@ function ClarificationCarouselBody({ onSelectOption={onSelectOption} onCustomDraftChange={onCustomDraftChange} onSubmitCustom={onSubmitCustom} - onRequestFinalSubmit={handleSubmit} + onRequestFinalSubmit={onSubmit} /> {!isSingleQuestion && ( setActiveIndex(Math.max(0, activeIndex - 1))} onNext={() => setActiveIndex(Math.min(total - 1, activeIndex + 1))} - onSubmit={handleSubmit} - canSubmit={allAnswered} /> )} setActiveIndex(Math.max(0, activeIndex - 1))} onNext={() => setActiveIndex(Math.min(total - 1, activeIndex + 1))} onSkip={() => void group.skipAll("User skipped")} - onSubmit={handleSubmit} + onSubmit={onSubmit} /> ); @@ -442,6 +433,20 @@ export function ClarificationInputOverlay({ useResolveCallback(group.submitState, onResolved); + // group is a fresh object every render, but its submitCollected callback is + // memoised by the hook — depend on the function only so this useCallback + // doesn't churn on every keystroke (via the live-record path). + const submitCollected = group.submitCollected; + const allAnswered = + sortedMessages.length > 0 && + sortedMessages.every((m) => { + const id = readSingleQuestionMeta(m)?.questionId; + return id ? Boolean(group.answers[id]) : false; + }); + const handleSubmit = useCallback(() => { + if (allAnswered) void submitCollected(); + }, [allAnswered, submitCollected]); + if (sortedMessages.length === 0) return null; const isSubmitting = group.submitState === "submitting"; @@ -475,16 +480,35 @@ export function ClarificationInputOverlay({ )}
- +
+ {total > 1 && ( + + )} + +
); diff --git a/apps/web/components/task/chat/clarification-overlay-parts.tsx b/apps/web/components/task/chat/clarification-overlay-parts.tsx index 7a3a7ce3fd..e7cfbc55ae 100644 --- a/apps/web/components/task/chat/clarification-overlay-parts.tsx +++ b/apps/web/components/task/chat/clarification-overlay-parts.tsx @@ -226,18 +226,17 @@ type CarouselNavProps = { isSubmitting: boolean; onPrev: () => void; onNext: () => void; - onSubmit: () => void; - canSubmit: boolean; }; +// Back/Next carousel nav. The final-submit affordance lives in the overlay +// header so it stays visible even when the question card scrolls past the +// fold; this nav only handles per-question navigation. export function ClarificationCarouselNav({ activeIndex, total, isSubmitting, onPrev, onNext, - onSubmit, - canSubmit, }: CarouselNavProps) { const isFirst = activeIndex === 0; const isLast = activeIndex === total - 1; @@ -258,34 +257,21 @@ export function ClarificationCarouselNav({ Back - {isLast ? ( - - ) : ( - - )} + ); } diff --git a/apps/web/components/task/task-chat-panel.tsx b/apps/web/components/task/task-chat-panel.tsx index 4a69e21fea..5e5d65136e 100644 --- a/apps/web/components/task/task-chat-panel.tsx +++ b/apps/web/components/task/task-chat-panel.tsx @@ -233,7 +233,7 @@ export const TaskChatPanel = memo(function TaskChatPanel({
{ }; }); - expect(initial.overflowY).toBe("auto"); + // overflow-y: scroll reserves a permanent scrollbar gutter so users + // discover scrollable content (and the in-overlay Submit) without having + // to wait for macOS auto-hide scrollbars to fade in on hover. + expect(initial.overflowY).toBe("scroll"); // Default state: no inline height → container sizes to its content. expect(initial.inlineHeight).toBe(""); // Sanity check: content-sized overlay is at least tall enough for the diff --git a/apps/web/e2e/tests/settings/repository-delete.spec.ts b/apps/web/e2e/tests/settings/repository-delete.spec.ts new file mode 100644 index 0000000000..fcbc4a5039 --- /dev/null +++ b/apps/web/e2e/tests/settings/repository-delete.spec.ts @@ -0,0 +1,151 @@ +import path from "node:path"; +import fs from "node:fs"; +import { execSync } from "node:child_process"; +import { test, expect } from "../../fixtures/test-base"; +import { makeGitEnv } from "../../helpers/git-helper"; + +/** + * Covers the pre-deletion guard on repositories. + * + * The card calls `GET /api/v1/repositories/:id/active-session-count` when the + * user clicks Delete. When the count is zero the dialog offers the destructive + * action; when the count is positive the destructive button is hidden and the + * dialog explains the user must stop the active sessions first. This is the + * UX parity with the workflow-delete flow. + */ +test.describe("Repository deletion", () => { + async function makeRepo(opts: { + apiClient: import("../../helpers/api-client").ApiClient; + backendTmpDir: string; + workspaceId: string; + suffix: string; + }): Promise<{ id: string; name: string }> { + const repoName = `E2E Repo Delete ${opts.suffix}`; + const repoDir = path.join(opts.backendTmpDir, "repos", `e2e-repo-delete-${opts.suffix}`); + fs.mkdirSync(repoDir, { recursive: true }); + const env = makeGitEnv(opts.backendTmpDir); + execSync("git init -b main", { cwd: repoDir, env }); + execSync('git commit --allow-empty -m "init"', { cwd: repoDir, env }); + const repo = await opts.apiClient.createRepository(opts.workspaceId, repoDir, "main", { + name: repoName, + }); + return { id: repo.id, name: repoName }; + } + + test("removes the card after confirm when no active sessions reference the repository", async ({ + testPage, + apiClient, + backend, + seedData, + }) => { + test.setTimeout(60_000); + + // testPage runs e2eReset before every test/retry, so a static suffix is + // safe and keeps test output deterministic. + const suffix = "ok"; + const repo = await makeRepo({ + apiClient, + backendTmpDir: backend.tmpDir, + workspaceId: seedData.workspaceId, + suffix, + }); + + await testPage.goto(`/settings/workspace/${seedData.workspaceId}/repositories`); + + const card = testPage.locator('[data-slot="card"]', { hasText: repo.name }); + await expect(card).toBeVisible({ timeout: 15_000 }); + + await card.getByRole("button", { name: "Delete", exact: true }).click(); + + const dialog = testPage.getByRole("dialog"); + await expect(dialog).toBeVisible({ timeout: 10_000 }); + await expect(dialog.getByText(/This action cannot be undone/i)).toBeVisible(); + await expect(dialog.getByRole("button", { name: "Delete Repository" })).toBeVisible(); + + await dialog.getByRole("button", { name: "Delete Repository" }).click(); + + await expect(card).toBeHidden({ timeout: 10_000 }); + await expect(dialog).toBeHidden({ timeout: 10_000 }); + }); + + test("blocks deletion and hides destructive button when an active session uses the repository", async ({ + testPage, + apiClient, + backend, + seedData, + }) => { + test.setTimeout(90_000); + + const suffix = "blocked"; + const repo = await makeRepo({ + apiClient, + backendTmpDir: backend.tmpDir, + workspaceId: seedData.workspaceId, + suffix, + }); + + // Long delay keeps the mock-agent in RUNNING for the whole test so the + // CountActiveTaskSessionsByRepository query is guaranteed to return 1. + // e2eReset on the next test tears the task down via DeleteTask, which + // shuts the agentctl instance via the cleanup goroutine. + await apiClient.createTaskWithAgent( + seedData.workspaceId, + `Repo Delete Blocker ${suffix}`, + seedData.agentProfileId, + { + description: 'e2e:delay(60000)\ne2e:message("done")', + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [repo.id], + }, + ); + + // Wait until the count endpoint actually reports the session as active — + // there is a small window between task create, session insert, and the + // join-against-task_repositories query observing the row. Without this + // poll the click can race the SQL and the dialog opens in the + // no-active-sessions branch. + await expect + .poll( + async () => { + const res = await apiClient.rawRequest( + "GET", + `/api/v1/repositories/${repo.id}/active-session-count`, + ); + if (!res.ok) return -1; + const body = (await res.json()) as { active_session_count: number }; + return body.active_session_count; + }, + { timeout: 15_000, intervals: [200, 500, 1_000] }, + ) + .toBeGreaterThan(0); + + await testPage.goto(`/settings/workspace/${seedData.workspaceId}/repositories`); + + const card = testPage.locator('[data-slot="card"]', { hasText: repo.name }); + await expect(card).toBeVisible({ timeout: 15_000 }); + + await card.getByRole("button", { name: "Delete", exact: true }).click(); + + const dialog = testPage.getByRole("dialog"); + await expect(dialog).toBeVisible({ timeout: 10_000 }); + await expect(dialog.getByText(/active agent session/i)).toBeVisible(); + + // Destructive button is gated behind hasActiveSessions === false, so the + // count > 0 path must not render it. + await expect(dialog.getByRole("button", { name: "Delete Repository" })).toHaveCount(0); + + // Radix DialogContent renders its own X close icon also labelled "Close"; + // scope to the footer slot so the locator resolves strictly to the visible + // footer button instead of silently disambiguating with .first(). + const closeButton = dialog + .locator('[data-slot="dialog-footer"]') + .getByRole("button", { name: "Close", exact: true }); + await expect(closeButton).toBeVisible(); + await closeButton.click(); + await expect(dialog).toBeHidden({ timeout: 5_000 }); + + // The card stays — the repository was not deleted. + await expect(card).toBeVisible(); + }); +});