Self-review

Author: EhabY

packages/shared/src/tasks/api.ts

@@ -54,9 +54,9 @@ const sendTaskMessage = defineRequest<TaskIdParams & { message: string }, void>(
 	"sendTaskMessage",
 );
 
-const viewInCoder = defineCommand<{ taskId: string }>("viewInCoder");
-const viewLogs = defineCommand<{ taskId: string }>("viewLogs");
-const closeWorkspaceLogs = defineCommand("closeWorkspaceLogs");
+const viewInCoder = defineCommand<TaskIdParams>("viewInCoder");
+const viewLogs = defineCommand<TaskIdParams>("viewLogs");
+const closeWorkspaceLogs = defineCommand<void>("closeWorkspaceLogs");
 
 const taskUpdated = defineNotification<Task>("taskUpdated");
 const tasksUpdated = defineNotification<Task[]>("tasksUpdated");

packages/shared/src/tasks/types.ts

@@ -29,16 +29,18 @@ export interface TaskPreset {
 	isDefault: boolean;
 }
 
-/** Status of log fetching */
-export type LogsStatus = "ok" | "not_available" | "error";
+/** Result of fetching task logs: either logs or an error/unavailable state. */
+export type TaskLogs =
+	| { status: "ok"; logs: TaskLogEntry[] }
+	| { status: "not_available" }
+	| { status: "error" };
 
 /**
  * Full details for a selected task, including logs and action availability.
  */
 export interface TaskDetails extends TaskPermissions {
 	task: Task;
-	logs: TaskLogEntry[];
-	logsStatus: LogsStatus;
+	logs: TaskLogs;
 }
 
 export interface TaskPermissions {

packages/shared/src/tasks/utils.ts

@@ -4,11 +4,6 @@ export function getTaskLabel(task: Task): string {
 	return task.display_name || task.name || task.id;
 }
 
-/** Whether the agent is actively working (status is active and state is working). */
-export function isTaskWorking(task: Task): boolean {
-	return task.status === "active" && task.current_state?.state === "working";
-}
-
 const PAUSABLE_STATUSES: readonly TaskStatus[] = [
 	"active",
 	"initializing",
@@ -42,6 +37,11 @@ export function getTaskPermissions(task: Task): TaskPermissions {
 	};
 }
 
+/** Whether the agent is actively working (status is active and state is working). */
+export function isTaskWorking(task: Task): boolean {
+	return task.status === "active" && task.current_state?.state === "working";
+}
+
 /**
  * Task statuses where logs won't change (stable/terminal states).
  * "complete" is a TaskState (sub-state of active), checked separately.
@@ -64,7 +64,9 @@ export function isBuildingWorkspace(task: Task): boolean {
 
 /** Whether the workspace is running but the agent hasn't reached "ready" yet. */
 export function isAgentStarting(task: Task): boolean {
-	if (task.workspace_status !== "running") return false;
+	if (task.workspace_status !== "running") {
+		return false;
+	}
 	const lc = task.workspace_agent_lifecycle;
 	return lc === "created" || lc === "starting";
 }

packages/tasks/src/App.tsx

@@ -1,6 +1,5 @@
-import { TasksApi, type InitResponse } from "@repo/shared";
+import { type InitResponse } from "@repo/shared";
 import { getState, setState } from "@repo/webview-shared";
-import { useIpc } from "@repo/webview-shared/react";
 import {
 	VscodeCollapsible,
 	VscodeProgressRing,
@@ -17,6 +16,7 @@ import { TaskList } from "./components/TaskList";
 import { useCollapsibleToggle } from "./hooks/useCollapsibleToggle";
 import { useScrollableHeight } from "./hooks/useScrollableHeight";
 import { useSelectedTask } from "./hooks/useSelectedTask";
+import { useTasksApi } from "./hooks/useTasksApi";
 import { useTasksQuery } from "./hooks/useTasksQuery";
 
 interface PersistedState extends InitResponse {
@@ -46,10 +46,10 @@ export default function App() {
 	useScrollableHeight(createRef, createScrollRef);
 	useScrollableHeight(historyRef, historyScrollRef);
 
-	const { onNotification } = useIpc();
+	const { onShowCreateForm } = useTasksApi();
 	useEffect(() => {
-		return onNotification(TasksApi.showCreateForm, () => setCreateOpen(true));
-	}, [onNotification, setCreateOpen]);
+		return onShowCreateForm(() => setCreateOpen(true));
+	}, [onShowCreateForm, setCreateOpen]);
 
 	useEffect(() => {
 		if (data) {

packages/tasks/src/components/AgentChatHistory.tsx

@@ -1,10 +1,9 @@
 import { LogViewer, LogViewerPlaceholder } from "./LogViewer";
 
-import type { LogsStatus, TaskLogEntry } from "@repo/shared";
+import type { TaskLogEntry, TaskLogs } from "@repo/shared";
 
 interface AgentChatHistoryProps {
-	logs: TaskLogEntry[];
-	logsStatus: LogsStatus;
+	taskLogs: TaskLogs;
 	isThinking: boolean;
 }
 
@@ -28,38 +27,35 @@ function LogEntry({
 }
 
 export function AgentChatHistory({
-	logs,
-	logsStatus,
+	taskLogs,
 	isThinking,
 }: AgentChatHistoryProps) {
-	const isEmpty = logs.length === 0 && (logsStatus !== "ok" || !isThinking);
+	const logs = taskLogs.status === "ok" ? taskLogs.logs : [];
 
 	return (
 		<LogViewer header="Agent chat history">
-			{isEmpty ? (
-				<LogViewerPlaceholder error={logsStatus === "error"}>
-					{getEmptyMessage(logsStatus)}
+			{logs.length === 0 ? (
+				<LogViewerPlaceholder error={taskLogs.status === "error"}>
+					{getEmptyMessage(taskLogs.status)}
 				</LogViewerPlaceholder>
 			) : (
-				<>
-					{logs.map((log, index) => (
-						<LogEntry
-							key={log.id}
-							log={log}
-							isGroupStart={index === 0 || log.type !== logs[index - 1].type}
-						/>
-					))}
-					{isThinking && (
-						<div className="log-entry log-entry-thinking">Thinking...</div>
-					)}
-				</>
+				logs.map((log, index) => (
+					<LogEntry
+						key={log.id}
+						log={log}
+						isGroupStart={index === 0 || log.type !== logs[index - 1].type}
+					/>
+				))
+			)}
+			{isThinking && (
+				<div className="log-entry log-entry-thinking">Thinking...</div>
 			)}
 		</LogViewer>
 	);
 }
 
-function getEmptyMessage(logsStatus: LogsStatus): string {
-	switch (logsStatus) {
+function getEmptyMessage(status: TaskLogs["status"]): string {
+	switch (status) {
 		case "not_available":
 			return "Logs not available in current task state";
 		case "error":

packages/tasks/src/components/LogViewer.tsx

@@ -27,15 +27,11 @@ export function LogViewerPlaceholder({
 	children,
 	error,
 }: {
-	children: ReactNode;
+	children: string;
 	error?: boolean;
 }) {
 	return (
-		<div
-			className={
-				error ? "log-viewer-empty log-viewer-error" : "log-viewer-empty"
-			}
-		>
+		<div className={`log-viewer-empty${error ? " log-viewer-error" : ""}`}>
 			{children}
 		</div>
 	);

packages/tasks/src/components/TaskDetailView.tsx

@@ -4,8 +4,6 @@ import {
 	type TaskDetails,
 } from "@repo/shared";
 
-import { useWorkspaceLogs } from "../hooks/useWorkspaceLogs";
-
 import { AgentChatHistory } from "./AgentChatHistory";
 import { ErrorBanner } from "./ErrorBanner";
 import { TaskDetailHeader } from "./TaskDetailHeader";
@@ -18,24 +16,19 @@ interface TaskDetailViewProps {
 }
 
 export function TaskDetailView({ details, onBack }: TaskDetailViewProps) {
-	const { task, logs, logsStatus } = details;
+	const { task, logs } = details;
 
 	const starting = isWorkspaceStarting(task);
-	const workspaceLines = useWorkspaceLogs(starting);
 	const isThinking = isTaskWorking(task);
 
 	return (
 		<div className="task-detail-view">
 			<TaskDetailHeader task={task} onBack={onBack} />
 			{task.status === "error" && <ErrorBanner task={task} />}
 			{starting ? (
-				<WorkspaceLogs task={task} lines={workspaceLines} />
+				<WorkspaceLogs task={task} />
 			) : (
-				<AgentChatHistory
-					logs={logs}
-					logsStatus={logsStatus}
-					isThinking={isThinking}
-				/>
+				<AgentChatHistory taskLogs={logs} isThinking={isThinking} />
 			)}
 			<TaskMessageInput task={task} />
 		</div>

packages/tasks/src/components/WorkspaceLogs.tsx

@@ -1,14 +1,15 @@
 import { isBuildingWorkspace, type Task } from "@repo/shared";
 
+import { useWorkspaceLogs } from "../hooks/useWorkspaceLogs";
+
 import { LogViewer, LogViewerPlaceholder } from "./LogViewer";
 
-export function WorkspaceLogs({
-	task,
-	lines,
-}: {
-	task: Task;
-	lines: string[];
-}) {
+function LogLine({ children }: { children: string }) {
+	return <div className="log-entry">{children}</div>;
+}
+
+export function WorkspaceLogs({ task }: { task: Task }) {
+	const lines = useWorkspaceLogs();
 	const header = isBuildingWorkspace(task)
 		? "Building workspace..."
 		: "Running startup scripts...";
@@ -18,11 +19,7 @@ export function WorkspaceLogs({
 			{lines.length === 0 ? (
 				<LogViewerPlaceholder>Waiting for logs...</LogViewerPlaceholder>
 			) : (
-				lines.map((line, i) => (
-					<div key={i} className="log-entry">
-						{line}
-					</div>
-				))
+				lines.map((line, i) => <LogLine key={i}>{line}</LogLine>)
 			)}
 		</LogViewer>
 	);

packages/tasks/src/hooks/useSelectedTask.ts

@@ -1,10 +1,4 @@
-import {
-	TasksApi,
-	isStableTask,
-	type Task,
-	type TaskDetails,
-} from "@repo/shared";
-import { useIpc } from "@repo/webview-shared/react";
+import { isStableTask, type Task, type TaskDetails } from "@repo/shared";
 import { skipToken, useQuery, useQueryClient } from "@tanstack/react-query";
 import { useEffect, useState } from "react";
 
@@ -20,7 +14,6 @@ const QUERY_KEY = "task-details";
 export function useSelectedTask(tasks: readonly Task[]) {
 	const api = useTasksApi();
 	const queryClient = useQueryClient();
-	const { onNotification } = useIpc();
 	const [selectedTaskId, setSelectedTaskId] = useState<string | null>(null);
 
 	// Auto-deselect when the selected task disappears from the list
@@ -48,14 +41,14 @@ export function useSelectedTask(tasks: readonly Task[]) {
 
 	// Keep selected task in sync with push updates between polls
 	useEffect(() => {
-		return onNotification(TasksApi.taskUpdated, (updatedTask) => {
+		return api.onTaskUpdated((updatedTask) => {
 			if (updatedTask.id !== selectedTaskId) return;
 			queryClient.setQueryData<TaskDetails>(
 				[QUERY_KEY, selectedTaskId],
 				(prev) => (prev ? { ...prev, task: updatedTask } : undefined),
 			);
 		});
-	}, [onNotification, selectedTaskId, queryClient]);
+	}, [api.onTaskUpdated, selectedTaskId, queryClient]);
 
 	const deselectTask = () => {
 		setSelectedTaskId(null);

packages/tasks/src/hooks/useTasksApi.ts

@@ -12,24 +12,13 @@
 import {
 	TasksApi,
 	type CreateTaskParams,
+	type Task,
 	type TaskActionParams,
 } from "@repo/shared";
-import { logger } from "@repo/webview-shared/logger";
 import { useIpc } from "@repo/webview-shared/react";
 
 export function useTasksApi() {
-	const { request, command } = useIpc();
-
-	function safeCommand<P>(
-		definition: { method: string; _types?: { params: P } },
-		...args: P extends void ? [] : [params: P]
-	): void {
-		try {
-			command(definition, ...args);
-		} catch (err) {
-			logger.error(`Command ${definition.method} failed`, err);
-		}
-	}
+	const { request, command, onNotification } = useIpc();
 
 	return {
 		// Requests
@@ -53,8 +42,19 @@ export function useTasksApi() {
 			request(TasksApi.sendTaskMessage, { taskId, message }),
 
 		// Commands
-		viewInCoder: (taskId: string) =>
-			safeCommand(TasksApi.viewInCoder, { taskId }),
-		viewLogs: (taskId: string) => safeCommand(TasksApi.viewLogs, { taskId }),
+		viewInCoder: (taskId: string) => command(TasksApi.viewInCoder, { taskId }),
+		viewLogs: (taskId: string) => command(TasksApi.viewLogs, { taskId }),
+		closeWorkspaceLogs: () => command(TasksApi.closeWorkspaceLogs),
+
+		// Notifications
+		onTaskUpdated: (cb: (task: Task) => void) =>
+			onNotification(TasksApi.taskUpdated, cb),
+		onTasksUpdated: (cb: (tasks: Task[]) => void) =>
+			onNotification(TasksApi.tasksUpdated, cb),
+		onWorkspaceLogsAppend: (cb: (lines: string[]) => void) =>
+			onNotification(TasksApi.workspaceLogsAppend, cb),
+		onRefresh: (cb: () => void) => onNotification(TasksApi.refresh, cb),
+		onShowCreateForm: (cb: () => void) =>
+			onNotification(TasksApi.showCreateForm, cb),
 	};
 }