Skip to content

Commit 8d70b9d

Browse files
EhabYEhabY
committed
Address review comments 2
1 parent d1bb1ce commit 8d70b9d

File tree

3 files changed

+86
-182
lines changed

3 files changed

+86
-182
lines changed

‎packages/shared/src/tasks/types.ts‎

Lines changed: 31 additions & 59 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.
@@ -52,68 +43,49 @@ export interface TaskDetails extends TaskActions {
5243

5344
export interface TaskActions {
5445
canPause: boolean;
46+
pauseDisabled: boolean;
5547
canResume: boolean;
5648
}
5749

58-
const RESUMABLE_STATUSES: readonly WorkspaceStatus[] = [
59-
"stopped",
60-
"failed",
61-
"canceled",
50+
const PAUSABLE_STATUSES: readonly TaskStatus[] = [
51+
"active",
52+
"initializing",
53+
"pending",
54+
"error",
55+
"unknown",
6256
];
6357

64-
const PAUSED_STATUSES: readonly WorkspaceStatus[] = [
65-
"stopped",
66-
"stopping",
67-
"canceled",
58+
const PAUSE_DISABLED_STATUSES: readonly TaskStatus[] = [
59+
"pending",
60+
"initializing",
6861
];
6962

70-
const INITIALIZING_STATUSES: readonly WorkspaceStatus[] = [
71-
"starting",
72-
"pending",
63+
const RESUMABLE_STATUSES: readonly TaskStatus[] = [
64+
"paused",
65+
"error",
66+
"unknown",
7367
];
7468

7569
export function getTaskActions(task: Task): TaskActions {
7670
const hasWorkspace = task.workspace_id !== null;
77-
const status = task.workspace_status;
71+
const status = task.status;
7872
return {
79-
canPause: hasWorkspace && status === "running",
80-
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),
8176
};
8277
}
8378

84-
/** UI state derived from task status, state, and workspace status */
85-
export type TaskUIState =
86-
| "working"
87-
| "idle"
88-
| "complete"
89-
| "error"
90-
| "paused"
91-
| "initializing";
92-
93-
/** Compute the UI state from a Task object */
94-
export function getTaskUIState(task: Task): TaskUIState {
95-
const taskState = task.current_state?.state;
96-
const workspaceStatus = task.workspace_status;
97-
98-
if (task.status === "error" || taskState === "failed") {
99-
return "error";
100-
}
101-
102-
if (workspaceStatus && PAUSED_STATUSES.includes(workspaceStatus)) {
103-
return "paused";
104-
}
105-
106-
if (workspaceStatus && INITIALIZING_STATUSES.includes(workspaceStatus)) {
107-
return "initializing";
108-
}
109-
110-
if (task.status === "active" && workspaceStatus === "running" && taskState) {
111-
return taskState;
112-
}
113-
114-
if (taskState === "complete") {
115-
return "complete";
116-
}
117-
118-
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+
);
11991
}

‎src/webviews/tasks/tasksPanel.ts‎

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import * as vscode from "vscode";
44
import {
55
commandHandler,
66
getTaskActions,
7-
getTaskUIState,
7+
isStableTask,
88
requestHandler,
99
TasksApi,
1010
type CreateTaskParams,
@@ -15,7 +15,6 @@ import {
1515
type LogsStatus,
1616
type TaskDetails,
1717
type TaskTemplate,
18-
type TaskUIState,
1918
} from "@repo/shared";
2019

2120
import { type CoderApi } from "../../api/coderApi";
@@ -63,13 +62,6 @@ function isIpcCommand(
6362
);
6463
}
6564

66-
/** UI states where task logs won't change */
67-
const STABLE_UI_STATES: readonly TaskUIState[] = [
68-
"complete",
69-
"error",
70-
"paused",
71-
];
72-
7365
export class TasksPanel
7466
implements vscode.WebviewViewProvider, vscode.Disposable
7567
{
@@ -88,7 +80,6 @@ export class TasksPanel
8880
taskId: string;
8981
logs: TaskLogEntry[];
9082
status: LogsStatus;
91-
uiState: TaskUIState;
9283
};
9384

9485
/**
@@ -462,23 +453,18 @@ export class TasksPanel
462453
private async getLogsWithCache(
463454
task: Task,
464455
): Promise<{ logs: TaskLogEntry[]; logsStatus: LogsStatus }> {
465-
const uiState = getTaskUIState(task);
466-
const isStable = STABLE_UI_STATES.includes(uiState);
456+
const stable = isStableTask(task);
467457

468-
// Use cache if same task in same stable state
469-
if (
470-
this.cachedLogs?.taskId === task.id &&
471-
isStable &&
472-
this.cachedLogs.uiState === uiState
473-
) {
458+
// Use cache if same task in stable state
459+
if (this.cachedLogs?.taskId === task.id && stable) {
474460
return { logs: this.cachedLogs.logs, logsStatus: this.cachedLogs.status };
475461
}
476462

477463
const { logs, status } = await this.fetchTaskLogs(task.id);
478464

479465
// Cache only for stable states
480-
if (isStable) {
481-
this.cachedLogs = { taskId: task.id, logs, status, uiState };
466+
if (stable) {
467+
this.cachedLogs = { taskId: task.id, logs, status };
482468
}
483469

484470
return { logs, logsStatus: status };

‎test/webview/shared/tasks/types.test.ts‎

Lines changed: 49 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,16 @@ import { describe, expect, it } from "vitest";
22

33
import {
44
getTaskActions,
5-
getTaskUIState,
5+
isStableTask,
66
type Task,
77
type TaskActions,
8-
type TaskUIState,
98
} from "@repo/shared";
109

11-
import { minimalTask as task, taskState as state } from "../../../mocks/tasks";
10+
import {
11+
minimalTask as task,
12+
task as fullTask,
13+
taskState as state,
14+
} from "../../../mocks/tasks";
1215

1316
describe("getTaskActions", () => {
1417
interface TaskActionsTestCase {
@@ -20,136 +23,79 @@ describe("getTaskActions", () => {
2023
{
2124
name: "no workspace",
2225
overrides: {},
23-
expected: { canPause: false, canResume: false },
26+
expected: { canPause: false, pauseDisabled: false, canResume: false },
2427
},
2528
{
26-
name: "running workspace",
27-
overrides: { workspace_id: "ws", workspace_status: "running" },
28-
expected: { canPause: true, canResume: false },
29+
name: "active task",
30+
overrides: { workspace_id: "ws", status: "active" },
31+
expected: { canPause: true, pauseDisabled: false, canResume: false },
2932
},
3033
{
31-
name: "stopped workspace",
32-
overrides: { workspace_id: "ws", workspace_status: "stopped" },
33-
expected: { canPause: false, canResume: true },
34+
name: "paused task",
35+
overrides: { workspace_id: "ws", status: "paused" },
36+
expected: { canPause: false, pauseDisabled: false, canResume: true },
3437
},
3538
{
36-
name: "failed workspace",
37-
overrides: { workspace_id: "ws", workspace_status: "failed" },
38-
expected: { canPause: false, canResume: true },
39+
name: "error task (both actions available)",
40+
overrides: { workspace_id: "ws", status: "error" },
41+
expected: { canPause: true, pauseDisabled: false, canResume: true },
3942
},
4043
{
41-
name: "canceled workspace",
42-
overrides: { workspace_id: "ws", workspace_status: "canceled" },
43-
expected: { canPause: false, canResume: true },
44+
name: "unknown task (both actions available)",
45+
overrides: { workspace_id: "ws", status: "unknown" },
46+
expected: { canPause: true, pauseDisabled: false, canResume: true },
4447
},
4548
{
46-
name: "starting workspace",
47-
overrides: { workspace_id: "ws", workspace_status: "starting" },
48-
expected: { canPause: false, canResume: false },
49+
name: "initializing task (pause disabled)",
50+
overrides: { workspace_id: "ws", status: "initializing" },
51+
expected: { canPause: true, pauseDisabled: true, canResume: false },
4952
},
5053
{
51-
name: "pending workspace",
52-
overrides: { workspace_id: "ws", workspace_status: "pending" },
53-
expected: { canPause: false, canResume: false },
54+
name: "pending task (pause disabled)",
55+
overrides: { workspace_id: "ws", status: "pending" },
56+
expected: { canPause: true, pauseDisabled: true, canResume: false },
5457
},
5558
{
56-
name: "workspace_id null with running status",
57-
overrides: { workspace_id: null, workspace_status: "running" },
58-
expected: { canPause: false, canResume: false },
59+
name: "no workspace ignores task status",
60+
overrides: { workspace_id: null, status: "active" },
61+
expected: { canPause: false, pauseDisabled: false, canResume: false },
5962
},
6063
])("$name", ({ overrides, expected }) => {
6164
expect(getTaskActions(task(overrides))).toEqual(expected);
6265
});
6366
});
6467

65-
describe("getTaskUIState", () => {
66-
interface TaskUIStateTestCase {
67-
name: string;
68-
overrides: Partial<Task>;
69-
expected: TaskUIState;
70-
}
71-
it.each<TaskUIStateTestCase>([
72-
// Error states (highest priority)
73-
{
74-
name: "task status error",
75-
overrides: { status: "error" },
76-
expected: "error",
77-
},
78-
{
79-
name: "task state failed",
80-
overrides: { current_state: state("failed") },
81-
expected: "error",
82-
},
83-
{
84-
name: "error takes priority over paused",
85-
overrides: { status: "error", workspace_status: "stopped" },
86-
expected: "error",
87-
},
88-
89-
// Paused states
68+
describe("isStableTask", () => {
69+
it.each<{ name: string; overrides: Partial<Task>; expected: boolean }>([
70+
{ name: "error status", overrides: { status: "error" }, expected: true },
71+
{ name: "paused status", overrides: { status: "paused" }, expected: true },
9072
{
91-
name: "stopped workspace",
92-
overrides: { workspace_status: "stopped" },
93-
expected: "paused",
94-
},
95-
{
96-
name: "stopping workspace",
97-
overrides: { workspace_status: "stopping" },
98-
expected: "paused",
99-
},
100-
{
101-
name: "canceled workspace",
102-
overrides: { workspace_status: "canceled" },
103-
expected: "paused",
104-
},
105-
106-
// Initializing states
107-
{
108-
name: "starting workspace",
109-
overrides: { workspace_status: "starting" },
110-
expected: "initializing",
111-
},
112-
{
113-
name: "pending workspace",
114-
overrides: { workspace_status: "pending" },
115-
expected: "initializing",
73+
name: "complete state",
74+
overrides: { current_state: state("complete") },
75+
expected: true,
11676
},
117-
118-
// Active states
77+
{ name: "active status", overrides: { status: "active" }, expected: false },
11978
{
120-
name: "working state",
121-
overrides: {
122-
status: "active",
123-
workspace_status: "running",
124-
current_state: state("working"),
125-
},
126-
expected: "working",
79+
name: "active with working state",
80+
overrides: { status: "active", current_state: state("working") },
81+
expected: false,
12782
},
12883
{
129-
name: "idle state",
130-
overrides: {
131-
status: "active",
132-
workspace_status: "running",
133-
current_state: state("idle"),
134-
},
135-
expected: "idle",
84+
name: "initializing status",
85+
overrides: { status: "initializing" },
86+
expected: false,
13687
},
137-
138-
// Complete state
13988
{
140-
name: "complete state",
141-
overrides: { current_state: state("complete") },
142-
expected: "complete",
89+
name: "pending status",
90+
overrides: { status: "pending" },
91+
expected: false,
14392
},
144-
145-
// Default fallback
146-
{ name: "no workspace or state", overrides: {}, expected: "idle" },
14793
{
148-
name: "running but no state",
149-
overrides: { workspace_status: "running", current_state: null },
150-
expected: "idle",
94+
name: "unknown status",
95+
overrides: { status: "unknown" },
96+
expected: false,
15197
},
15298
])("$name → $expected", ({ overrides, expected }) => {
153-
expect(getTaskUIState(task(overrides))).toBe(expected);
99+
expect(isStableTask(fullTask(overrides))).toBe(expected);
154100
});
155101
});

0 commit comments

Comments
 (0)