Skip to content

Commit 378804c

Browse files
committed
🤖 refactor: reduce task service duplication
1 parent b3f9fa2 commit 378804c

File tree

2 files changed

+32
-85
lines changed

2 files changed

+32
-85
lines changed

src/node/services/taskService.test.ts

Lines changed: 28 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,29 @@ import { execSync } from "node:child_process";
77
import { Config } from "@/node/config";
88
import { HistoryService } from "@/node/services/historyService";
99
import { PartialService } from "@/node/services/partialService";
10-
import { TaskService } from "@/node/services/taskService";
10+
import { TaskService, type AgentTaskStatus } from "@/node/services/taskService";
1111
import { createRuntime } from "@/node/runtime/runtimeFactory";
1212
import { Ok, Err, type Result } from "@/common/types/result";
1313
import type { StreamEndEvent } from "@/common/types/stream";
1414
import { createMuxMessage } from "@/common/types/message";
1515
import type { WorkspaceMetadata } from "@/common/types/workspace";
1616
import type { AIService } from "@/node/services/aiService";
1717
import type { WorkspaceService } from "@/node/services/workspaceService";
18-
import type { InitStateManager } from "@/node/services/initStateManager";
19-
import { InitStateManager as RealInitStateManager } from "@/node/services/initStateManager";
18+
import {
19+
InitStateManager as RealInitStateManager,
20+
type InitStateManager,
21+
} from "@/node/services/initStateManager";
22+
interface TaskServiceStatusInternal {
23+
setTaskStatus: (
24+
workspaceId: string,
25+
status: AgentTaskStatus,
26+
options?: { allowMissing?: boolean }
27+
) => Promise<void>;
28+
}
29+
30+
interface TaskServiceWaiterInternal extends TaskServiceStatusInternal {
31+
resolveWaiters: (taskId: string, report: { reportMarkdown: string; title?: string }) => void;
32+
}
2033

2134
function initGitRepo(projectPath: string): void {
2235
execSync("git init -b main", { cwd: projectPath, stdio: "ignore" });
@@ -384,15 +397,8 @@ describe("TaskService", () => {
384397
expect(queued.data.status).toBe("queued");
385398

386399
// Free the slot by marking the first task as reported.
387-
await config.editConfig((cfg) => {
388-
for (const [_project, project] of cfg.projects) {
389-
const ws = project.workspaces.find((w) => w.id === running.data.taskId);
390-
if (ws) {
391-
ws.taskStatus = "reported";
392-
}
393-
}
394-
return cfg;
395-
});
400+
const internal = taskService as unknown as TaskServiceStatusInternal;
401+
await internal.setTaskStatus(running.data.taskId, "reported");
396402

397403
await taskService.initialize();
398404

@@ -488,9 +494,8 @@ describe("TaskService", () => {
488494
requestingWorkspaceId: parentTask.data.taskId,
489495
});
490496

491-
const internal = taskService as unknown as {
497+
const internal = taskService as unknown as TaskServiceWaiterInternal & {
492498
maybeStartQueuedTasks: () => Promise<void>;
493-
resolveWaiters: (taskId: string, report: { reportMarkdown: string; title?: string }) => void;
494499
};
495500

496501
await internal.maybeStartQueuedTasks();
@@ -609,15 +614,8 @@ describe("TaskService", () => {
609614
);
610615

611616
// Free slot and start queued tasks.
612-
await config.editConfig((cfg) => {
613-
for (const [_project, project] of cfg.projects) {
614-
const ws = project.workspaces.find((w) => w.id === running.data.taskId);
615-
if (ws) {
616-
ws.taskStatus = "reported";
617-
}
618-
}
619-
return cfg;
620-
});
617+
const internal = taskService as unknown as TaskServiceStatusInternal;
618+
await internal.setTaskStatus(running.data.taskId, "reported");
621619

622620
await taskService.initialize();
623621

@@ -1354,14 +1352,7 @@ describe("TaskService", () => {
13541352
// Wait longer than timeout while task is still queued.
13551353
await new Promise((r) => setTimeout(r, 100));
13561354

1357-
const internal = taskService as unknown as {
1358-
setTaskStatus: (
1359-
workspaceId: string,
1360-
status: "queued" | "running" | "awaiting_report" | "reported",
1361-
options?: { allowMissing?: boolean }
1362-
) => Promise<void>;
1363-
resolveWaiters: (taskId: string, report: { reportMarkdown: string; title?: string }) => void;
1364-
};
1355+
const internal = taskService as unknown as TaskServiceWaiterInternal;
13651356

13661357
await internal.setTaskStatus(childId, "running");
13671358
internal.resolveWaiters(childId, { reportMarkdown: "ok" });
@@ -1403,12 +1394,7 @@ describe("TaskService", () => {
14031394

14041395
const { taskService } = createTaskServiceHarness(config);
14051396

1406-
const internal = taskService as unknown as {
1407-
setTaskStatus: (
1408-
workspaceId: string,
1409-
status: "queued" | "running" | "awaiting_report" | "reported"
1410-
) => Promise<void>;
1411-
};
1397+
const internal = taskService as unknown as TaskServiceStatusInternal;
14121398

14131399
await internal.setTaskStatus(childId, "running");
14141400

@@ -1452,12 +1438,7 @@ describe("TaskService", () => {
14521438

14531439
const { taskService } = createTaskServiceHarness(config);
14541440

1455-
const internal = taskService as unknown as {
1456-
setTaskStatus: (
1457-
workspaceId: string,
1458-
status: "queued" | "running" | "awaiting_report" | "reported"
1459-
) => Promise<void>;
1460-
};
1441+
const internal = taskService as unknown as TaskServiceStatusInternal;
14611442

14621443
await internal.setTaskStatus(childId, "reported");
14631444

@@ -1560,9 +1541,7 @@ describe("TaskService", () => {
15601541

15611542
const { taskService } = createTaskServiceHarness(config);
15621543

1563-
const internal = taskService as unknown as {
1564-
resolveWaiters: (taskId: string, report: { reportMarkdown: string; title?: string }) => void;
1565-
};
1544+
const internal = taskService as unknown as TaskServiceWaiterInternal;
15661545
internal.resolveWaiters(childId, { reportMarkdown: "ok", title: "t" });
15671546

15681547
await config.removeWorkspace(childId);
@@ -1603,9 +1582,7 @@ describe("TaskService", () => {
16031582

16041583
const { taskService } = createTaskServiceHarness(config);
16051584

1606-
const internal = taskService as unknown as {
1607-
resolveWaiters: (taskId: string, report: { reportMarkdown: string; title?: string }) => void;
1608-
};
1585+
const internal = taskService as unknown as TaskServiceWaiterInternal;
16091586
internal.resolveWaiters(childId, { reportMarkdown: "ok", title: "t" });
16101587

16111588
await config.removeWorkspace(childId);
@@ -1645,9 +1622,7 @@ describe("TaskService", () => {
16451622

16461623
const { taskService } = createTaskServiceHarness(config);
16471624

1648-
const internal = taskService as unknown as {
1649-
resolveWaiters: (taskId: string, report: { reportMarkdown: string; title?: string }) => void;
1650-
};
1625+
const internal = taskService as unknown as TaskServiceWaiterInternal;
16511626
internal.resolveWaiters(childId, { reportMarkdown: "ok", title: "t" });
16521627

16531628
await config.removeWorkspace(childId);
@@ -1687,8 +1662,7 @@ describe("TaskService", () => {
16871662

16881663
const { taskService } = createTaskServiceHarness(config);
16891664

1690-
const internal = taskService as unknown as {
1691-
resolveWaiters: (taskId: string, report: { reportMarkdown: string; title?: string }) => void;
1665+
const internal = taskService as unknown as TaskServiceWaiterInternal & {
16921666
cleanupExpiredCompletedReports: (nowMs: number) => void;
16931667
};
16941668
internal.resolveWaiters(childId, { reportMarkdown: "ok", title: "t" });

src/node/services/taskService.ts

Lines changed: 4 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1266,7 +1266,7 @@ export class TaskService {
12661266
`listDescendantAgentTasks: task ${taskId} is missing parentWorkspaceId`
12671267
);
12681268

1269-
const status: AgentTaskStatus = entry.taskStatus ?? "running";
1269+
const status = normalizeAgentTaskStatus(entry.taskStatus);
12701270
if (!statusFilter || statusFilter.has(status)) {
12711271
result.push({
12721272
taskId,
@@ -1446,7 +1446,7 @@ export class TaskService {
14461446
private countActiveAgentTasks(config: ReturnType<Config["loadConfigOrDefault"]>): number {
14471447
let activeCount = 0;
14481448
for (const task of this.listAgentTaskWorkspaces(config)) {
1449-
const status: AgentTaskStatus = task.taskStatus ?? "running";
1449+
const status = normalizeAgentTaskStatus(task.taskStatus);
14501450
// If this task workspace is blocked in a foreground wait, do not count it towards parallelism.
14511451
// This prevents deadlocks where a task spawns a nested task in the foreground while
14521452
// maxParallelAgentTasks is low (e.g. 1).
@@ -1824,7 +1824,6 @@ export class TaskService {
18241824
parentWorkspaceId: ws.parentWorkspaceId ?? null,
18251825
persistedStatus: ws.taskStatus ?? null,
18261826
});
1827-
shouldStartWaiters = currentStatus === "running";
18281827
return;
18291828
}
18301829

@@ -1953,8 +1952,7 @@ export class TaskService {
19531952
workspace: WorkspaceConfigEntry;
19541953
}): Promise<void> {
19551954
const childWorkspaceId = entry.workspace.id;
1956-
const parentWorkspaceId = entry.workspace.parentWorkspaceId;
1957-
if (!childWorkspaceId || !parentWorkspaceId) {
1955+
if (!childWorkspaceId) {
19581956
return;
19591957
}
19601958

@@ -1966,35 +1964,10 @@ export class TaskService {
19661964
"posting its last assistant output as a fallback.)*\n\n" +
19671965
(lastText?.trim().length ? lastText : "(No assistant output found.)");
19681966

1969-
// Notify clients immediately even if we can't delete the workspace yet.
1970-
await this.setTaskStatus(childWorkspaceId, "reported", { allowMissing: true });
1971-
1972-
await this.deliverReportToParent(parentWorkspaceId, entry, {
1967+
await this.finalizeAgentTaskReport(childWorkspaceId, entry, {
19731968
reportMarkdown,
19741969
title: `Subagent (${agentType}) report (fallback)`,
19751970
});
1976-
1977-
this.resolveWaiters(childWorkspaceId, {
1978-
reportMarkdown,
1979-
title: `Subagent (${agentType}) report (fallback)`,
1980-
});
1981-
1982-
await this.maybeStartQueuedTasks();
1983-
await this.cleanupReportedLeafTask(childWorkspaceId);
1984-
1985-
const postCfg = this.config.loadConfigOrDefault();
1986-
const hasActiveDescendants = this.hasActiveDescendantAgentTasks(postCfg, parentWorkspaceId);
1987-
if (!hasActiveDescendants && !this.aiService.isStreaming(parentWorkspaceId)) {
1988-
const resumeResult = await this.workspaceService.resumeStream(parentWorkspaceId, {
1989-
model: entry.workspace.taskModelString ?? defaultModel,
1990-
});
1991-
if (!resumeResult.success) {
1992-
log.error("Failed to auto-resume parent after fallback report", {
1993-
parentWorkspaceId,
1994-
error: resumeResult.error,
1995-
});
1996-
}
1997-
}
19981971
}
19991972

20001973
private async readLatestAssistantText(workspaceId: string): Promise<string | null> {

0 commit comments

Comments
 (0)