feat(web): differentiate pending permission icon from turn finished in sidebar (#882)
This commit is contained in:
@@ -10,7 +10,10 @@ import { useTasks } from "@/hooks/use-tasks";
|
||||
import { useTaskActions, useArchiveAndSwitchTask } from "@/hooks/use-task-actions";
|
||||
import { useTaskRemoval } from "@/hooks/use-task-removal";
|
||||
import { getSessionInfoForTask } from "@/lib/utils/session-info";
|
||||
import { hasPendingClarificationForSession } from "@/lib/utils/pending-clarification";
|
||||
import {
|
||||
hasPendingClarificationForSession,
|
||||
hasPendingPermissionForSession,
|
||||
} from "@/lib/utils/pending-clarification";
|
||||
import type {
|
||||
TaskState,
|
||||
TaskSessionState,
|
||||
@@ -128,6 +131,10 @@ export function useSheetData(workspaceId: string | null, workflowId: string | nu
|
||||
messagesBySession,
|
||||
task.primarySessionId,
|
||||
),
|
||||
hasPendingPermission: hasPendingPermissionForSession(
|
||||
messagesBySession,
|
||||
task.primarySessionId,
|
||||
),
|
||||
};
|
||||
});
|
||||
}, [
|
||||
|
||||
@@ -6,6 +6,7 @@ import { TaskItem } from "./task-item";
|
||||
|
||||
const REVIEW_ICON_TEST_ID = "task-state-review";
|
||||
const WAITING_FOR_INPUT_ICON_TEST_ID = "task-state-waiting-for-input";
|
||||
const PENDING_PERMISSION_ICON_TEST_ID = "task-state-pending-permission";
|
||||
|
||||
afterEach(() => cleanup());
|
||||
|
||||
@@ -39,6 +40,25 @@ describe("TaskItem status icon", () => {
|
||||
expect(screen.queryByTestId(REVIEW_ICON_TEST_ID)).toBeNull();
|
||||
});
|
||||
|
||||
it("shows alert icon when a permission request is pending", () => {
|
||||
renderTaskItem({ sessionState: "WAITING_FOR_INPUT", hasPendingPermission: true });
|
||||
|
||||
expect(screen.queryByTestId(PENDING_PERMISSION_ICON_TEST_ID)).not.toBeNull();
|
||||
expect(screen.queryByTestId(REVIEW_ICON_TEST_ID)).toBeNull();
|
||||
expect(screen.queryByTestId(WAITING_FOR_INPUT_ICON_TEST_ID)).toBeNull();
|
||||
});
|
||||
|
||||
it("prefers clarification icon over permission icon when both are pending", () => {
|
||||
renderTaskItem({
|
||||
sessionState: "WAITING_FOR_INPUT",
|
||||
hasPendingClarification: true,
|
||||
hasPendingPermission: true,
|
||||
});
|
||||
|
||||
expect(screen.queryByTestId(WAITING_FOR_INPUT_ICON_TEST_ID)).not.toBeNull();
|
||||
expect(screen.queryByTestId(PENDING_PERMISSION_ICON_TEST_ID)).toBeNull();
|
||||
});
|
||||
|
||||
it("keeps the review check for completed review tasks", () => {
|
||||
renderTaskItem({ sessionState: "COMPLETED" });
|
||||
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
IconGitPullRequest,
|
||||
IconMessageQuestion,
|
||||
IconPinFilled,
|
||||
IconShieldQuestion,
|
||||
} from "@tabler/icons-react";
|
||||
import { PRTaskIcon } from "@/components/github/pr-task-icon";
|
||||
import { IssueTaskIcon } from "@/components/github/issue-task-icon";
|
||||
@@ -18,7 +19,7 @@ import { DEBUG_UI } from "@/lib/config";
|
||||
import { useTaskColor } from "@/hooks/use-task-color";
|
||||
import { TASK_COLOR_BAR_CLASS, type TaskColor } from "@/lib/task-colors";
|
||||
import type { TaskState, TaskSessionState } from "@/lib/types/http";
|
||||
import { shouldUseQuestionTaskIcon } from "@/lib/ui/state-icons";
|
||||
import { shouldUseQuestionTaskIcon, shouldUsePermissionTaskIcon } from "@/lib/ui/state-icons";
|
||||
import type { SessionPollMode } from "@/lib/state/slices/session-runtime/types";
|
||||
import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip";
|
||||
import { RemoteCloudTooltip } from "./remote-cloud-tooltip";
|
||||
@@ -47,6 +48,7 @@ type TaskItemProps = {
|
||||
taskId?: string;
|
||||
primarySessionId?: string | null;
|
||||
hasPendingClarification?: boolean;
|
||||
hasPendingPermission?: boolean;
|
||||
parentTaskTitle?: string;
|
||||
isSubTask?: boolean;
|
||||
/** Number of subtasks under this parent task. Only set for parent rows. */
|
||||
@@ -97,11 +99,13 @@ function TaskStateIcon({
|
||||
state,
|
||||
isInProgress,
|
||||
hasPendingClarification,
|
||||
hasPendingPermission,
|
||||
}: {
|
||||
sessionState?: TaskSessionState;
|
||||
state?: TaskState;
|
||||
isInProgress: boolean;
|
||||
hasPendingClarification?: boolean;
|
||||
hasPendingPermission?: boolean;
|
||||
}) {
|
||||
if (shouldUseQuestionTaskIcon(state, hasPendingClarification)) {
|
||||
return (
|
||||
@@ -111,6 +115,14 @@ function TaskStateIcon({
|
||||
/>
|
||||
);
|
||||
}
|
||||
if (shouldUsePermissionTaskIcon(hasPendingPermission)) {
|
||||
return (
|
||||
<IconShieldQuestion
|
||||
data-testid="task-state-pending-permission"
|
||||
className="mt-[1px] h-3.5 w-3.5 shrink-0 text-amber-500"
|
||||
/>
|
||||
);
|
||||
}
|
||||
if (isInProgress) {
|
||||
return (
|
||||
<IconCircleDashed
|
||||
@@ -301,6 +313,7 @@ export const TaskItem = memo(function TaskItem({
|
||||
taskId,
|
||||
primarySessionId,
|
||||
hasPendingClarification,
|
||||
hasPendingPermission,
|
||||
isSubTask,
|
||||
subtaskCount,
|
||||
subtasksCollapsed,
|
||||
@@ -341,6 +354,7 @@ export const TaskItem = memo(function TaskItem({
|
||||
state={state}
|
||||
isInProgress={isInProgress}
|
||||
hasPendingClarification={hasPendingClarification}
|
||||
hasPendingPermission={hasPendingPermission}
|
||||
/>
|
||||
<TaskItemContent
|
||||
title={title}
|
||||
|
||||
@@ -30,7 +30,10 @@ import { getWebSocketClient } from "@/lib/ws/connection";
|
||||
import { useArchivedTaskState } from "./task-archived-context";
|
||||
import { useRepositories } from "@/hooks/domains/workspace/use-repositories";
|
||||
import { useWorkspacePRs } from "@/hooks/domains/github/use-task-pr";
|
||||
import { hasPendingClarificationForSession } from "@/lib/utils/pending-clarification";
|
||||
import {
|
||||
hasPendingClarificationForSession,
|
||||
hasPendingPermissionForSession,
|
||||
} from "@/lib/utils/pending-clarification";
|
||||
|
||||
/**
|
||||
* Stabilize a derived array of primary session IDs so the reference only
|
||||
@@ -138,6 +141,10 @@ function toSidebarItem(
|
||||
ctx.messagesBySession,
|
||||
task.primarySessionId,
|
||||
);
|
||||
const hasPendingPermission = hasPendingPermissionForSession(
|
||||
ctx.messagesBySession,
|
||||
task.primarySessionId,
|
||||
);
|
||||
|
||||
const diffStats = resolveDiffStats(
|
||||
sessionInfo.diffStats,
|
||||
@@ -165,6 +172,7 @@ function toSidebarItem(
|
||||
remoteExecutorName: task.primaryExecutorName ?? undefined,
|
||||
primarySessionId: task.primarySessionId ?? null,
|
||||
hasPendingClarification: hasPendingClarificationRequest,
|
||||
hasPendingPermission,
|
||||
updatedAt: sessionInfo.updatedAt ?? task.updatedAt ?? task.createdAt,
|
||||
createdAt: task.createdAt,
|
||||
isArchived: false as boolean,
|
||||
@@ -209,6 +217,7 @@ function buildArchivedItem(s: ReturnType<typeof useArchivedTaskState>): SidebarI
|
||||
remoteExecutorName: undefined,
|
||||
primarySessionId: null,
|
||||
hasPendingClarification: false,
|
||||
hasPendingPermission: false,
|
||||
updatedAt: s.archivedTaskUpdatedAt,
|
||||
createdAt: undefined,
|
||||
isArchived: true,
|
||||
|
||||
@@ -47,6 +47,7 @@ export type TaskSwitcherItem = {
|
||||
isArchived?: boolean;
|
||||
primarySessionId?: string | null;
|
||||
hasPendingClarification?: boolean;
|
||||
hasPendingPermission?: boolean;
|
||||
parentTaskTitle?: string;
|
||||
parentTaskId?: string;
|
||||
prInfo?: { number: number; state: string };
|
||||
@@ -195,6 +196,7 @@ function TaskRow({
|
||||
taskId={task.id}
|
||||
primarySessionId={task.primarySessionId ?? null}
|
||||
hasPendingClarification={task.hasPendingClarification}
|
||||
hasPendingPermission={task.hasPendingPermission}
|
||||
updatedAt={task.updatedAt}
|
||||
repositories={task.repositories}
|
||||
prInfo={task.prInfo}
|
||||
|
||||
@@ -69,6 +69,10 @@ export function shouldUseQuestionTaskIcon(
|
||||
return isWaitingForInputState(state) || hasPendingClarification;
|
||||
}
|
||||
|
||||
export function shouldUsePermissionTaskIcon(hasPendingPermission = false): boolean {
|
||||
return hasPendingPermission;
|
||||
}
|
||||
|
||||
function getTaskStateIconConfig(state?: TaskState, hasPendingClarification = false): IconConfig {
|
||||
if (shouldUseQuestionTaskIcon(state, hasPendingClarification)) {
|
||||
return TASK_STATE_ICONS.WAITING_FOR_INPUT;
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { Message } from "@/lib/types/http";
|
||||
import { findPendingClarificationGroup, hasPendingClarification } from "./pending-clarification";
|
||||
import {
|
||||
findPendingClarificationGroup,
|
||||
hasPendingClarification,
|
||||
hasPendingPermissionRequest,
|
||||
} from "./pending-clarification";
|
||||
|
||||
function message(overrides: Partial<Message>): Message {
|
||||
return {
|
||||
@@ -132,3 +136,51 @@ describe("findPendingClarificationGroup", () => {
|
||||
expect(findPendingClarificationGroup([old, a]).map((m) => m.id)).toEqual(["a"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("hasPendingPermissionRequest", () => {
|
||||
it("detects permission requests with pending status", () => {
|
||||
expect(
|
||||
hasPendingPermissionRequest([
|
||||
message({ type: "permission_request", metadata: { status: "pending" } }),
|
||||
]),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("treats missing permission status as pending", () => {
|
||||
expect(hasPendingPermissionRequest([message({ type: "permission_request" })])).toBe(true);
|
||||
});
|
||||
|
||||
it("ignores approved permission requests", () => {
|
||||
expect(
|
||||
hasPendingPermissionRequest([
|
||||
message({ type: "permission_request", metadata: { status: "approved" } }),
|
||||
]),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("ignores rejected permission requests", () => {
|
||||
expect(
|
||||
hasPendingPermissionRequest([
|
||||
message({ type: "permission_request", metadata: { status: "rejected" } }),
|
||||
]),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("ignores expired permission requests", () => {
|
||||
expect(
|
||||
hasPendingPermissionRequest([
|
||||
message({ type: "permission_request", metadata: { status: "expired" } }),
|
||||
]),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for non-permission messages", () => {
|
||||
expect(hasPendingPermissionRequest([message({ type: "message" })])).toBe(false);
|
||||
});
|
||||
|
||||
it("returns false for empty or null input", () => {
|
||||
expect(hasPendingPermissionRequest([])).toBe(false);
|
||||
expect(hasPendingPermissionRequest(null)).toBe(false);
|
||||
expect(hasPendingPermissionRequest(undefined)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -54,3 +54,27 @@ export function hasPendingClarificationForSession(
|
||||
if (!sessionId) return false;
|
||||
return hasPendingClarification(messagesBySession[sessionId]);
|
||||
}
|
||||
|
||||
// --- Permission request helpers ---
|
||||
|
||||
export function isPendingPermissionMessage(message: Message): boolean {
|
||||
if (message.type !== "permission_request") return false;
|
||||
const metadata = message.metadata as { status?: string } | undefined;
|
||||
return !metadata?.status || metadata.status === "pending";
|
||||
}
|
||||
|
||||
export function hasPendingPermissionRequest(messages?: readonly Message[] | null): boolean {
|
||||
if (!messages) return false;
|
||||
for (let i = messages.length - 1; i >= 0; i--) {
|
||||
if (isPendingPermissionMessage(messages[i])) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
export function hasPendingPermissionForSession(
|
||||
messagesBySession: Record<string, readonly Message[] | undefined>,
|
||||
sessionId?: string | null,
|
||||
): boolean {
|
||||
if (!sessionId) return false;
|
||||
return hasPendingPermissionRequest(messagesBySession[sessionId]);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user