Skip to content

Commit b1f941b

Browse files
committed
Address code review feedback for Tasks panel IPC
- Remove unused `scope` feature from IPC protocol (YAGNI) - Remove redundant array spreads in TasksPanel - Make TaskDetails extend TaskActions to reduce duplication - Remove handler fallback in TasksPanel (keep requests/commands separate) - Add notification subscription support to useIpc hook - Add tests for onNotification feature - Remove redundant individual exports from tasks/api.ts (export as group only)
1 parent 68ae6e1 commit b1f941b

File tree

13 files changed

+609
-611
lines changed

13 files changed

+609
-611
lines changed

packages/shared/src/ipc/protocol.ts

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,23 +10,20 @@
1010
/** Request definition: params P, response R */
1111
export interface RequestDef<P = void, R = void> {
1212
readonly method: string;
13-
readonly scope?: string;
1413
/** @internal Phantom types for inference - not present at runtime */
1514
readonly _types?: { params: P; response: R };
1615
}
1716

1817
/** Command definition: params P, no response */
1918
export interface CommandDef<P = void> {
2019
readonly method: string;
21-
readonly scope?: string;
2220
/** @internal Phantom type for inference - not present at runtime */
2321
readonly _types?: { params: P };
2422
}
2523

2624
/** Notification definition: data D (extension to webview) */
2725
export interface NotificationDef<D = void> {
2826
readonly method: string;
29-
readonly scope?: string;
3027
/** @internal Phantom type for inference - not present at runtime */
3128
readonly _types?: { data: D };
3229
}
@@ -36,25 +33,20 @@ export interface NotificationDef<D = void> {
3633
/** Define a request with typed params and response */
3734
export function defineRequest<P = void, R = void>(
3835
method: string,
39-
scope?: string,
4036
): RequestDef<P, R> {
41-
return { method, scope } as RequestDef<P, R>;
37+
return { method } as RequestDef<P, R>;
4238
}
4339

4440
/** Define a fire-and-forget command */
45-
export function defineCommand<P = void>(
46-
method: string,
47-
scope?: string,
48-
): CommandDef<P> {
49-
return { method, scope } as CommandDef<P>;
41+
export function defineCommand<P = void>(method: string): CommandDef<P> {
42+
return { method } as CommandDef<P>;
5043
}
5144

5245
/** Define a push notification (extension to webview) */
5346
export function defineNotification<D = void>(
5447
method: string,
55-
scope?: string,
5648
): NotificationDef<D> {
57-
return { method, scope } as NotificationDef<D>;
49+
return { method } as NotificationDef<D>;
5850
}
5951

6052
// --- Wire format ---
@@ -63,7 +55,6 @@ export function defineNotification<D = void>(
6355
export interface IpcRequest<P = unknown> {
6456
readonly requestId: string;
6557
readonly method: string;
66-
readonly scope?: string;
6758
readonly params?: P;
6859
}
6960

packages/shared/src/tasks/api.ts

Lines changed: 20 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,18 @@ import {
1717

1818
import type { Task, TaskDetails, TaskLogEntry, TaskTemplate } from "./types";
1919

20-
// --- Requests ---
21-
2220
export interface InitResponse {
23-
tasks: Task[];
24-
templates: TaskTemplate[];
21+
tasks: readonly Task[];
22+
templates: readonly TaskTemplate[];
2523
baseUrl: string;
2624
tasksSupported: boolean;
2725
}
2826

29-
export const init = defineRequest<void, InitResponse>("init");
30-
export const getTasks = defineRequest<void, Task[]>("getTasks");
31-
export const getTemplates = defineRequest<void, TaskTemplate[]>("getTemplates");
32-
export const getTask = defineRequest<{ taskId: string }, Task>("getTask");
33-
export const getTaskDetails = defineRequest<{ taskId: string }, TaskDetails>(
27+
const init = defineRequest<void, InitResponse>("init");
28+
const getTasks = defineRequest<void, Task[]>("getTasks");
29+
const getTemplates = defineRequest<void, TaskTemplate[]>("getTemplates");
30+
const getTask = defineRequest<{ taskId: string }, Task>("getTask");
31+
const getTaskDetails = defineRequest<{ taskId: string }, TaskDetails>(
3432
"getTaskDetails",
3533
);
3634

@@ -39,31 +37,25 @@ export interface CreateTaskParams {
3937
prompt: string;
4038
presetId?: string;
4139
}
42-
export const createTask = defineRequest<CreateTaskParams, Task>("createTask");
43-
44-
export const deleteTask = defineRequest<{ taskId: string }, void>("deleteTask");
45-
export const pauseTask = defineRequest<{ taskId: string }, void>("pauseTask");
46-
export const resumeTask = defineRequest<{ taskId: string }, void>("resumeTask");
40+
const createTask = defineRequest<CreateTaskParams, Task>("createTask");
4741

48-
// --- Commands ---
42+
const deleteTask = defineRequest<{ taskId: string }, void>("deleteTask");
43+
const pauseTask = defineRequest<{ taskId: string }, void>("pauseTask");
44+
const resumeTask = defineRequest<{ taskId: string }, void>("resumeTask");
4945

50-
export const viewInCoder = defineCommand<{ taskId: string }>("viewInCoder");
51-
export const viewLogs = defineCommand<{ taskId: string }>("viewLogs");
52-
export const downloadLogs = defineCommand<{ taskId: string }>("downloadLogs");
53-
export const sendTaskMessage = defineCommand<{
46+
const viewInCoder = defineCommand<{ taskId: string }>("viewInCoder");
47+
const viewLogs = defineCommand<{ taskId: string }>("viewLogs");
48+
const downloadLogs = defineCommand<{ taskId: string }>("downloadLogs");
49+
const sendTaskMessage = defineCommand<{
5450
taskId: string;
5551
message: string;
5652
}>("sendTaskMessage");
5753

58-
// --- Notifications ---
59-
60-
export const taskUpdated = defineNotification<Task>("taskUpdated");
61-
export const tasksUpdated = defineNotification<Task[]>("tasksUpdated");
62-
export const logsAppend = defineNotification<TaskLogEntry[]>("logsAppend");
63-
export const refresh = defineNotification<void>("refresh");
64-
export const showCreateForm = defineNotification<void>("showCreateForm");
65-
66-
// --- Grouped export ---
54+
const taskUpdated = defineNotification<Task>("taskUpdated");
55+
const tasksUpdated = defineNotification<Task[]>("tasksUpdated");
56+
const logsAppend = defineNotification<TaskLogEntry[]>("logsAppend");
57+
const refresh = defineNotification<void>("refresh");
58+
const showCreateForm = defineNotification<void>("showCreateForm");
6759

6860
export const TasksApi = {
6961
// Requests

packages/shared/src/tasks/types.ts

Lines changed: 32 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -5,19 +5,10 @@ import type {
55
TaskState,
66
TaskStatus,
77
Template,
8-
WorkspaceStatus,
98
} from "coder/site/src/api/typesGenerated";
109

1110
// Re-export SDK types for convenience
12-
export type {
13-
Preset,
14-
Task,
15-
TaskLogEntry,
16-
TaskState,
17-
TaskStatus,
18-
Template,
19-
WorkspaceStatus,
20-
};
11+
export type { Preset, Task, TaskLogEntry, TaskState, TaskStatus, Template };
2112

2213
/**
2314
* Combined template and preset information for the create task form.
@@ -44,78 +35,57 @@ export type LogsStatus = "ok" | "not_available" | "error";
4435
/**
4536
* Full details for a selected task, including logs and action availability.
4637
*/
47-
export interface TaskDetails {
38+
export interface TaskDetails extends TaskActions {
4839
task: Task;
4940
logs: TaskLogEntry[];
5041
logsStatus: LogsStatus;
51-
canResume: boolean;
52-
canPause: boolean;
5342
}
5443

5544
export interface TaskActions {
5645
canPause: boolean;
46+
pauseDisabled: boolean;
5747
canResume: boolean;
5848
}
5949

60-
const RESUMABLE_STATUSES: readonly WorkspaceStatus[] = [
61-
"stopped",
62-
"failed",
63-
"canceled",
50+
const PAUSABLE_STATUSES: readonly TaskStatus[] = [
51+
"active",
52+
"initializing",
53+
"pending",
54+
"error",
55+
"unknown",
6456
];
6557

66-
const PAUSED_STATUSES: readonly WorkspaceStatus[] = [
67-
"stopped",
68-
"stopping",
69-
"canceled",
58+
const PAUSE_DISABLED_STATUSES: readonly TaskStatus[] = [
59+
"pending",
60+
"initializing",
7061
];
7162

72-
const INITIALIZING_STATUSES: readonly WorkspaceStatus[] = [
73-
"starting",
74-
"pending",
63+
const RESUMABLE_STATUSES: readonly TaskStatus[] = [
64+
"paused",
65+
"error",
66+
"unknown",
7567
];
7668

7769
export function getTaskActions(task: Task): TaskActions {
7870
const hasWorkspace = task.workspace_id !== null;
79-
const status = task.workspace_status;
71+
const status = task.status;
8072
return {
81-
canPause: hasWorkspace && status === "running",
82-
canResume: hasWorkspace && !!status && RESUMABLE_STATUSES.includes(status),
73+
canPause: hasWorkspace && PAUSABLE_STATUSES.includes(status),
74+
pauseDisabled: PAUSE_DISABLED_STATUSES.includes(status),
75+
canResume: hasWorkspace && RESUMABLE_STATUSES.includes(status),
8376
};
8477
}
8578

86-
/** UI state derived from task status, state, and workspace status */
87-
export type TaskUIState =
88-
| "working"
89-
| "idle"
90-
| "complete"
91-
| "error"
92-
| "paused"
93-
| "initializing";
94-
95-
/** Compute the UI state from a Task object */
96-
export function getTaskUIState(task: Task): TaskUIState {
97-
const taskState = task.current_state?.state;
98-
const workspaceStatus = task.workspace_status;
99-
100-
if (task.status === "error" || taskState === "failed") {
101-
return "error";
102-
}
103-
104-
if (workspaceStatus && PAUSED_STATUSES.includes(workspaceStatus)) {
105-
return "paused";
106-
}
107-
108-
if (workspaceStatus && INITIALIZING_STATUSES.includes(workspaceStatus)) {
109-
return "initializing";
110-
}
111-
112-
if (task.status === "active" && workspaceStatus === "running" && taskState) {
113-
return taskState;
114-
}
115-
116-
if (taskState === "complete") {
117-
return "complete";
118-
}
119-
120-
return "idle";
79+
/**
80+
* Task statuses where logs won't change (stable/terminal states).
81+
* "complete" is a TaskState (sub-state of active), checked separately.
82+
*/
83+
const STABLE_STATUSES: readonly TaskStatus[] = ["error", "paused"];
84+
85+
/** Whether a task is in a stable state where its logs won't change. */
86+
export function isStableTask(task: Task): boolean {
87+
return (
88+
STABLE_STATUSES.includes(task.status) ||
89+
task.current_state?.state === "complete"
90+
);
12191
}

packages/tasks/src/hooks/useTasksApi.ts

Lines changed: 17 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -9,44 +9,32 @@
99
* ```
1010
*/
1111

12-
import {
13-
createTask as createTaskDef,
14-
deleteTask as deleteTaskDef,
15-
downloadLogs as downloadLogsDef,
16-
getTask as getTaskDef,
17-
getTaskDetails as getTaskDetailsDef,
18-
getTasks as getTasksDef,
19-
getTemplates as getTemplatesDef,
20-
init as initDef,
21-
pauseTask as pauseTaskDef,
22-
resumeTask as resumeTaskDef,
23-
sendTaskMessage as sendTaskMessageDef,
24-
viewInCoder as viewInCoderDef,
25-
viewLogs as viewLogsDef,
26-
type CreateTaskParams,
27-
} from "@repo/shared";
12+
import { TasksApi, type CreateTaskParams } from "@repo/shared";
2813
import { useIpc } from "@repo/webview-shared/react";
2914

3015
export function useTasksApi() {
3116
const { request, command } = useIpc();
3217

3318
return {
3419
// Requests
35-
init: () => request(initDef),
36-
getTasks: () => request(getTasksDef),
37-
getTemplates: () => request(getTemplatesDef),
38-
getTask: (taskId: string) => request(getTaskDef, { taskId }),
39-
getTaskDetails: (taskId: string) => request(getTaskDetailsDef, { taskId }),
40-
createTask: (params: CreateTaskParams) => request(createTaskDef, params),
41-
deleteTask: (taskId: string) => request(deleteTaskDef, { taskId }),
42-
pauseTask: (taskId: string) => request(pauseTaskDef, { taskId }),
43-
resumeTask: (taskId: string) => request(resumeTaskDef, { taskId }),
20+
init: () => request(TasksApi.init),
21+
getTasks: () => request(TasksApi.getTasks),
22+
getTemplates: () => request(TasksApi.getTemplates),
23+
getTask: (taskId: string) => request(TasksApi.getTask, { taskId }),
24+
getTaskDetails: (taskId: string) =>
25+
request(TasksApi.getTaskDetails, { taskId }),
26+
createTask: (params: CreateTaskParams) =>
27+
request(TasksApi.createTask, params),
28+
deleteTask: (taskId: string) => request(TasksApi.deleteTask, { taskId }),
29+
pauseTask: (taskId: string) => request(TasksApi.pauseTask, { taskId }),
30+
resumeTask: (taskId: string) => request(TasksApi.resumeTask, { taskId }),
4431

4532
// Commands
46-
viewInCoder: (taskId: string) => command(viewInCoderDef, { taskId }),
47-
viewLogs: (taskId: string) => command(viewLogsDef, { taskId }),
48-
downloadLogs: (taskId: string) => command(downloadLogsDef, { taskId }),
33+
viewInCoder: (taskId: string) => command(TasksApi.viewInCoder, { taskId }),
34+
viewLogs: (taskId: string) => command(TasksApi.viewLogs, { taskId }),
35+
downloadLogs: (taskId: string) =>
36+
command(TasksApi.downloadLogs, { taskId }),
4937
sendTaskMessage: (taskId: string, message: string) =>
50-
command(sendTaskMessageDef, { taskId, message }),
38+
command(TasksApi.sendTaskMessage, { taskId, message }),
5139
};
5240
}

packages/webview-shared/src/index.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ export interface WebviewMessage {
33
method: string;
44
params?: unknown;
55
requestId?: string;
6-
scope?: string;
76
}
87

98
// VS Code state API

0 commit comments

Comments
 (0)