From 5b2347832350a659c06ca99ecfe367d00d314474 Mon Sep 17 00:00:00 2001 From: Salim Belkhir <90771306+Salim-belkhir@users.noreply.github.com> Date: Wed, 13 May 2026 19:04:37 +0200 Subject: [PATCH] feat(web): differentiate pending permission icon from turn finished in sidebar (#882) --- .../session-task-switcher-sheet-hooks.ts | 9 +++- apps/web/components/task/task-item.test.tsx | 20 +++++++ apps/web/components/task/task-item.tsx | 16 +++++- .../components/task/task-session-sidebar.tsx | 11 +++- apps/web/components/task/task-switcher.tsx | 2 + apps/web/lib/ui/state-icons.tsx | 4 ++ .../lib/utils/pending-clarification.test.ts | 54 ++++++++++++++++++- apps/web/lib/utils/pending-clarification.ts | 24 +++++++++ 8 files changed, 136 insertions(+), 4 deletions(-) diff --git a/apps/web/components/task/mobile/session-task-switcher-sheet-hooks.ts b/apps/web/components/task/mobile/session-task-switcher-sheet-hooks.ts index 325c23b7de..e2427caa98 100644 --- a/apps/web/components/task/mobile/session-task-switcher-sheet-hooks.ts +++ b/apps/web/components/task/mobile/session-task-switcher-sheet-hooks.ts @@ -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, + ), }; }); }, [ diff --git a/apps/web/components/task/task-item.test.tsx b/apps/web/components/task/task-item.test.tsx index 5b1264da2b..a648d44ff8 100644 --- a/apps/web/components/task/task-item.test.tsx +++ b/apps/web/components/task/task-item.test.tsx @@ -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" }); diff --git a/apps/web/components/task/task-item.tsx b/apps/web/components/task/task-item.tsx index 7340c8743a..4faaea4c7c 100644 --- a/apps/web/components/task/task-item.tsx +++ b/apps/web/components/task/task-item.tsx @@ -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 ( + + ); + } if (isInProgress) { return ( ): SidebarI remoteExecutorName: undefined, primarySessionId: null, hasPendingClarification: false, + hasPendingPermission: false, updatedAt: s.archivedTaskUpdatedAt, createdAt: undefined, isArchived: true, diff --git a/apps/web/components/task/task-switcher.tsx b/apps/web/components/task/task-switcher.tsx index 837463508e..b0c75b83a9 100644 --- a/apps/web/components/task/task-switcher.tsx +++ b/apps/web/components/task/task-switcher.tsx @@ -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} diff --git a/apps/web/lib/ui/state-icons.tsx b/apps/web/lib/ui/state-icons.tsx index c2994b6828..7740c3ce31 100644 --- a/apps/web/lib/ui/state-icons.tsx +++ b/apps/web/lib/ui/state-icons.tsx @@ -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; diff --git a/apps/web/lib/utils/pending-clarification.test.ts b/apps/web/lib/utils/pending-clarification.test.ts index 95b180cfd5..6e5cccef48 100644 --- a/apps/web/lib/utils/pending-clarification.test.ts +++ b/apps/web/lib/utils/pending-clarification.test.ts @@ -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 { 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); + }); +}); diff --git a/apps/web/lib/utils/pending-clarification.ts b/apps/web/lib/utils/pending-clarification.ts index 129af13fad..b0c4869b2f 100644 --- a/apps/web/lib/utils/pending-clarification.ts +++ b/apps/web/lib/utils/pending-clarification.ts @@ -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, + sessionId?: string | null, +): boolean { + if (!sessionId) return false; + return hasPendingPermissionRequest(messagesBySession[sessionId]); +}