Skip to content

Commit 7328cbf

Browse files
authored
fix(task): preserve subtask links after repeated Stop (#1678)
* fix(task): preserve subtask links after repeated Stop * test(task): cover stale cancellation guard cleanup * fix(task): refresh delegated child state inside cancellation lock * test(task): assert refreshed child rehydration
1 parent 78b74ec commit 7328cbf

2 files changed

Lines changed: 168 additions & 5 deletions

File tree

‎src/core/webview/ClineProvider.ts‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3637,16 +3637,30 @@ export class ClineProvider
36373637
const { historyItem: parentHistory } = await this.getTaskWithId(task.parentTaskId!)
36383638

36393639
if (parentHistory?.status === "delegated" && parentHistory?.awaitingChildId === task.taskId) {
3640+
// Refresh the child after acquiring the parent transition lock. The pre-abort
3641+
// history snapshot can be stale if another serialized path interrupted it.
3642+
historyItem =
3643+
this.taskHistoryStore.get(task.taskId) ??
3644+
(await this.getTaskWithId(task.taskId)).historyItem
36403645
// Mark the child interrupted and leave parent delegated with awaitingChildId
36413646
// intact — the user can resume this child later and it will report back.
3642-
historyItem = interruptDelegatedChild(parentHistory, historyItem!)
3643-
await this.updateTaskHistory(historyItem)
3647+
// A previous cancellation may already have persisted the interrupted status
3648+
// before its caller lost the response. Treat that replay as success without
3649+
// weakening the lifecycle state machine's self-loop rejection.
3650+
if (historyItem!.status !== "interrupted") {
3651+
historyItem = interruptDelegatedChild(parentHistory, historyItem!)
3652+
await this.updateTaskHistory(historyItem)
3653+
this.log(
3654+
`[cancelTask] Marked child ${task.taskId} interrupted; parent ${task.parentTaskId} stays delegated`,
3655+
)
3656+
} else {
3657+
this.log(
3658+
`[cancelTask] Child ${task.taskId} is already interrupted; parent ${task.parentTaskId} stays delegated`,
3659+
)
3660+
}
36443661
// Clear any stale fail-closed entry from a prior failed cancel attempt so
36453662
// reopenParentFromDelegation is not incorrectly blocked on resume.
36463663
this.cancelledDelegationChildIds.delete(task.taskId)
3647-
this.log(
3648-
`[cancelTask] Marked child ${task.taskId} interrupted; parent ${task.parentTaskId} stays delegated`,
3649-
)
36503664
}
36513665
})
36523666
} catch (error) {

‎src/core/webview/__tests__/ClineProvider.flicker-free-cancel.spec.ts‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -663,6 +663,155 @@ describe("ClineProvider flicker-free cancel", () => {
663663
)
664664
})
665665

666+
it.each([
667+
["a stale cancellation guard", true],
668+
["an empty cancellation guard", false],
669+
] as const)(
670+
"preserves delegated lineage when cancelling an already-interrupted child with %s",
671+
async (_case, seedGuard) => {
672+
const childHistory: HistoryItem = {
673+
id: "child-1",
674+
number: 2,
675+
task: "child task",
676+
ts: Date.now(),
677+
tokensIn: 10,
678+
tokensOut: 20,
679+
totalCost: 0.001,
680+
workspace: "/test/workspace",
681+
parentTaskId: "parent-1",
682+
rootTaskId: "root-1",
683+
status: "interrupted",
684+
}
685+
const parentHistory: HistoryItem = {
686+
id: "parent-1",
687+
number: 1,
688+
task: "parent task",
689+
ts: Date.now(),
690+
tokensIn: 10,
691+
tokensOut: 20,
692+
totalCost: 0.001,
693+
workspace: "/test/workspace",
694+
status: "delegated",
695+
awaitingChildId: "child-1",
696+
delegatedToId: "child-1",
697+
}
698+
699+
Object.assign(mockTask1, {
700+
taskId: "child-1",
701+
instanceId: "instance-child",
702+
rootTask: { taskId: "root-1" },
703+
parentTask: { taskId: "parent-1" },
704+
parentTaskId: "parent-1",
705+
cancelCurrentRequest: vi.fn(),
706+
abortTask: vi.fn().mockResolvedValue(undefined),
707+
abandoned: false,
708+
isStreaming: false,
709+
didFinishAbortingStream: true,
710+
isWaitingForFirstChunk: false,
711+
})
712+
seedRegistry(provider, mockTask1)
713+
provider.getTaskWithId = vi.fn().mockImplementation((id) => {
714+
if (id === "child-1") return Promise.resolve({ historyItem: childHistory })
715+
if (id === "parent-1") return Promise.resolve({ historyItem: parentHistory })
716+
throw new Error(`unexpected task lookup: ${id}`)
717+
}) as unknown as ClineProvider["getTaskWithId"]
718+
719+
const updateTaskHistorySpy = vi.spyOn(provider, "updateTaskHistory").mockResolvedValue([])
720+
const createTaskWithHistoryItemSpy = vi
721+
.spyOn(provider, "createTaskWithHistoryItem")
722+
.mockResolvedValue(undefined as unknown as CreatedHistoryTask)
723+
if (seedGuard) provider["cancelledDelegationChildIds"].add("child-1")
724+
expect(provider["cancelledDelegationChildIds"].has("child-1")).toBe(seedGuard)
725+
726+
await provider.cancelTask()
727+
728+
expect(updateTaskHistorySpy).not.toHaveBeenCalled()
729+
expect(createTaskWithHistoryItemSpy).toHaveBeenCalledWith(
730+
expect.objectContaining({
731+
id: "child-1",
732+
status: "interrupted",
733+
parentTaskId: "parent-1",
734+
rootTaskId: "root-1",
735+
parentTask: expect.objectContaining({ taskId: "parent-1" }),
736+
rootTask: expect.objectContaining({ taskId: "root-1" }),
737+
}),
738+
)
739+
expect(provider["cancelledDelegationChildIds"].has("child-1")).toBe(false)
740+
},
741+
)
742+
743+
it("uses the in-lock child status when another transition interrupted it", async () => {
744+
const activeChild: HistoryItem = {
745+
id: "child-race",
746+
number: 2,
747+
task: "child task",
748+
ts: Date.now(),
749+
tokensIn: 10,
750+
tokensOut: 20,
751+
totalCost: 0.001,
752+
workspace: "/test/workspace",
753+
parentTaskId: "parent-race",
754+
rootTaskId: "root-race",
755+
status: "active",
756+
}
757+
const interruptedChild: HistoryItem = { ...activeChild, status: "interrupted" }
758+
const parentHistory: HistoryItem = {
759+
id: "parent-race",
760+
number: 1,
761+
task: "parent task",
762+
ts: Date.now(),
763+
tokensIn: 10,
764+
tokensOut: 20,
765+
totalCost: 0.001,
766+
workspace: "/test/workspace",
767+
status: "delegated",
768+
awaitingChildId: "child-race",
769+
delegatedToId: "child-race",
770+
}
771+
772+
Object.assign(mockTask1, {
773+
taskId: "child-race",
774+
instanceId: "instance-child-race",
775+
rootTask: { taskId: "root-race" },
776+
parentTask: { taskId: "parent-race" },
777+
parentTaskId: "parent-race",
778+
cancelCurrentRequest: vi.fn(),
779+
abortTask: vi.fn().mockResolvedValue(undefined),
780+
abandoned: false,
781+
isStreaming: false,
782+
didFinishAbortingStream: true,
783+
isWaitingForFirstChunk: false,
784+
})
785+
seedRegistry(provider, mockTask1)
786+
let childReads = 0
787+
provider.getTaskWithId = vi.fn().mockImplementation((id) => {
788+
if (id === "child-race") {
789+
childReads += 1
790+
return Promise.resolve({ historyItem: childReads === 1 ? activeChild : interruptedChild })
791+
}
792+
if (id === "parent-race") return Promise.resolve({ historyItem: parentHistory })
793+
throw new Error(`unexpected task lookup: ${id}`)
794+
}) as unknown as ClineProvider["getTaskWithId"]
795+
796+
const updateTaskHistorySpy = vi.spyOn(provider, "updateTaskHistory").mockResolvedValue([])
797+
const createTaskWithHistoryItemSpy = vi
798+
.spyOn(provider, "createTaskWithHistoryItem")
799+
.mockResolvedValue(undefined as unknown as CreatedHistoryTask)
800+
801+
await provider.cancelTask()
802+
803+
expect(childReads).toBe(2)
804+
expect(createTaskWithHistoryItemSpy).toHaveBeenCalledWith(
805+
expect.objectContaining({
806+
id: "child-race",
807+
status: "interrupted",
808+
parentTaskId: "parent-race",
809+
rootTaskId: "root-race",
810+
}),
811+
)
812+
expect(updateTaskHistorySpy).not.toHaveBeenCalled()
813+
})
814+
666815
it("detaches runtime parent links when delegated parent detach fails", async () => {
667816
const mockRootTask = { taskId: "root-1" }
668817
const mockParentTask = { taskId: "parent-1" }

0 commit comments

Comments
 (0)