Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions apps/vscode-e2e/src/fixtures/subtasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ const SUBTASK_FAST_PARENT_MARKER = "SUBTASK_PARENT_IMMEDIATE_COMPLETION"
const SUBTASK_FAST_CHILD_MARKER = "SUBTASK_CHILD_IMMEDIATE_COMPLETION"
const SUBTASK_APPROVAL_RESTORE_PARENT_MARKER = "SUBTASK_PARENT_APPROVAL_RESTORE"
const SUBTASK_APPROVAL_RESTORE_CHILD_MARKER = "SUBTASK_CHILD_APPROVAL_RESTORE"
export const SUBTASK_PENDING_REPLAY_ROOT = "SUBTASK_PENDING_REPLAY_ROOT: Create a child task."
const SUBTASK_PENDING_REPLAY_CHILD = "SUBTASK_PENDING_REPLAY_CHILD: Create a grandchild task."
const SUBTASK_PENDING_REPLAY_LEAF = "SUBTASK_PENDING_REPLAY_LEAF: Wait for user instructions."
const SUBTASK_XPROFILE_PARENT_MARKER = "SUBTASK_PARENT_CROSS_PROFILE"
const SUBTASK_XPROFILE_SAME_CHILD_MARKER = "SUBTASK_CHILD_SAME_PROFILE"
const SUBTASK_XPROFILE_DIFFERENT_CHILD_MARKER = "SUBTASK_CHILD_DIFFERENT_PROFILE"
Expand Down Expand Up @@ -127,6 +130,30 @@ const completionAfterAnswer = (followupId: string, completionId: string) => ({
})

export function addSubtaskFixtures(mock: InstanceType<typeof LLMock>) {
for (const [prompt, childPrompt, id] of [
[SUBTASK_PENDING_REPLAY_ROOT, SUBTASK_PENDING_REPLAY_CHILD, "call_pending_replay_root"],
[SUBTASK_PENDING_REPLAY_CHILD, SUBTASK_PENDING_REPLAY_LEAF, "call_pending_replay_child"],
]) {
mock.addFixture({
match: { userMessage: prompt, sequenceIndex: 0 },
response: {
toolCalls: [{ name: "new_task", arguments: JSON.stringify({ mode: "ask", message: childPrompt }), id }],
},
})
}
mock.addFixture({
match: { userMessage: SUBTASK_PENDING_REPLAY_LEAF },
response: {
toolCalls: [
{
name: "ask_followup_question",
arguments: JSON.stringify({ question: "What should I do next?", follow_up: [] }),
id: "call_pending_replay_wait",
},
],
},
})

mock.addFixture({
match: {
userMessage: new RegExp(SUBTASK_APPROVAL_RESTORE_PARENT_MARKER),
Expand Down
57 changes: 57 additions & 0 deletions apps/vscode-e2e/src/suite/subtasks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import {
SUBTASK_INTERRUPT_PARENT_PROMPT,
SUBTASK_INTERRUPT_PARENT_RESULT,
SUBTASK_PARENT_PROMPT,
SUBTASK_PENDING_REPLAY_ROOT,
SUBTASK_XPROFILE_DIFFERENT_CHILD_RESULT,
SUBTASK_XPROFILE_PARENT_PROMPT,
SUBTASK_XPROFILE_PARENT_RESULT,
Expand Down Expand Up @@ -260,6 +261,62 @@ suite("Roo Code Subtasks", function () {
}
})

test("interrupted child settles pending new_task before resume despite subtask auto-approval", async () => {
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const api = globalThis.api
const asks: Record<string, ClineMessage[]> = {}
const delegations: Array<[string, string]> = []
const onMessage = ({ taskId, message }: { taskId: string; message: ClineMessage }) => {
if (isCompletedAsk(message)) (asks[taskId] ??= []).push(message)
}
const onDelegated = (parentId: string, childId: string) => {
delegations.push([parentId, childId])
}
const hasNewTaskAsk = (taskId: string) =>
asks[taskId]?.some(
(message) => message.ask === "tool" && JSON.parse(message.text ?? "{}").tool === "newTask",
) ?? false
api.on(RooCodeEventName.Message, onMessage)
api.on(RooCodeEventName.TaskDelegated, onDelegated)
try {
const rootId = await api.startNewTask({
configuration: {
mode: "ask",
autoApprovalEnabled: true,
alwaysAllowSubtasks: false,
enableCheckpoints: false,
},
text: SUBTASK_PENDING_REPLAY_ROOT,
})
await waitFor(() => hasNewTaskAsk(rootId))
await api.approveCurrentAsk()
await waitFor(() => delegations.length === 1)
assert.ok(delegations[0])
const childId = delegations[0][1]
await waitFor(() => hasNewTaskAsk(childId))
await api.clearCurrentTask()
const interrupted = await api.getTaskHistoryItem(childId)
assert.strictEqual(interrupted?.status, "interrupted")
assert.strictEqual(interrupted?.pendingAction?.kind, "create_subtask")

await api.setConfiguration({ autoApprovalEnabled: true, alwaysAllowSubtasks: true })
await api.resumeTask(childId)
// A generic resume ask is emitted only after authoritative pending-action settlement.
// Waiting for it verifies the recovery path finished without creating another child.
await waitFor(() => asks[childId]?.some(({ ask }) => ask === "resume_task") ?? false)
const resumed = await api.getTaskHistoryItem(childId)
assert.strictEqual(resumed?.status, "interrupted")
assert.strictEqual(resumed?.pendingAction, undefined)
assert.strictEqual(resumed?.parentTaskId, rootId)
assert.deepStrictEqual(resumed?.childIds ?? [], [])
assert.strictEqual(api.getCurrentTaskStack().at(-1), childId)
assert.strictEqual(delegations.length, 1)
} finally {
api.off(RooCodeEventName.Message, onMessage)
api.off(RooCodeEventName.TaskDelegated, onDelegated)
while (api.getCurrentTaskStack().length > 0) await api.clearCurrentTask()
}
})

// Smoke: child completing normally must resume the parent task.
test("child task returns to parent after normal completion", async () => {
const api = globalThis.api
Expand Down
6 changes: 5 additions & 1 deletion docs/architecture/task-lifecycle-model.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ Each task slot can also hold one of two pending `create_subtask` actions. A `sta

Production completion also accepts a recovery-compatible `active` parent that still awaits the returning child, then clears the stale pointers. Normal model transitions never create that intermediate state, so it is covered by a focused reducer test rather than admitted as a generally valid reachable state.

An approved live delegation can also resume an `interrupted` task directly into `delegated`. The `resume-delegate` action and detached-task-delegation landmark cover this path without allowing arbitrary message saves to reactivate interrupted tasks. The reducer retains the task's own parent link only when that parent still awaits it; otherwise it clears stale lineage instead of taking ownership back from a newer sibling. Provider rollback persists an error tool result before rehydrating a failed pending delegation, so history resume reconciles the action rather than auto-approving it again. Failed settlement or result persistence stops restoration. Reload remains conservative: persisted pending actions do not distinguish an unattempted approval from a rejected delegation whose settlement failed, so interrupted pending approvals are settled before generic resume rather than replayed. Provider and history-resume tests cover this persistence boundary; the subtask extension-host smoke test checks that enabling subtask auto-approval does not replay the interrupted action (#1714).

## Shared-store concurrency model

The same `pnpm lifecycle:model-check` command also runs a second bounded explorer over two `TaskHistoryStore` hosts. It imports the production `computeHistoryDelta` and `mergeHistoryDelta` functions, so its semantics match the store rather than assuming coherent caches or transactional pair writes:
Expand Down Expand Up @@ -132,7 +134,9 @@ This model is not production-backed. The flags are fields on `Task`, not pure re

Provider locking, paused-child/current-task publication, and semaphore admission/release are explicit model abstractions rather than imported production code. Focused provider and `TaskScheduler` tests cover those concrete adapters. Lifecycle commits and completion use the real reducers. Parent publication and its queued continuation share an explicit transition owner: the fixed policy retains that ownership through matching resume invocation, then models the resumed run settling outside transition ownership. This permits a new delegation generation to begin while the prior resumed run remains active without allowing a stale continuation to start across the newer transition. The fixed policy checks every successor for continuous publication, one child start and commit per generation, exact commit-before-start ownership, permit release before parent resume or redelegation, matching parent transition/continuation ownership at resume invocation, and consistent final child/parent publication. It also requires both resume phases, every other action, and named semantic landmarks to remain reachable and fails if the depth boundary has an unseen successor.

Six injected legacy transition policies must produce deterministic shortest counterexamples through the same explorer: start before commit, resume before permit release, redelegation before permit release, empty current-task publication, two stale provider commits from competing snapshots, and releasing parent-transition serialization immediately after publication. The last witness must causally include first-child completion and parent publication, a second-child commit, release of the first child's scheduler permit, and then the stale first-child continuation. The checker prints the distinct reachable-state count, complete scenario/action/landmark coverage, bounds, and each named counterexample trace. It deliberately does not add a WAL, global profile projection, or scheduler state to persisted `HistoryItem` records.
Seven injected legacy transition policies must produce deterministic shortest counterexamples through the same explorer: start after cancellation, start before commit, resume before permit release, redelegation before permit release, empty current-task publication, two stale provider commits from competing snapshots, and releasing parent-transition serialization immediately after publication. The last witness must causally include first-child completion and parent publication, a second-child commit, release of the first child's scheduler permit, and then the stale first-child continuation. The checker prints the distinct reachable-state count, complete scenario/action/landmark coverage, bounds, and each named counterexample trace. It deliberately does not add a WAL, global profile projection, or scheduler state to persisted `HistoryItem` records.

Handoff cancellation is terminal for scheduling: a paused child is removed before commit, or interrupted with the production reducer after commit so the handoff remains recoverable. The model requires both cancellation landmarks and rejects subsequent child starts or retained active children. It abstracts successful cleanup as one step; focused adapter tests cover cancellation/disposal after approval, during parent flush/eviction, child creation, metadata commit, and publication. Intentional parent eviction is not treated as a cancellation of its own handoff.

For #921, the execution-context matrix checks saved, unsaved, and locked profile selection at the handoff boundary. For that bounded matrix, it establishes only that delegation writes the requested task-local mode and cloned configuration into the child context. It does not prove that every downstream consumer reads that context. The checker retains a divergent-mode witness in which the child task mode differs from the shared provider mode so reader refinements can demonstrate that choosing the wrong source is observable.

Expand Down
49 changes: 47 additions & 2 deletions scripts/check-provider-handoff-scheduler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import assert from "node:assert/strict"
import type { HistoryItem, ProviderSettings } from "@roo-code/types"

import { selectHandoffExecutionContext } from "../src/core/task/providerHandoff"
import { completeDelegatedChild, delegateTaskToChild } from "../src/core/task-persistence/taskLifecycle"
import {
completeDelegatedChild,
delegateTaskToChild,
interruptDelegatedChild,
} from "../src/core/task-persistence/taskLifecycle"

const PROVIDERS = ["a", "b"] as const
type Provider = (typeof PROVIDERS)[number]
Expand All @@ -18,9 +22,11 @@ type Policy = {
emptyPublication?: boolean
staleConcurrentCommits?: boolean
releaseParentTransitionAfterPublication?: boolean
startAfterCancellation?: boolean
}

type ModelState = {
cancelled?: boolean
generation: Generation
parent: HistoryItem
children: Partial<Record<PublishedTask, HistoryItem>>
Expand Down Expand Up @@ -62,8 +68,11 @@ const EXPECTED_ACTIONS = [
"resume-parent",
"settle-parent",
"redelegate",
"cancel-handoff",
] as const
const LANDMARKS = {
"cancelled-before-commit": (state: ModelState) => state.cancelled === true && state.commitOwner === undefined,
"cancelled-after-commit": (state: ModelState) => state.cancelled === true && state.commitOwner !== undefined,
"competing-claims": (state: ModelState) => Object.keys(state.claims).length === 2,
"prepared-before-commit": (state: ModelState) => state.prepared !== undefined && state.commitOwner === undefined,
"committed-before-start": (state: ModelState) =>
Expand All @@ -84,6 +93,11 @@ const LANDMARKS = {

const FIXED_POLICY: Policy = { name: "fixed" }
const LEGACY_POLICIES: Array<Policy & { expectedViolation: string }> = [
{
name: "start-after-cancellation",
startAfterCancellation: true,
expectedViolation: "child started after cancellation",
},
{ name: "start-before-commit", startBeforeCommit: true, expectedViolation: "child started without exact commit" },
{
name: "resume-before-permit-release",
Expand Down Expand Up @@ -251,6 +265,33 @@ function explore(

function transitions(state: ModelState, policy: Policy): Transition[] {
const result: Transition[] = []
if (state.cancelled) {
if (policy.startAfterCancellation && state.commitOwner) {
return [
action("start-cancelled-child", "start", state, (next) => {
next.startedProviders = [state.commitOwner!]
next.childPermit = "held"
}),
]
}
return result
}
if (state.startedProviders.length === 0 && !state.parentQueued) {
result.push(
action("cancel-handoff", "cancel-handoff", state, (next) => {
next.cancelled = true
next.publishedTask = undefined
if (state.commitOwner) {
const childId = childIdFor(state.generation, state.commitOwner)
next.children[childId] = interruptDelegatedChild(state.parent, state.children[childId]!)
}
if (state.prepared) {
delete next.children[childIdFor(state.generation, state.prepared.provider)]
next.prepared = undefined
}
}),
)
}
for (const provider of PROVIDERS) {
if (!state.parentQueued && !state.claims[provider] && state.commitOwner === undefined) {
result.push(
Expand Down Expand Up @@ -407,7 +448,11 @@ function transitions(state: ModelState, policy: Policy): Transition[] {

function invariantViolations(state: ModelState): string[] {
const violations: string[] = []
if (!state.publishedTask) violations.push("observable current task is empty")
if (state.cancelled && state.startedProviders.length > 0) violations.push("child started after cancellation")
if (state.cancelled && Object.values(state.children).some((child) => child?.status === "active")) {
violations.push("cancelled handoff retained an active child")
}
if (!state.publishedTask && !state.cancelled) violations.push("observable current task is empty")
if (state.startedProviders.length > 1) violations.push("multiple child starts for one parent generation")
if (state.committedProviders.length > 1) violations.push("multiple provider commits for one parent generation")
for (const provider of state.parentQueued ? [] : state.startedProviders) {
Expand Down
49 changes: 43 additions & 6 deletions scripts/check-task-lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,20 @@ interface WitnessContext {
const MAX_DEPTH = 12
const MAX_STATES = 10_000
const actionIds = ["action-1", "action-2"] as const
const expectedActions = ["delegate", "interrupt", "complete", "abandon", "stage", "settle-rejected"] as const
const expectedActions = [
"delegate",
"resume-delegate",
"interrupt",
"complete",
"abandon",
"stage",
"settle-rejected",
] as const
const semanticLandmarks = {
"detached-task-delegation": (state: ModelState) =>
state["child-a"]?.status === "delegated" &&
state["child-a"].parentTaskId === undefined &&
state["child-a"].awaitingChildId === "child-b",
"interrupted-child-redelegation": (state: ModelState) =>
state.parent?.status === "delegated" &&
state.parent.awaitingChildId === "child-b" &&
Expand Down Expand Up @@ -146,14 +158,20 @@ function transitions(state: ModelState): Transition[] {

const awaitedStatus = parent.awaitingChildId ? state[parent.awaitingChildId as TaskId]?.status : undefined
const delegationValid =
parent.status === "active" || (parent.status === "delegated" && awaitedStatus === "interrupted")
parent.status === "active" ||
parent.status === "interrupted" ||
(parent.status === "delegated" && awaitedStatus === "interrupted")

for (const childId of taskIds) {
if (childId === parentId || state[childId]) continue
if (!delegationValid) continue
const delegated = { ...delegateTaskToChild(parent, childId, awaitedStatus), pendingAction: undefined }
const owningParent = parent.parentTaskId ? state[parent.parentTaskId as TaskId] : undefined
const delegated = {
...delegateTaskToChild(parent, childId, awaitedStatus, owningParent),
pendingAction: undefined,
}
result.push({
name: `delegate(${parentId}, ${childId})`,
name: `${parent.status === "interrupted" ? "resume-delegate" : "delegate"}(${parentId}, ${childId})`,
next: replace(state, delegated, task(childId, parentId)),
delegation: { parentId },
})
Expand All @@ -168,7 +186,13 @@ function transitions(state: ModelState): Transition[] {
}

const pending = parent.pendingAction
if (!delegationValid && parent.status !== "completed" && pending?.kind === "create_subtask") {
// Reload conservatively settles interrupted approvals: persisted actions do not
// distinguish an unattempted approval from a rejected attempt whose settlement failed.
if (
(!delegationValid || parent.status === "interrupted") &&
parent.status !== "completed" &&
pending?.kind === "create_subtask"
) {
result.push({
name: `settle-rejected(${parentId}, ${actionId})`,
next: replace(state, settleRejectedCreateSubtaskAction(parent, actionId)),
Expand Down Expand Up @@ -416,8 +440,17 @@ function runRepresentativeScenarios(): void {
assert.throws(() => delegateTaskToChild(delegated, "child-b", "active"), /not interrupted/)

const interruptedA = interruptDelegatedChild(delegated, childA)
const resumedA = delegateTaskToChild(interruptedA, "child-b", undefined, delegated)
assert.equal(resumedA.status, "delegated")
assert.equal(resumedA.parentTaskId, "parent")
const nestedReturn = completeDelegatedChild(resumedA, task("child-b", "child-a"), "nested result")
assert.equal(completeDelegatedChild(delegated, nestedReturn.parent, "resumed result").child.status, "completed")

const redelegated = delegateTaskToChild(delegated, "child-b", interruptedA.status)
assert.throws(() => completeDelegatedChild(redelegated, interruptedA, "stale"), /not delegated to child/)
const detached = delegateTaskToChild(interruptedA, "new-grandchild", undefined, redelegated)
assert.equal(detached.parentTaskId, undefined)
assert.equal(detached.rootTaskId, undefined)

const abandoned = abandonDelegatedChild(delegated, interruptedA)
assert.throws(() => completeDelegatedChild(abandoned.parent, abandoned.child, "late"), /not delegated to child/)
Expand All @@ -437,7 +470,11 @@ function runRepresentativeScenarios(): void {
status: "interrupted",
pendingAction: createSubtaskAction("action-1"),
}
assert.throws(() => delegateTaskToChild(rejectedParent, childA.id), /Invalid task status transition/)
assert.equal(delegateTaskToChild(rejectedParent, childA.id).status, "delegated")
assert.throws(
() => delegateTaskToChild({ ...rejectedParent, status: "completed" }, childA.id),
/Invalid task status transition/,
)
const settled = settleRejectedCreateSubtaskAction(rejectedParent, "action-1")
assert.equal(settled.status, "interrupted")
assert.equal(settled.pendingAction, undefined)
Expand Down
Loading
Loading