Skip to content

Commit a1446cf

Browse files
roomoteroomote
committed
test(task): cover remaining handoff guards
1 parent e3073e3 commit a1446cf

4 files changed

Lines changed: 409 additions & 5 deletions

File tree

‎src/__tests__/history-resume-delegation.spec.ts‎

Lines changed: 134 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,75 @@ describe("History resume delegation - parent metadata transitions", () => {
313313
)
314314
})
315315

316+
it("preserves an unrelated child pending action when completion has no action owner", async () => {
317+
const parentHistoryItem = {
318+
id: "parent-unowned-action",
319+
status: "delegated",
320+
awaitingChildId: "child-unowned-action",
321+
childIds: ["child-unowned-action"],
322+
ts: 1,
323+
task: "Parent",
324+
tokensIn: 0,
325+
tokensOut: 0,
326+
totalCost: 0,
327+
}
328+
const pendingAction = {
329+
kind: "finish_subtask" as const,
330+
actionId: "other-action",
331+
approvalText: "{}",
332+
parentTaskId: "parent-unowned-action",
333+
result: "Other result",
334+
}
335+
const childHistoryItem = {
336+
id: "child-unowned-action",
337+
status: "active",
338+
pendingAction,
339+
}
340+
let updatedChild: HistoryItem | undefined
341+
const taskHistoryStore = makeTaskHistoryStoreStub(childHistoryItem, parentHistoryItem, {
342+
atomicUpdatePair: vi.fn(
343+
async (
344+
_firstId: string,
345+
_secondId: string,
346+
firstUpdater: (item: HistoryItem) => HistoryItem,
347+
secondUpdater: (item: HistoryItem) => HistoryItem,
348+
options?: { whileFirstFileLocked?: () => Promise<void> },
349+
) => {
350+
firstUpdater(parentHistoryItem as HistoryItem)
351+
updatedChild = secondUpdater(childHistoryItem as HistoryItem)
352+
await options?.whileFirstFileLocked?.()
353+
return []
354+
},
355+
),
356+
})
357+
const provider = makeProviderStub({
358+
contextProxy: { globalStorageUri: { fsPath: "/tmp" } },
359+
getTaskWithId: vi.fn().mockResolvedValue({ historyItem: parentHistoryItem }),
360+
emit: vi.fn(),
361+
getCurrentTask: vi.fn(() => ({ taskId: "different-task" })),
362+
removeClineFromStack: vi.fn(),
363+
createTaskWithHistoryItem: vi.fn().mockResolvedValue({
364+
resumeAfterDelegation: vi.fn().mockResolvedValue(undefined),
365+
overwriteClineMessages: vi.fn().mockResolvedValue(undefined),
366+
overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined),
367+
}),
368+
taskHistoryStore,
369+
})
370+
371+
vi.mocked(readTaskMessages).mockResolvedValue([])
372+
vi.mocked(readApiMessages).mockResolvedValue([])
373+
vi.mocked(saveTaskMessages).mockResolvedValue(undefined)
374+
vi.mocked(saveApiMessages).mockResolvedValue(undefined)
375+
376+
await ClineProvider.prototype.reopenParentFromDelegation.call(provider, {
377+
parentTaskId: "parent-unowned-action",
378+
childTaskId: "child-unowned-action",
379+
completionResultSummary: "Done",
380+
})
381+
382+
expect(updatedChild?.pendingAction).toEqual(pendingAction)
383+
})
384+
316385
it("reopenParentFromDelegation injects subtask_result into both UI and API histories", async () => {
317386
const parentItem = {
318387
id: "p1",
@@ -517,11 +586,17 @@ describe("History resume delegation - parent metadata transitions", () => {
517586
const existingApiMessages = [
518587
{
519588
role: "assistant" as const,
520-
content: [{ type: "tool_use" as const, name: "new_task", id: "tool-existing", input: {} }],
589+
content: [
590+
{ type: "tool_use" as const, name: "read_file", id: "tool-unrelated", input: {} },
591+
{ type: "tool_use" as const, name: "new_task", id: "tool-existing", input: {} },
592+
],
521593
},
522594
{
523595
role: "user" as const,
524-
content: [{ type: "tool_result" as const, tool_use_id: "tool-existing", content: "old result" }],
596+
content: [
597+
{ type: "tool_result" as const, tool_use_id: "tool-unrelated", content: "read result" },
598+
{ type: "tool_result" as const, tool_use_id: "tool-existing", content: "old result" },
599+
],
525600
},
526601
]
527602

@@ -540,13 +615,13 @@ describe("History resume delegation - parent metadata transitions", () => {
540615
expect(persistedApiMessages).toHaveLength(2)
541616
expect(persistedApiMessages[1]).toMatchObject({
542617
role: "user",
543-
content: [
618+
content: expect.arrayContaining([
544619
{
545620
type: "tool_result",
546621
tool_use_id: "tool-existing",
547622
content: "Subtask c-existing-result completed.\n\nResult:\nreplacement result",
548623
},
549-
],
624+
]),
550625
})
551626
})
552627

@@ -600,6 +675,61 @@ describe("History resume delegation - parent metadata transitions", () => {
600675
expect((injected.content[0] as any).text).toContain("Subtask c-no-tool completed")
601676
})
602677

678+
it("keeps already-injected UI and fallback API completion records idempotent", async () => {
679+
const parentItem = {
680+
id: "p-existing-fallback",
681+
status: "delegated",
682+
awaitingChildId: "c-existing-fallback",
683+
childIds: ["c-existing-fallback"],
684+
ts: 100,
685+
task: "Parent with existing fallback",
686+
tokensIn: 0,
687+
tokensOut: 0,
688+
totalCost: 0,
689+
}
690+
const completionResultSummary = "Already recorded"
691+
const fallbackText = `Subtask c-existing-fallback completed.\n\nResult:\n${completionResultSummary}`
692+
const existingUiMessages = [
693+
{
694+
type: "say" as const,
695+
say: "subtask_result" as const,
696+
text: completionResultSummary,
697+
ts: 50,
698+
},
699+
]
700+
const existingApiMessages = [
701+
{ role: "user" as const, content: [{ type: "text" as const, text: fallbackText }], ts: 50 },
702+
]
703+
const taskHistoryStore = makeTaskHistoryStoreStub({ id: "c-existing-fallback", status: "active" }, parentItem)
704+
const provider = makeProviderStub({
705+
contextProxy: { globalStorageUri: { fsPath: "/storage" } },
706+
getTaskWithId: vi.fn().mockResolvedValue({ historyItem: parentItem }),
707+
emit: vi.fn(),
708+
getCurrentTask: vi.fn(() => ({ taskId: "c-existing-fallback" })),
709+
removeClineFromStack: vi.fn().mockResolvedValue(undefined),
710+
createTaskWithHistoryItem: vi.fn().mockResolvedValue({
711+
resumeAfterDelegation: vi.fn().mockResolvedValue(undefined),
712+
overwriteClineMessages: vi.fn().mockResolvedValue(undefined),
713+
overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined),
714+
}),
715+
taskHistoryStore,
716+
})
717+
718+
vi.mocked(readTaskMessages).mockResolvedValue(existingUiMessages)
719+
vi.mocked(readApiMessages).mockResolvedValue(existingApiMessages)
720+
vi.mocked(saveTaskMessages).mockResolvedValue(undefined)
721+
vi.mocked(saveApiMessages).mockResolvedValue(undefined)
722+
723+
await ClineProvider.prototype.reopenParentFromDelegation.call(provider, {
724+
parentTaskId: "p-existing-fallback",
725+
childTaskId: "c-existing-fallback",
726+
completionResultSummary,
727+
})
728+
729+
expect(vi.mocked(saveTaskMessages).mock.calls[0][0].messages).toEqual(existingUiMessages)
730+
expect(vi.mocked(saveApiMessages).mock.calls[0][0].messages).toEqual(existingApiMessages)
731+
})
732+
603733
it("reopenParentFromDelegation sets skipPrevResponseIdOnce via resumeAfterDelegation", async () => {
604734
const parentInstance: any = {
605735
skipPrevResponseIdOnce: false,

‎src/__tests__/provider-delegation.spec.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,48 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => {
131131
expect(current).toMatchObject({ status: "delegated", awaitingChildId: "child-1" })
132132
})
133133

134+
it("preserves an unrelated pending action when delegation has no action owner", async () => {
135+
const pendingAction = {
136+
kind: "create_subtask" as const,
137+
actionId: "other-action",
138+
approvalText: "{}",
139+
mode: "code",
140+
message: "Other request",
141+
todos: [],
142+
}
143+
let current: HistoryItem = { ...parentHistoryItem, status: "active", pendingAction }
144+
const taskHistoryStore = {
145+
withTaskFileLock: vi.fn(async (_taskId: string, callback: () => Promise<unknown>) => callback()),
146+
get: vi.fn(() => current),
147+
atomicReadAndUpdate: vi.fn(async (_taskId: string, updater: (item: HistoryItem) => HistoryItem) => {
148+
current = updater(current)
149+
return [current]
150+
}),
151+
}
152+
const parentTask = makeParentTask()
153+
const child = { taskId: "child-1", run: vi.fn().mockResolvedValue(undefined) }
154+
const provider = {
155+
taskScheduler: new TaskScheduler(),
156+
emit: vi.fn(),
157+
getCurrentTask: vi.fn(() => parentTask),
158+
removeClineFromStack: vi.fn().mockResolvedValue(undefined),
159+
createTask: vi.fn().mockResolvedValue(child),
160+
handleModeSwitch: vi.fn().mockResolvedValue(undefined),
161+
log: vi.fn(),
162+
isViewLaunched: false,
163+
taskHistoryStore,
164+
} as unknown as ClineProvider
165+
166+
await ClineProvider.prototype.delegateParentAndOpenChild.call(provider, {
167+
parentTaskId: "parent-1",
168+
message: "Do something",
169+
initialTodos: [],
170+
mode: "code",
171+
})
172+
173+
expect(current.pendingAction).toEqual(pendingAction)
174+
})
175+
134176
it("rolls back when pending-action ownership changes before the atomic parent update", async () => {
135177
const pendingAction = {
136178
kind: "create_subtask" as const,

‎src/core/task-persistence/TaskHistoryStore.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1152,7 +1152,7 @@ export class TaskHistoryStore {
11521152
const rollbackSnapshot = firstDiskSnapshot
11531153
let restoredFirst = rollbackSnapshot
11541154
await safeWriteJson(await this.getTaskFilePath(firstId), rollbackSnapshot, {
1155-
lockAcquired: holdFirstFileLock || options?.firstFileLockAcquired,
1155+
lockAcquired: true,
11561156
merge: (existing) => {
11571157
if (!existing || typeof existing !== "object" || !("id" in existing)) {
11581158
throw new Error(

0 commit comments

Comments
 (0)