Skip to content

Commit f7f9316

Browse files
EhabYEhabY
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 abdf3f3 commit f7f9316

File tree

15 files changed

+512
-421
lines changed

15 files changed

+512
-421
lines changed

‎.github/workflows/ci.yaml‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ jobs:
2626

2727
- run: pnpm install
2828

29-
- run: pnpm prettier --check .
29+
- run: pnpm typecheck
30+
31+
- run: pnpm fmt:check
3032

3133
- run: pnpm lint
3234

‎package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
"test:extension": "ELECTRON_RUN_AS_NODE=1 electron node_modules/vitest/vitest.mjs --project extension",
3333
"test:integration": "tsc -p test --outDir out && node esbuild.mjs && vscode-test",
3434
"test:webview": "vitest --project webview",
35+
"typecheck": "tsc --noEmit",
3536
"vscode:prepublish": "pnpm build:production",
3637
"watch": "pnpm watch:all",
3738
"watch:all": "concurrently -n extension,webviews \"pnpm watch:extension\" \"pnpm watch:webviews\"",
@@ -537,7 +538,7 @@
537538
"extensionPack": [
538539
"ms-vscode-remote.remote-ssh"
539540
],
540-
"packageManager": "pnpm@10.27.0",
541+
"packageManager": "pnpm@10.28.2",
541542
"engines": {
542543
"vscode": "^1.95.0",
543544
"node": ">= 20"

‎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: 18 additions & 26 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 {
2321
tasks: Task[];
2422
templates: 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: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,10 @@ export type LogsStatus = "ok" | "not_available" | "error";
4444
/**
4545
* Full details for a selected task, including logs and action availability.
4646
*/
47-
export interface TaskDetails {
47+
export interface TaskDetails extends TaskActions {
4848
task: Task;
4949
logs: TaskLogEntry[];
5050
logsStatus: LogsStatus;
51-
canResume: boolean;
52-
canPause: boolean;
5351
}
5452

5553
export interface TaskActions {

‎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)