feat(repositories): guard delete behind active-session check + UI fixes (#968)

This commit is contained in:
Carlos Florêncio
2026-05-21 09:24:34 +01:00
committed by GitHub
parent 1f264adbc4
commit 2ca48fc7df
23 changed files with 695 additions and 151 deletions
@@ -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
}
@@ -335,3 +335,7 @@ type BulkMoveTasksResponse struct {
type TaskCountResponse struct {
TaskCount int `json:"task_count"`
}
type RepositoryActiveSessionCountResponse struct {
ActiveSessionCount int `json:"active_session_count"`
}
@@ -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
}
@@ -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 {
@@ -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)
@@ -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.
@@ -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)
}
}
@@ -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) {
+22 -5
View File
@@ -36,13 +36,22 @@ async function fetchJson<T>(url: string, options?: RequestInit): Promise<T> {
...(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`,
@@ -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(
<DiffErrorBoundary filePath="src/foo.ts">
<div data-testid="child">child content</div>
</DiffErrorBoundary>,
);
expect(screen.getByTestId("child")).toBeTruthy();
});
it("renders fallback when a child throws during render", () => {
render(
<DiffErrorBoundary filePath="src/foo.ts">
<Boom />
</DiffErrorBoundary>,
);
expect(screen.getByText(/Unable to render diff/i)).toBeTruthy();
});
it("resets fallback when filePath changes", () => {
const { rerender } = render(
<DiffErrorBoundary filePath="src/foo.ts">
<Boom />
</DiffErrorBoundary>,
);
expect(screen.getByText(/Unable to render diff/i)).toBeTruthy();
rerender(
<DiffErrorBoundary filePath="src/bar.ts">
<div data-testid="child">recovered</div>
</DiffErrorBoundary>,
);
expect(screen.getByTestId("child")).toBeTruthy();
});
});
@@ -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<DiffErrorBoundaryProps, DiffErrorBoundaryState> {
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 (
<div className="flex items-center gap-2 py-4 px-3 text-xs text-muted-foreground border-t">
<IconAlertTriangle className="h-4 w-4 text-amber-500 shrink-0" />
<span>
Unable to render diff for this file. The cached diff may be out of sync with the working
tree — refresh to retry.
</span>
</div>
);
}
return this.props.children;
}
}
+1
View File
@@ -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";
@@ -106,7 +106,7 @@ export const QuickChatContent = memo(function QuickChatContent({
<div
ref={clarificationContainerRef}
data-testid="clarification-overlay-container"
className="px-1 overflow-y-auto overscroll-contain max-h-[50vh]"
className="px-1 overflow-y-scroll overscroll-contain max-h-[50vh]"
style={clarificationHeight === null ? undefined : { height: clarificationHeight }}
>
<ClarificationInputOverlay
@@ -9,6 +9,7 @@ vi.mock("@/components/diff", () => ({
FileDiffViewer: ({ filePath }: { filePath: string }) => (
<div data-testid="diff-stub">{filePath}</div>
),
DiffErrorBoundary: ({ children }: { children: ReactNode }) => <>{children}</>,
}));
vi.mock("@/components/editors/file-actions-dropdown", () => ({
+28 -19
View File
@@ -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 (
<>
<FileDiffViewer
filePath={file.path}
diff={file.diff}
status={file.status}
enableComments
enableAcceptReject
onRevertBlock={onRevertBlock}
onCommentRun={onCommentRun}
sessionId={sessionId}
wordWrap={wordWrap}
enableExpansion={enableExpansion}
baseRef="HEAD"
hideHeader
expandUnchanged={expandUnchanged}
onToggleExpandUnchanged={onToggleExpandUnchanged}
repo={file.repository_name}
/>
<DiffErrorBoundary filePath={file.path}>
<FileDiffViewer
filePath={file.path}
diff={file.diff}
status={file.status}
enableComments
enableAcceptReject
onRevertBlock={onRevertBlock}
onCommentRun={onCommentRun}
sessionId={sessionId}
wordWrap={wordWrap}
enableExpansion={enableExpansion}
baseRef="HEAD"
hideHeader
expandUnchanged={expandUnchanged}
onToggleExpandUnchanged={onToggleExpandUnchanged}
repo={file.repository_name}
/>
</DiffErrorBoundary>
{file.diff_skip_reason === "truncated" && (
<div className="py-1 text-center text-xs text-muted-foreground border-t">
Diff truncated — showing first 256 KB
@@ -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 (
<Dialog open={open} onOpenChange={onOpenChange}>
<DialogContent>
<DialogHeader>
<DialogTitle>Delete repository</DialogTitle>
<DialogDescription>
This will remove the repository and its scripts. This action cannot be undone.
</DialogDescription>
</DialogHeader>
<DialogFooter>
<Button type="button" variant="outline" onClick={() => onOpenChange(false)}>
Cancel
</Button>
<Button type="button" variant="destructive" onClick={onDelete}>
Delete Repository
</Button>
</DialogFooter>
</DialogContent>
</Dialog>
);
}
type RepositoryCardProps = {
repository: RepositoryWithScripts;
isRepositoryDirty: boolean;
@@ -516,6 +481,69 @@ type RepositoryCardProps = {
onDelete: (repoId: string) => Promise<void> | void;
};
function useRepositoryDelete(
repositoryId: string,
onDelete: (repoId: string) => Promise<void> | 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 (
<>
<EditableCard
@@ -582,8 +593,8 @@ export function RepositoryCard({
onUpdateScript={onUpdateScript}
onDeleteScript={onDeleteScript}
onSave={handleSave}
onOpenDelete={() => setDeleteOpen(true)}
deleteLoading={deleteRequest.isLoading}
onOpenDelete={deleteState.handleOpenDelete}
deleteLoading={deleteState.buttonLoading}
close={close}
/>
)}
@@ -591,16 +602,18 @@ export function RepositoryCard({
<RepositoryPreview
repository={repository}
isDirty={isDirty}
deleteLoading={deleteRequest.isLoading}
onOpenDelete={() => setDeleteOpen(true)}
deleteLoading={deleteState.buttonLoading}
onOpenDelete={deleteState.handleOpenDelete}
open={open}
/>
)}
/>
<DeleteRepositoryDialog
open={deleteOpen}
onOpenChange={setDeleteOpen}
onDelete={handleDelete}
open={deleteState.deleteOpen}
onOpenChange={deleteState.setDeleteOpen}
onDelete={deleteState.handleDelete}
activeSessionCount={deleteState.activeSessionCount}
deleteLoading={deleteState.dialogDeleteLoading}
/>
</>
);
@@ -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 (
<Dialog open={open} onOpenChange={onOpenChange}>
<DialogContent>
<DialogHeader>
<DialogTitle>Delete repository</DialogTitle>
<DialogDescription>{description}</DialogDescription>
</DialogHeader>
<DialogFooter>
<Button
type="button"
variant="outline"
className="cursor-pointer"
onClick={() => onOpenChange(false)}
>
{hasActiveSessions ? "Close" : "Cancel"}
</Button>
{!hasActiveSessions && (
<Button
type="button"
variant="destructive"
className="cursor-pointer"
onClick={onDelete}
disabled={deleteLoading}
>
Delete Repository
</Button>
)}
</DialogFooter>
</DialogContent>
</Dialog>
);
}
@@ -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<string, string>;
setCustomDrafts: React.Dispatch<React.SetStateAction<Record<string, string>>>;
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 && (
<ClarificationCarouselNav
@@ -405,8 +398,6 @@ function ClarificationCarouselBody({
isSubmitting={isSubmitting}
onPrev={() => setActiveIndex(Math.max(0, activeIndex - 1))}
onNext={() => setActiveIndex(Math.min(total - 1, activeIndex + 1))}
onSubmit={handleSubmit}
canSubmit={allAnswered}
/>
)}
<CarouselKeyboardShortcuts
@@ -418,7 +409,7 @@ function ClarificationCarouselBody({
onPrev={() => 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({
</span>
)}
</div>
<button
type="button"
onClick={() => void group.skipAll("User skipped")}
disabled={isSubmitting}
className="text-muted-foreground hover:text-foreground cursor-pointer disabled:opacity-50"
data-testid="clarification-skip"
aria-label="Skip all questions"
>
<IconX className="h-4 w-4" />
</button>
<div className="flex items-center gap-2">
{total > 1 && (
<button
type="button"
onClick={handleSubmit}
disabled={!allAnswered || isSubmitting}
data-testid="clarification-submit"
className={cn(
"inline-flex items-center gap-1 text-xs px-3 py-1 rounded font-medium transition-colors",
allAnswered && !isSubmitting
? "bg-blue-500 text-white hover:bg-blue-500/90 cursor-pointer"
: "bg-muted text-muted-foreground cursor-not-allowed",
)}
>
{isSubmitting ? "Submitting…" : "Submit"}
<IconCheck className="h-3 w-3" />
</button>
)}
<button
type="button"
onClick={() => void group.skipAll("User skipped")}
disabled={isSubmitting}
className="text-muted-foreground hover:text-foreground cursor-pointer disabled:opacity-50"
data-testid="clarification-skip"
aria-label="Skip all questions"
>
<IconX className="h-4 w-4" />
</button>
</div>
</div>
<ClarificationCarouselBody
sortedMessages={sortedMessages}
@@ -493,6 +517,9 @@ export function ClarificationInputOverlay({
setActiveIndex={setActiveIndex}
customDrafts={customDrafts}
setCustomDrafts={setCustomDrafts}
allAnswered={allAnswered}
isSubmitting={isSubmitting}
onSubmit={handleSubmit}
/>
</div>
);
@@ -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({
<IconArrowLeft className="h-3 w-3" />
Back
</button>
{isLast ? (
<button
type="button"
onClick={onSubmit}
disabled={!canSubmit || isSubmitting}
data-testid="clarification-submit"
className={cn(
"inline-flex items-center gap-1 text-xs px-3 py-1 rounded font-medium",
canSubmit && !isSubmitting
? "bg-blue-500 text-white hover:bg-blue-500/90 cursor-pointer"
: "bg-muted text-muted-foreground cursor-not-allowed",
)}
>
{isSubmitting ? "Submitting…" : "Submit answers"}
<IconCheck className="h-3 w-3" />
</button>
) : (
<button
type="button"
onClick={onNext}
disabled={isSubmitting}
data-testid="clarification-next"
className="inline-flex items-center gap-1 text-xs px-2 py-1 rounded border border-border text-foreground/80 hover:bg-muted/50 cursor-pointer"
>
Next
<IconArrowRight className="h-3 w-3" />
</button>
)}
<button
type="button"
onClick={onNext}
disabled={isLast || isSubmitting}
data-testid="clarification-next"
className={cn(
"inline-flex items-center gap-1 text-xs px-2 py-1 rounded border",
isLast
? "border-transparent text-muted-foreground/40 cursor-not-allowed"
: "border-border text-foreground/80 hover:bg-muted/50 cursor-pointer",
)}
>
Next
<IconArrowRight className="h-3 w-3" />
</button>
</div>
);
}
+1 -1
View File
@@ -233,7 +233,7 @@ export const TaskChatPanel = memo(function TaskChatPanel({
<div
ref={clarificationContainerRef}
data-testid="clarification-overlay-container"
className="px-1 overflow-y-auto overscroll-contain max-h-[50vh]"
className="px-1 overflow-y-scroll overscroll-contain max-h-[50vh]"
style={clarificationHeight === null ? undefined : { height: clarificationHeight }}
>
<ClarificationInputOverlay
+1 -1
View File
@@ -363,7 +363,7 @@ export class SessionPage {
return this.clarificationOverlay().getByTestId("clarification-next");
}
/** Final "Submit answers" button (rendered only on the last step). */
/** Sticky "Submit" button in the overlay header (multi-question only). */
clarificationSubmit(): Locator {
return this.clarificationOverlay().getByTestId("clarification-submit");
}
@@ -121,7 +121,10 @@ test.describe("Clarification overlay resizable layout", () => {
};
});
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
@@ -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();
});
});