diff --git a/docs/architecture/task-lifecycle-model.md b/docs/architecture/task-lifecycle-model.md index d39c0fd9e2..9266d49987 100644 --- a/docs/architecture/task-lifecycle-model.md +++ b/docs/architecture/task-lifecycle-model.md @@ -6,13 +6,14 @@ Zoo Code checks task lifecycle protocols through one compositional verification pnpm lifecycle:model-check ``` -The command runs five independent bounded submodels in sequence: +The command runs six independent bounded submodels in sequence: 1. the persisted task delegation lifecycle; 2. shared-store concurrency across task-history hosts; -3. the task cleanup protocol; -4. request-stream parser scoping; and -5. completion persistence. +3. production-backed provider handoff and scheduler ordering; +4. the task cleanup protocol; +5. request-stream parser scoping; and +6. completion persistence. This umbrella command is the single model-check entry point in the `compile` CI job after type checking. Command-level composition does not merge the submodels' state spaces: each checker retains its own bounds, transitions, invariant ownership, reachability requirements, and counterexample format. In particular, parser state is not part of the persisted lifecycle graph. The focused parser checker remains directly runnable with `pnpm parser-scope:model-check` for debugging. @@ -79,6 +80,14 @@ The known-unsafe witnesses currently compare exact shortest action sequences. Th The umbrella command also runs a separate bounded child model for in-memory abort, disposal, and provider-shutdown ordering. It models cleanup settlement and rejection as environment transitions and makes no filesystem, editor Promise, fairness, or timing-liveness claim. See [Task cleanup protocol model check](./task-cleanup-protocol-model.md). +## Provider handoff and scheduler model + +`scripts/check-provider-handoff-scheduler.ts` is a separate bounded adapter model for the runtime boundary that the persisted lifecycle graph does not represent. Its breadth-first explorer normalizes provider-keyed records and owner arrays before deduplicating canonical states, then exhaustively explores enabled action orderings through depth 15 with a 20,000-state budget. It imports `selectHandoffExecutionContext` and the existing `delegateTaskToChild` and `completeDelegatedChild` reducers. A direct saved, unsaved, and locked-profile matrix verifies task-local configuration isolation. Stale provider lookup is caught before this pure selector, so focused provider tests verify the failed lookup, contextual log, and fallback. The protocol state then models two provider instances, their claims and parent snapshots, authoritative parent/child records, current task publication, commit/start ownership, the child scheduler permit, queued and resumed parent state, and one bounded redelegation generation. + +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. + ## Completion persistence model `scripts/check-completion-persistence.ts` models the completion-readiness protocol that protects the public `TaskCompleted` event. It starts from both standalone and delegated tasks and exhaustively interleaves: @@ -121,16 +130,16 @@ These are safety claims within the documented bounds. The checks do not claim li The following map separates issue observations from the architectural interpretation encoded here. Open issues can change after this document is written; follow each link for current status. -| Issue and directly observed evidence | Derived protocol rule | Production transition and current check | -| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| [#1469](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1469): the issue report states that a barrier-controlled two-host run reproduced an old child completion clearing a newer handoff 25/25 times. | Completion is conditional on the parent still awaiting that exact child; a live-linked child must remain owned by its parent. | `completeDelegatedChild` rejects stale authoritative input. The lifecycle explorer checks that reducer rule, while the shared-store explorer reproduces the cross-host stale-cache counterexample with an exact causal witness. | -| [#1021](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1021): an in-flight `saveClineMessages` can restore parent/root IDs after abandonment cleared them. | Detachment should be monotonic: later lifecycle work must not reattach an abandoned child. | `abandonDelegatedChild` clears both sides. The shared-store explorer proves the detach commit occurs, then reproduces a refreshed-cache delta that preserves interrupted status while restoring stale live-task lineage. | -| [#1453](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1453), under user report [#1279](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1279): CI observed `TaskCompleted` before restart-visible API history once; 120 local repetitions did not reproduce it. | Completion implies restart-visible assistant history. Delayed or failed writes keep completion pending, and cancellation settles readiness without starting stale retries or emitting completion. | The completion persistence explorer checks the bounded event-ordering and cancellation contract for standalone and delegated tasks. Focused `Task` and `AttemptCompletionTool` tests cover the production adapter; `restart-persistence.test.ts` verifies visibility through a fresh extension host. | -| [#921](https://github.com/Zoo-Code-Org/Zoo-Code/issues/921): delegation across parallel tabs lacks coverage for different view-local mode/profile state. | Delegation must bind an explicit immutable execution-context snapshot rather than read whichever view is focused later. | The persisted ownership transition is covered; mode/profile snapshot isolation is outside this state model and belongs in a production adapter/model-based test. | -| [#920](https://github.com/Zoo-Code-Org/Zoo-Code/issues/920): issue analysis identifies a missing cross-instance history-update test and potential lost writes. | Distinct task writes must not overwrite one another, and same-task conflicts need an explicit merge/ownership rule. | The shared-store explorer checks distinct-task writes and same-record independent deltas. Cross-instance store tests retain production API coverage, and the synchronized real-filesystem smoke test exercises the actual lock/write path without claiming exhaustive filesystem proof. | -| [#369](https://github.com/Zoo-Code-Org/Zoo-Code/issues/369) and [#372](https://github.com/Zoo-Code-Org/Zoo-Code/issues/372): planned fan-out keeps a parent live while a child runs and requires completion routing by explicit parent ID, single-writer result readiness, permit release, and orphan cleanup. | Persisted `delegated` status is ownership, not proof that the parent instance is suspended. Completion must route by IDs; scheduler resources and live-instance state need separate invariants. | Nested and sibling lifecycle ownership are covered. Scheduler permits, live/suspended parent selection, orphan cancellation, and single-writer message readiness must be added when fan-out lands; they should not be folded into `HistoryItem` fields prematurely. | +| Issue and directly observed evidence | Derived protocol rule | Production transition and current check | +| -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| [#1469](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1469): the issue report states that a barrier-controlled two-host run reproduced an old child completion clearing a newer handoff 25/25 times. | Completion is conditional on the parent still awaiting that exact child; a live-linked child must remain owned by its parent. | `completeDelegatedChild` rejects stale authoritative input. The lifecycle explorer checks that reducer rule, while the shared-store explorer reproduces the cross-host stale-cache counterexample with an exact causal witness. | +| [#1021](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1021): an in-flight `saveClineMessages` can restore parent/root IDs after abandonment cleared them. | Detachment should be monotonic: later lifecycle work must not reattach an abandoned child. | `abandonDelegatedChild` clears both sides. The shared-store explorer proves the detach commit occurs, then reproduces a refreshed-cache delta that preserves interrupted status while restoring stale live-task lineage. | +| [#1453](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1453), under user report [#1279](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1279): CI observed `TaskCompleted` before restart-visible API history once; 120 local repetitions did not reproduce it. | Completion implies restart-visible assistant history. Delayed or failed writes keep completion pending, and cancellation settles readiness without starting stale retries or emitting completion. | The completion persistence explorer checks the bounded event-ordering and cancellation contract for standalone and delegated tasks. Focused `Task` and `AttemptCompletionTool` tests cover the production adapter; `restart-persistence.test.ts` verifies visibility through a fresh extension host. | +| [#921](https://github.com/Zoo-Code-Org/Zoo-Code/issues/921): delegation across parallel tabs lacks coverage for different view-local mode/profile state. | Delegation must bind an explicit immutable execution-context snapshot rather than read whichever view is focused later. | The persisted ownership transition is covered; mode/profile snapshot isolation is outside this state model and belongs in a production adapter/model-based test. | +| [#920](https://github.com/Zoo-Code-Org/Zoo-Code/issues/920): issue analysis identifies a missing cross-instance history-update test and potential lost writes. | Distinct task writes must not overwrite one another, and same-task conflicts need an explicit merge/ownership rule. | The shared-store explorer checks distinct-task writes and same-record independent deltas. Cross-instance store tests retain production API coverage, and the synchronized real-filesystem smoke test exercises the actual lock/write path without claiming exhaustive filesystem proof. | +| [#369](https://github.com/Zoo-Code-Org/Zoo-Code/issues/369) and [#372](https://github.com/Zoo-Code-Org/Zoo-Code/issues/372): planned fan-out keeps a parent live while a child runs and requires completion routing by explicit parent ID, single-writer result readiness, permit release, and orphan cleanup. | Persisted `delegated` status is ownership, not proof that the parent instance is suspended. Completion must route by IDs; scheduler resources and live-instance state need separate invariants. | Nested and sibling lifecycle ownership are covered. Scheduler permits, live/suspended parent selection, orphan cancellation, and single-writer message readiness must be added when fan-out lands; they should not be folded into `HistoryItem` fields prematurely. | | [#1468](https://github.com/Zoo-Code-Org/Zoo-Code/issues/1468): a late chunk from one request combined tool identity with arguments from another request; rerun passed. | Every stream accumulator needs a request/task generation key, and late events cannot mutate another scope. | Separate protocol. The [native tool-call parser request-scope model](./native-tool-call-parser-scoping-model.md), whose source of truth is `scripts/check-native-tool-call-parser-scoping.ts`, exhaustively replays bounded production-parser interleavings without adding fields to this lifecycle model. | -| [#612](https://github.com/Zoo-Code-Org/Zoo-Code/issues/612): the CLI copied a status union and omitted `interrupted`. | Lifecycle vocabulary should have one type owner. | `HistoryItemStatus` is derived from `HistoryItem`, and production/checker transitions share `taskLifecycle.ts`; consumers should import rather than copy the union. | +| [#612](https://github.com/Zoo-Code-Org/Zoo-Code/issues/612): the CLI copied a status union and omitted `interrupted`. | Lifecycle vocabulary should have one type owner. | `HistoryItemStatus` is derived from `HistoryItem`, and production/checker transitions share `taskLifecycle.ts`; consumers should import rather than copy the union. | The issue-derived cases intentionally map to bug classes rather than issue-specific flags. In particular, stale event ownership, monotonic terminal/detached state, explicit scope, and single-writer boundaries generalize to future concurrent task work. diff --git a/package.json b/package.json index 94f2d52e27..a421ef3958 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "check-types": "turbo check-types --log-order grouped --output-logs new-only", "test": "turbo test --log-order grouped --output-logs new-only", "test:mutation-ci": "node --test scripts/stryker-diff.test.mjs", - "lifecycle:model-check": "tsx scripts/check-task-lifecycle.ts && tsx scripts/check-task-store-concurrency.ts && pnpm cleanup-protocol:model-check && pnpm parser-scope:model-check && tsx scripts/check-completion-persistence.ts", + "lifecycle:model-check": "tsx scripts/check-task-lifecycle.ts && tsx scripts/check-task-store-concurrency.ts && tsx scripts/check-provider-handoff-scheduler.ts && pnpm cleanup-protocol:model-check && pnpm parser-scope:model-check && tsx scripts/check-completion-persistence.ts", "cleanup-protocol:model-check": "tsx scripts/check-task-cleanup-protocol.ts", "parser-scope:model-check": "node scripts/run-native-tool-call-parser-scoping.mjs", "test:coverage": "turbo test:coverage --log-order grouped --output-logs new-only", diff --git a/scripts/check-provider-handoff-scheduler.ts b/scripts/check-provider-handoff-scheduler.ts new file mode 100644 index 0000000000..3fbba11b35 --- /dev/null +++ b/scripts/check-provider-handoff-scheduler.ts @@ -0,0 +1,487 @@ +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" + +const PROVIDERS = ["a", "b"] as const +type Provider = (typeof PROVIDERS)[number] +type Generation = 0 | 1 +type PublishedTask = "parent" | `child-${Generation}-${Provider}` + +type Policy = { + name: string + startBeforeCommit?: boolean + resumeBeforePermitRelease?: boolean + redelegateBeforePermitRelease?: boolean + emptyPublication?: boolean + staleConcurrentCommits?: boolean + releaseParentTransitionAfterPublication?: boolean +} + +type ModelState = { + generation: Generation + parent: HistoryItem + children: Partial> + claims: Partial> + prepared?: { provider: Provider; generation: Generation } + commitOwner?: Provider + committedProviders: Provider[] + startedProviders: Provider[] + childPermit: "free" | "held" | "released" + parentQueued: boolean + parentPublished: boolean + parentResumeStarted: boolean + parentResumed: boolean + redelegationOpened: boolean + priorPermitReleased: boolean + parentTransitionOwner?: Generation + pendingParentContinuations: Generation[] + resumedContinuation?: Generation + resumeInvocationOwner?: Generation + resumeInvocationPermitReleased?: boolean + earlyRedelegation: boolean + continuationPublished: boolean + publishedTask?: PublishedTask +} + +type Transition = { name: string; kind: string; next: ModelState } +type TraceStep = { action: string; state: ModelState } + +const MAX_DEPTH = 15 +const MAX_STATES = 20_000 +const EXPECTED_ACTIONS = [ + "claim", + "prepare", + "commit", + "start", + "complete", + "publish-parent", + "release-permit", + "resume-parent", + "settle-parent", + "redelegate", +] as const +const LANDMARKS = { + "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) => + state.commitOwner !== undefined && state.startedProviders.length === 0, + "child-running-with-permit": (state: ModelState) => + state.startedProviders.length === 1 && state.childPermit === "held", + "completed-parent-queued": (state: ModelState) => state.parentQueued && !state.parentPublished, + "parent-published-before-release": (state: ModelState) => state.parentPublished && state.childPermit === "held", + "permit-released-before-resume": (state: ModelState) => + state.childPermit === "released" && !state.parentResumeStarted, + "parent-resume-started": (state: ModelState) => state.parentResumeStarted, + "parent-resumed": (state: ModelState) => state.parentResumed, + "bounded-redelegation": (state: ModelState) => state.generation === 1, + "second-generation-start": (state: ModelState) => state.generation === 1 && state.startedProviders.length === 1, + "resumed-run-with-new-transition": (state: ModelState) => + state.resumedContinuation === 0 && state.parentTransitionOwner === 1, +} satisfies Record boolean> + +const FIXED_POLICY: Policy = { name: "fixed" } +const LEGACY_POLICIES: Array = [ + { name: "start-before-commit", startBeforeCommit: true, expectedViolation: "child started without exact commit" }, + { + name: "resume-before-permit-release", + resumeBeforePermitRelease: true, + expectedViolation: "parent resumed before child permit release", + }, + { + name: "redelegate-before-permit-release", + redelegateBeforePermitRelease: true, + expectedViolation: "parent redelegated before child permit release", + }, + { name: "empty-publication", emptyPublication: true, expectedViolation: "observable current task is empty" }, + { + name: "stale-concurrent-provider-commits", + staleConcurrentCommits: true, + expectedViolation: "multiple provider commits for one parent generation", + }, + { + name: "publication-releases-parent-transition", + releaseParentTransitionAfterPublication: true, + expectedViolation: "stale parent continuation crossed a newer transition", + }, +] + +const parentConfiguration: ProviderSettings = { apiProvider: "anthropic", consecutiveMistakeLimit: 3 } +const savedConfiguration: ProviderSettings = { apiProvider: "openrouter", consecutiveMistakeLimit: 7 } +const parentContext = { mode: "code", apiConfigName: undefined, apiConfiguration: parentConfiguration } +const PROFILE_SCENARIOS = [ + { name: "unsaved", locked: false, saved: undefined, expectedName: undefined, expectedLimit: 3 }, + { + name: "saved", + locked: false, + saved: { name: "ask-profile", apiConfiguration: savedConfiguration }, + expectedName: "ask-profile", + expectedLimit: 7, + }, + { + name: "locked", + locked: true, + saved: { name: "ask-profile", apiConfiguration: savedConfiguration }, + expectedName: undefined, + expectedLimit: 3, + }, +] as const + +for (const scenario of PROFILE_SCENARIOS) { + const selected = selectHandoffExecutionContext( + parentContext, + "ask", + parentContext.mode, + scenario.locked, + scenario.saved, + ) + assert.equal(selected.mode, "ask", `${scenario.name}: requested mode must remain task-local`) + assert.equal(selected.apiConfigName, scenario.expectedName, `${scenario.name}: profile identity`) + assert.equal( + selected.apiConfiguration.consecutiveMistakeLimit, + scenario.expectedLimit, + `${scenario.name}: profile config`, + ) + assert.equal(parentContext.apiConfiguration.consecutiveMistakeLimit, 3, `${scenario.name}: parent context mutated`) +} + +const fixed = explore(FIXED_POLICY, false) +const counterexamples = LEGACY_POLICIES.map((policy) => { + const result = explore(policy, true) + assert.equal(result.violation, policy.expectedViolation, `${policy.name}: unexpected violation`) + assert.ok(result.trace, `${policy.name}: expected a counterexample trace`) + return { name: policy.name, violation: result.violation, trace: result.trace } +}) + +console.log( + `Provider handoff/scheduler model check passed: ${fixed.states} distinct reachable states, ${PROFILE_SCENARIOS.length}/${PROFILE_SCENARIOS.length} profile scenarios, ${fixed.actions.size}/${EXPECTED_ACTIONS.length} actions, ${fixed.landmarks.size}/${Object.keys(LANDMARKS).length} landmarks, depth <= ${MAX_DEPTH}, states <= ${MAX_STATES}, ${counterexamples.length}/${LEGACY_POLICIES.length} legacy counterexamples`, +) +for (const counterexample of counterexamples) { + console.log( + `Legacy counterexample ${counterexample.name}: ${counterexample.violation}\n ${counterexample + .trace!.map((step) => step.action) + .join(" -> ")}`, + ) +} + +function explore( + policy: Policy, + stopAtViolation: boolean, +): { + states: number + actions: Set + landmarks: Set + violation?: string + trace?: TraceStep[] +} { + const start = initialState() + const queue: Array<{ state: ModelState; trace: TraceStep[] }> = [ + { state: start, trace: [{ action: "initial", state: start }] }, + ] + const visited = new Set([canonical(start)]) + const actions = new Set() + const landmarks = new Set() + const frontier: ModelState[] = [] + + for (let index = 0; index < queue.length; index++) { + const node = queue[index]! + for (const [name, predicate] of Object.entries(LANDMARKS)) { + if (predicate(node.state)) landmarks.add(name) + } + const currentViolations = invariantViolations(node.state) + if (currentViolations.length) { + if (stopAtViolation) { + return { states: visited.size, actions, landmarks, violation: currentViolations[0], trace: node.trace } + } + throw new Error(formatViolation(policy, currentViolations, node.trace)) + } + if (node.trace.length - 1 === MAX_DEPTH) { + frontier.push(node.state) + continue + } + + for (const transition of transitions(node.state, policy)) { + actions.add(transition.kind) + const trace = [...node.trace, { action: transition.name, state: transition.next }] + const violations = invariantViolations(transition.next) + if (violations.length) { + if (stopAtViolation) { + return { states: visited.size, actions, landmarks, violation: violations[0], trace } + } + throw new Error(formatViolation(policy, violations, trace)) + } + const key = canonical(transition.next) + if (visited.has(key)) continue + visited.add(key) + queue.push({ state: transition.next, trace }) + if (visited.size > MAX_STATES) { + throw new Error(`${policy.name}: exceeded ${MAX_STATES}-state budget`) + } + } + } + + if (stopAtViolation) throw new Error(`${policy.name}: expected counterexample was not found`) + const missingActions = EXPECTED_ACTIONS.filter((action) => !actions.has(action)) + if (missingActions.length) throw new Error(`Fixed model has unreachable actions: ${missingActions.join(", ")}`) + const missingLandmarks = Object.keys(LANDMARKS).filter((name) => !landmarks.has(name)) + if (missingLandmarks.length) + throw new Error(`Fixed model has unreachable landmarks: ${missingLandmarks.join(", ")}`) + const unseen = frontier + .flatMap((state) => transitions(state, policy)) + .find(({ next }) => !visited.has(canonical(next))) + if (unseen) { + throw new Error(`Fixed model reached depth ${MAX_DEPTH} with unseen successor ${unseen.name}`) + } + return { states: visited.size, actions, landmarks } +} + +function transitions(state: ModelState, policy: Policy): Transition[] { + const result: Transition[] = [] + for (const provider of PROVIDERS) { + if (!state.parentQueued && !state.claims[provider] && state.commitOwner === undefined) { + result.push( + action(`claim(${provider}, g${state.generation})`, "claim", state, (next) => { + next.claims[provider] = { generation: state.generation, snapshot: structuredClone(state.parent) } + }), + ) + } + const claim = state.claims[provider] + if ( + claim?.generation === state.generation && + !state.parentQueued && + state.prepared === undefined && + (state.commitOwner === undefined || policy.staleConcurrentCommits) + ) { + result.push( + action(`prepare(${provider}, g${state.generation})`, "prepare", state, (next) => { + next.prepared = { provider, generation: state.generation } + const childId = childIdFor(state.generation, provider) + next.children[childId] = task(childId, "active", "parent") + next.publishedTask = policy.emptyPublication ? undefined : childId + }), + ) + } + if (claim?.generation === state.generation && state.prepared?.provider === provider) { + const mayCommit = !state.parentQueued && (state.commitOwner === undefined || policy.staleConcurrentCommits) + if (mayCommit && (!state.committedProviders.includes(provider) || state.commitOwner === undefined)) { + result.push( + action(`commit(${provider}, g${state.generation})`, "commit", state, (next) => { + const base = policy.staleConcurrentCommits ? claim.snapshot : state.parent + next.parent = delegateTaskToChild(base, childIdFor(state.generation, provider)) + next.commitOwner = provider + next.committedProviders = [...state.committedProviders, provider] + next.parentTransitionOwner = state.generation + next.prepared = undefined + }), + ) + } + } + const exactCommit = + state.commitOwner === provider && state.parent.awaitingChildId === childIdFor(state.generation, provider) + if ( + state.children[childIdFor(state.generation, provider)] !== undefined && + state.startedProviders.length === 0 && + (exactCommit || policy.startBeforeCommit) + ) { + result.push( + action(`start(${provider}, g${state.generation})`, "start", state, (next) => { + next.startedProviders = [provider] + next.childPermit = "held" + }), + ) + } + } + + if (state.generation === 0 && state.commitOwner && state.startedProviders.includes(state.commitOwner)) { + const childId = childIdFor(0, state.commitOwner) + const child = state.children[childId] + if (child?.status === "active" && state.parent.status === "delegated") { + result.push( + action("complete-child", "complete", state, (next) => { + const completed = completeDelegatedChild(state.parent, child, "done") + next.parent = completed.parent + next.children[childId] = completed.child + next.parentQueued = true + next.parentTransitionOwner = 0 + next.pendingParentContinuations = [0] + }), + ) + } + } + if (state.parentQueued && !state.parentPublished) { + result.push( + action("publish-parent", "publish-parent", state, (next) => { + next.parentPublished = true + next.continuationPublished = true + next.publishedTask = "parent" + if (policy.releaseParentTransitionAfterPublication) next.parentTransitionOwner = undefined + }), + ) + } + if ( + state.childPermit === "held" && + ((state.parentQueued && !policy.releaseParentTransitionAfterPublication) || + (policy.releaseParentTransitionAfterPublication && + state.generation === 1 && + state.commitOwner !== undefined)) + ) { + result.push( + action("release-child-permit", "release-permit", state, (next) => { + next.childPermit = "released" + }), + ) + } + const pendingContinuation = state.pendingParentContinuations[0] + if ( + pendingContinuation !== undefined && + state.continuationPublished && + !state.parentResumeStarted && + (state.childPermit === "released" || policy.resumeBeforePermitRelease) + ) { + result.push( + action(`resume-parent(g${pendingContinuation})`, "resume-parent", state, (next) => { + next.parentResumeStarted = true + next.resumedContinuation = pendingContinuation + next.resumeInvocationOwner = state.parentTransitionOwner + next.resumeInvocationPermitReleased = state.childPermit === "released" + next.pendingParentContinuations = state.pendingParentContinuations.slice(1) + if (state.parentTransitionOwner === pendingContinuation) next.parentTransitionOwner = undefined + }), + ) + } + if (state.resumedContinuation !== undefined) { + result.push( + action(`settle-parent(g${state.resumedContinuation})`, "settle-parent", state, (next) => { + next.parentResumed = true + next.resumedContinuation = undefined + next.resumeInvocationOwner = undefined + }), + ) + } + if ( + state.generation === 0 && + !state.redelegationOpened && + ((state.parentResumeStarted && state.childPermit === "released") || + (policy.releaseParentTransitionAfterPublication && + state.parentPublished && + state.parentTransitionOwner === undefined) || + (policy.redelegateBeforePermitRelease && + state.parentQueued && + state.parentPublished && + state.childPermit === "held")) + ) { + result.push( + action("redelegate(g1)", "redelegate", state, (next) => { + next.generation = 1 + next.claims = {} + next.prepared = undefined + next.commitOwner = undefined + next.committedProviders = [] + next.startedProviders = [] + next.childPermit = policy.releaseParentTransitionAfterPublication ? state.childPermit : "free" + next.parentQueued = false + next.parentPublished = false + next.redelegationOpened = true + next.priorPermitReleased = state.childPermit === "released" + next.earlyRedelegation = + state.childPermit !== "released" && policy.releaseParentTransitionAfterPublication === true + }), + ) + } + return result +} + +function invariantViolations(state: ModelState): string[] { + const violations: string[] = [] + if (!state.publishedTask) 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) { + if (state.commitOwner !== provider || state.parent.awaitingChildId !== childIdFor(state.generation, provider)) { + violations.push("child started without exact commit") + } + } + if (state.parentResumeStarted && !state.resumeInvocationPermitReleased) { + violations.push("parent resumed before child permit release") + } + if (state.generation === 1 && !state.priorPermitReleased) { + if (!state.earlyRedelegation) violations.push("parent redelegated before child permit release") + } + if (state.resumedContinuation !== undefined && state.resumedContinuation !== state.resumeInvocationOwner) { + violations.push("stale parent continuation crossed a newer transition") + } + if (state.parentPublished && state.publishedTask !== "parent") { + violations.push("published parent does not match current task") + } + if (state.parentQueued) { + const completedChildId = state.parent.completedByChildId as PublishedTask | undefined + if ( + !completedChildId || + state.children[completedChildId]?.status !== "completed" || + state.parent.status !== "active" + ) { + violations.push("final child/parent publication is inconsistent") + } + } + return violations +} + +function initialState(): ModelState { + return { + generation: 0, + parent: task("parent", "active"), + children: {}, + claims: {}, + committedProviders: [], + startedProviders: [], + childPermit: "free", + parentQueued: false, + parentPublished: false, + parentResumeStarted: false, + parentResumed: false, + redelegationOpened: false, + priorPermitReleased: false, + pendingParentContinuations: [], + earlyRedelegation: false, + continuationPublished: false, + publishedTask: "parent", + } +} + +function action(name: string, kind: string, state: ModelState, update: (next: ModelState) => void): Transition { + const next = structuredClone(state) + update(next) + return { name, kind, next } +} + +function childIdFor(generation: Generation, provider: Provider): `child-${Generation}-${Provider}` { + return `child-${generation}-${provider}` +} + +function canonical(state: ModelState): string { + return JSON.stringify({ + ...state, + children: Object.fromEntries( + Object.entries(state.children).sort(([left], [right]) => left.localeCompare(right)), + ), + claims: Object.fromEntries( + PROVIDERS.flatMap((provider) => (state.claims[provider] ? [[provider, state.claims[provider]]] : [])), + ), + committedProviders: [...state.committedProviders].sort(), + startedProviders: [...state.startedProviders].sort(), + }) +} + +function formatViolation(policy: Policy, violations: string[], trace: TraceStep[]): string { + return [ + `${policy.name}: ${violations.join("; ")}`, + `Bounds: depth=${MAX_DEPTH}, states=${MAX_STATES}`, + ...trace.map((step, index) => `${index}. ${step.action}\n ${canonical(step.state)}`), + ].join("\n") +} + +function task(id: string, status: HistoryItem["status"], parentTaskId?: string): HistoryItem { + return { id, status, parentTaskId, task: id, ts: 1, tokensIn: 0, tokensOut: 0, totalCost: 0 } +} diff --git a/src/__tests__/provider-delegation.spec.ts b/src/__tests__/ClineProvider.delegation.spec.ts similarity index 68% rename from src/__tests__/provider-delegation.spec.ts rename to src/__tests__/ClineProvider.delegation.spec.ts index 0b7aef8775..422c264e2c 100644 --- a/src/__tests__/provider-delegation.spec.ts +++ b/src/__tests__/ClineProvider.delegation.spec.ts @@ -2,7 +2,7 @@ import { describe, it, expect, vi } from "vitest" import type { HistoryItem } from "@roo-code/types" -import { RooCodeEventName } from "@roo-code/types" +import { providerIdentifiers, RooCodeEventName } from "@roo-code/types" import { ClineProvider } from "../core/webview/ClineProvider" import { TaskScheduler } from "../core/task/TaskScheduler" @@ -20,6 +20,7 @@ function makeStoreStub( overrides: Partial<{ atomicReadAndUpdate: ReturnType; get: ReturnType }> = {}, ) { return { + invalidate: vi.fn().mockResolvedValue(undefined), atomicReadAndUpdate: vi.fn(async (_taskId: string, updater: (h: HistoryItem) => HistoryItem) => { updater(parentHistoryItem) return [] @@ -37,6 +38,9 @@ function makeStoreStub( const makeParentTask = () => ({ taskId: "parent-1", + apiConfiguration: { apiProvider: providerIdentifiers.anthropic, anthropicApiKey: "task-local-key" }, + getTaskMode: vi.fn().mockResolvedValue("code"), + getTaskApiConfigName: vi.fn().mockResolvedValue("task-local-profile"), emit: vi.fn(), flushPendingToolResultsToHistory: vi.fn().mockResolvedValue(true), retrySaveApiConversationHistory: vi.fn(), @@ -97,6 +101,7 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => { } let current: HistoryItem = { ...parentHistoryItem, status: "active", pendingAction } const taskHistoryStore = { + invalidate: vi.fn().mockResolvedValue(undefined), get: vi.fn(() => current), atomicReadAndUpdate: vi.fn(async (_taskId: string, updater: (item: HistoryItem) => HistoryItem) => { current = updater(current) @@ -223,6 +228,14 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => { // Child task created with startTask: false and initialStatus: "active" expect(createTask).toHaveBeenCalledWith("Do something", undefined, parentTask, { + handoffExecutionContext: { + apiConfigName: "task-local-profile", + apiConfiguration: { + apiProvider: providerIdentifiers.anthropic, + anthropicApiKey: "task-local-key", + }, + mode: "code", + }, initialTodos: [], initialStatus: "active", startTask: false, @@ -249,8 +262,131 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => { // Provider-level event expect(providerEmit).toHaveBeenCalledWith(RooCodeEventName.TaskDelegated, "parent-1", "child-1") - // Mode switch - expect(handleModeSwitch).toHaveBeenCalledWith("code") + expect(handleModeSwitch).not.toHaveBeenCalled() + }) + + it("uses an explicitly saved different-mode profile without reading shared current identity", async () => { + const parentTask = makeParentTask() + const child = { taskId: "child-ask", run: vi.fn().mockResolvedValue(undefined) } + const createTask = vi.fn().mockResolvedValue(child) + const providerSettingsManager = { + getModeConfigId: vi.fn().mockResolvedValue("ask-profile-id"), + getProfile: vi.fn().mockResolvedValue({ + name: "ask-profile", + id: "ask-profile-id", + apiProvider: providerIdentifiers.openrouter, + openRouterModelId: "openai/gpt-4.1-mini", + }), + getCurrentProfileName: vi.fn(), + } + const workspaceGet = vi.fn().mockReturnValue(false) + const provider = { + taskScheduler: new TaskScheduler(), + emit: vi.fn(), + getCurrentTask: vi.fn(() => parentTask), + removeClineFromStack: vi.fn().mockResolvedValue(undefined), + createTask, + log: vi.fn(), + isViewLaunched: false, + taskHistoryStore: makeStoreStub(), + providerSettingsManager, + context: { workspaceState: { get: workspaceGet } }, + } as unknown as ClineProvider + + await ClineProvider.prototype.delegateParentAndOpenChild.call(provider, { + parentTaskId: "parent-1", + message: "Ask child", + initialTodos: [], + mode: "ask", + }) + + expect(providerSettingsManager.getModeConfigId).toHaveBeenCalledWith("ask") + expect(workspaceGet).toHaveBeenCalledWith("lockApiConfigAcrossModes", false) + expect(providerSettingsManager.getProfile).toHaveBeenCalledWith({ id: "ask-profile-id" }) + expect(providerSettingsManager.getCurrentProfileName).not.toHaveBeenCalled() + expect(createTask).toHaveBeenCalledWith( + "Ask child", + undefined, + parentTask, + expect.objectContaining({ + handoffExecutionContext: { + mode: "ask", + apiConfigName: "ask-profile", + apiConfiguration: { + apiProvider: providerIdentifiers.openrouter, + openRouterModelId: "openai/gpt-4.1-mini", + }, + }, + }), + ) + }) + + it.each([ + { name: "has no saved mode profile", savedConfigId: undefined, savedProfile: undefined }, + { + name: "has an unconfigured saved mode profile", + savedConfigId: "empty-id", + savedProfile: { name: "empty", id: "empty-id" }, + }, + { + name: "has a stale saved mode profile", + savedConfigId: "stale-id", + savedProfile: new Error("profile not found"), + }, + ])("keeps the parent task-local profile when a different mode $name", async ({ savedConfigId, savedProfile }) => { + const parentTask = makeParentTask() + const child = { taskId: "child-fallback", run: vi.fn().mockResolvedValue(undefined) } + const createTask = vi.fn().mockResolvedValue(child) + const getProfile = + savedProfile instanceof Error + ? vi.fn().mockRejectedValue(savedProfile) + : vi.fn().mockResolvedValue(savedProfile) + const log = vi.fn() + const provider = { + taskScheduler: new TaskScheduler(), + emit: vi.fn(), + getCurrentTask: vi.fn(() => parentTask), + removeClineFromStack: vi.fn().mockResolvedValue(undefined), + createTask, + log, + isViewLaunched: false, + taskHistoryStore: makeStoreStub(), + providerSettingsManager: { + getModeConfigId: vi.fn().mockResolvedValue(savedConfigId), + getProfile, + }, + context: { workspaceState: { get: vi.fn().mockReturnValue(false) } }, + } as unknown as ClineProvider + + await ClineProvider.prototype.delegateParentAndOpenChild.call(provider, { + parentTaskId: "parent-1", + message: "Fallback child", + initialTodos: [], + mode: "ask", + }) + + if (savedConfigId) expect(getProfile).toHaveBeenCalledWith({ id: savedConfigId }) + else expect(getProfile).not.toHaveBeenCalled() + if (savedProfile instanceof Error) { + expect( + log.mock.calls.some(([message]) => message.includes("stale-id") && message.includes("parent parent-1")), + ).toBe(true) + } + expect(createTask).toHaveBeenCalledWith( + "Fallback child", + undefined, + parentTask, + expect.objectContaining({ + handoffExecutionContext: { + mode: "ask", + apiConfigName: "task-local-profile", + apiConfiguration: { + apiProvider: providerIdentifiers.anthropic, + anthropicApiKey: "task-local-key", + }, + }, + }), + ) }) it("posts taskHistoryItemUpdated to the webview when isViewLaunched is true", async () => { @@ -258,7 +394,7 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => { const postMessageToWebview = vi.fn().mockResolvedValue(undefined) const parentTask = makeParentTask() const taskHistoryStore = makeStoreStub({ - get: vi.fn().mockReturnValue(updatedParent), + get: vi.fn().mockReturnValueOnce(parentHistoryItem).mockReturnValue(updatedParent), }) const provider = { @@ -468,11 +604,100 @@ describe("ClineProvider.delegateParentAndOpenChild()", () => { initialTodos: [], mode: "code", }), - ).rejects.toThrow("Cannot re-delegate") + ).rejects.toThrow("Cannot re-delegate while the awaited child is not interrupted") - // Rollback: child must not have run, and must be cleaned up + // The authoritative preflight rejects before either provider mutates its stack. expect(child.run).not.toHaveBeenCalled() - expect((provider as any).deleteTaskWithId).toHaveBeenCalledWith("child-2", false) + expect(createTask).not.toHaveBeenCalled() + expect((provider as any).deleteTaskWithId).not.toHaveBeenCalled() + }) + + it("rejects a delegated parent whose awaited-child identity is missing", async () => { + const parentTask = makeParentTask() + const taskHistoryStore = makeStoreStub({ + get: vi.fn().mockReturnValue({ ...parentHistoryItem, status: "delegated" }), + }) + const provider = { + getCurrentTask: vi.fn(() => parentTask), + removeClineFromStack: vi.fn(), + createTask: vi.fn(), + taskHistoryStore, + } as unknown as ClineProvider + + await expect( + ClineProvider.prototype.delegateParentAndOpenChild.call(provider, { + parentTaskId: "parent-1", + message: "Continue", + initialTodos: [], + mode: "code", + }), + ).rejects.toThrow("Cannot re-delegate a parent with no awaited child") + expect(provider.removeClineFromStack).not.toHaveBeenCalled() + }) + + it("serializes same-parent delegation across provider instances and starts only one child", async () => { + let durableParent = { ...parentHistoryItem, status: "active" as const } + let releaseCommit!: () => void + let markCommitStarted!: () => void + const commitStarted = new Promise((resolve) => { + markCommitStarted = resolve + }) + const commitMayFinish = new Promise((resolve) => { + releaseCommit = resolve + }) + + const makeProvider = (childId: string) => { + const parent = makeParentTask() + const child = { taskId: childId, run: vi.fn().mockResolvedValue(undefined) } + const removeClineFromStack = vi.fn().mockResolvedValue(undefined) + const store = { + invalidate: vi.fn().mockResolvedValue(undefined), + get: vi.fn((id: string) => (id === parent.taskId ? durableParent : undefined)), + atomicReadAndUpdate: vi.fn(async (_id: string, updater: (item: HistoryItem) => HistoryItem) => { + markCommitStarted() + await commitMayFinish + durableParent = updater(durableParent) as typeof durableParent + return [durableParent] + }), + } + const provider = { + taskScheduler: new TaskScheduler(), + emit: vi.fn(), + getCurrentTask: vi.fn(() => parent), + removeClineFromStack, + createTask: vi.fn().mockResolvedValue(child), + log: vi.fn(), + isViewLaunched: false, + taskHistoryStore: store, + } as unknown as ClineProvider + return { provider, child, removeClineFromStack } + } + + const first = makeProvider("child-1") + const second = makeProvider("child-2") + const firstDelegation = ClineProvider.prototype.delegateParentAndOpenChild.call(first.provider, { + parentTaskId: "parent-1", + message: "First", + initialTodos: [], + mode: "code", + }) + await commitStarted + const secondDelegation = ClineProvider.prototype.delegateParentAndOpenChild.call(second.provider, { + parentTaskId: "parent-1", + message: "Second", + initialTodos: [], + mode: "ask", + }) + + expect(second.removeClineFromStack).not.toHaveBeenCalled() + releaseCommit() + await expect(firstDelegation).resolves.toBe(first.child) + await expect(secondDelegation).rejects.toThrow("Cannot re-delegate") + await Promise.resolve() + + expect(first.child.run).toHaveBeenCalledOnce() + expect(second.child.run).not.toHaveBeenCalled() + expect(durableParent.awaitingChildId).toBe("child-1") }) it("rolls back the paused child and restores the parent when atomicReadAndUpdate fails", async () => { diff --git a/src/__tests__/history-resume-delegation.spec.ts b/src/__tests__/ClineProvider.history-resume-delegation.spec.ts similarity index 74% rename from src/__tests__/history-resume-delegation.spec.ts rename to src/__tests__/ClineProvider.history-resume-delegation.spec.ts index eed8127b82..f50dada0d3 100644 --- a/src/__tests__/history-resume-delegation.spec.ts +++ b/src/__tests__/ClineProvider.history-resume-delegation.spec.ts @@ -1,10 +1,12 @@ // npx vitest run __tests__/history-resume-delegation.spec.ts import { describe, it, expect, vi, beforeEach } from "vitest" -import { RooCodeEventName } from "@roo-code/types" +import * as vscode from "vscode" +import { providerIdentifiers, RooCodeEventName } from "@roo-code/types" import type { ClineMessage, HistoryItem } from "@roo-code/types" import type { ApiMessage } from "../core/task-persistence" +import type { Task } from "../core/task/Task" /* vscode mock for Task/Provider imports */ vi.mock("vscode", () => { @@ -77,8 +79,8 @@ function makeTaskHistoryStoreStub( firstUpdater: (h: HistoryItem) => HistoryItem, secondUpdater: (h: HistoryItem) => HistoryItem, ) => { - firstUpdater(itemMap.get(firstId) as HistoryItem) - secondUpdater(itemMap.get(secondId) as HistoryItem) + itemMap.set(firstId, firstUpdater(itemMap.get(firstId) as HistoryItem)) + itemMap.set(secondId, secondUpdater(itemMap.get(secondId) as HistoryItem)) return [] }, ) @@ -86,6 +88,38 @@ function makeTaskHistoryStoreStub( return { atomicUpdatePair: overrides.atomicUpdatePair ?? atomicUpdatePair, get: vi.fn((id: string) => itemMap.get(id)), + invalidate: vi.fn().mockResolvedValue(undefined), + } +} + +function makeStatefulTaskHistoryStore(...items: HistoryItem[]) { + const itemMap = new Map(items.map((item) => [item.id, item])) + + return { + get: vi.fn((id: string) => itemMap.get(id)), + invalidate: vi.fn().mockResolvedValue(undefined), + atomicUpdatePair: vi.fn( + async ( + firstId: string, + secondId: string, + firstUpdater: (item: HistoryItem) => HistoryItem, + secondUpdater: (item: HistoryItem) => HistoryItem, + ) => { + const first = itemMap.get(firstId) + const second = itemMap.get(secondId) + if (!first || !second) throw new Error(`Missing history item for atomic pair: ${firstId}, ${secondId}`) + itemMap.set(firstId, firstUpdater(first)) + itemMap.set(secondId, secondUpdater(second)) + return [itemMap.get(firstId), itemMap.get(secondId)] + }, + ), + atomicReadAndUpdate: vi.fn(async (id: string, updater: (item: HistoryItem) => HistoryItem) => { + const item = itemMap.get(id) + if (!item) throw new Error(`Missing history item: ${id}`) + const updated = updater(item) + itemMap.set(id, updated) + return [updated] + }), } } @@ -683,6 +717,7 @@ describe("History resume delegation - parent metadata transitions", () => { it("reopenParentFromDelegation sets skipPrevResponseIdOnce via resumeAfterDelegation", async () => { const parentInstance: any = { + taskId: "parent-2", skipPrevResponseIdOnce: false, resumeAfterDelegation: vi.fn().mockImplementation(async function (this: any) { // Simulate what the real resumeAfterDelegation does @@ -704,13 +739,16 @@ describe("History resume delegation - parent metadata transitions", () => { totalCost: 0, } const taskHistoryStore = makeTaskHistoryStoreStub({ id: "child-2", status: "active" }, parentItem) + let currentTask: object | undefined = { taskId: "child-2" } const provider = makeProviderStub({ contextProxy: { globalStorageUri: { fsPath: "/tmp" } }, getTaskWithId: vi.fn().mockResolvedValue({ historyItem: parentItem }), emit: vi.fn(), - getCurrentTask: vi.fn(() => ({ taskId: "child-2" })), - removeClineFromStack: vi.fn().mockResolvedValue(undefined), - createTaskWithHistoryItem: vi.fn().mockResolvedValue(parentInstance), + getCurrentTask: vi.fn(() => currentTask), + removeClineFromStack: vi.fn(async () => { + currentTask = undefined + }), + createTaskWithHistoryItem: vi.fn(async () => (currentTask = parentInstance)), taskHistoryStore, } as any) @@ -722,6 +760,7 @@ describe("History resume delegation - parent metadata transitions", () => { childTaskId: "child-2", completionResultSummary: "Done", }) + await vi.waitFor(() => expect(parentInstance.resumeAfterDelegation).toHaveBeenCalledTimes(1)) // Critical: verify skipPrevResponseIdOnce set to true by resumeAfterDelegation expect(parentInstance.skipPrevResponseIdOnce).toBe(true) @@ -743,17 +782,22 @@ describe("History resume delegation - parent metadata transitions", () => { } const taskHistoryStore = makeTaskHistoryStoreStub({ id: "c3", status: "active" }, parentItem) + const parentInstance = { + taskId: "p3", + resumeAfterDelegation: vi.fn().mockResolvedValue(undefined), + overwriteClineMessages: vi.fn().mockResolvedValue(undefined), + overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined), + } + let currentTask: object | undefined = { taskId: "c3" } const provider = makeProviderStub({ contextProxy: { globalStorageUri: { fsPath: "/tmp" } }, getTaskWithId: vi.fn().mockResolvedValue({ historyItem: parentItem }), emit: emitSpy, - getCurrentTask: vi.fn(() => ({ taskId: "c3" })), - removeClineFromStack: vi.fn().mockResolvedValue(undefined), - createTaskWithHistoryItem: vi.fn().mockResolvedValue({ - resumeAfterDelegation: vi.fn().mockResolvedValue(undefined), - overwriteClineMessages: vi.fn().mockResolvedValue(undefined), - overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined), + getCurrentTask: vi.fn(() => currentTask), + removeClineFromStack: vi.fn(async () => { + currentTask = undefined }), + createTaskWithHistoryItem: vi.fn(async () => (currentTask = parentInstance)), taskHistoryStore, } as any) @@ -765,6 +809,7 @@ describe("History resume delegation - parent metadata transitions", () => { childTaskId: "c3", completionResultSummary: "Summary", }) + await vi.waitFor(() => expect(parentInstance.resumeAfterDelegation).toHaveBeenCalledTimes(1)) // Verify both events emitted const eventNames = emitSpy.mock.calls.map((c) => c[0]) @@ -786,6 +831,7 @@ describe("History resume delegation - parent metadata transitions", () => { it("reopenParentFromDelegation continues when overwrite operations fail and still resumes/emits (RPD-06)", async () => { const emitSpy = vi.fn() const parentInstance = { + taskId: "parent-rpd06", resumeAfterDelegation: vi.fn().mockResolvedValue(undefined), overwriteClineMessages: vi.fn().mockRejectedValue(new Error("ui overwrite failed")), overwriteApiConversationHistory: vi.fn().mockRejectedValue(new Error("api overwrite failed")), @@ -804,13 +850,16 @@ describe("History resume delegation - parent metadata transitions", () => { } const taskHistoryStore = makeTaskHistoryStoreStub({ id: "child-rpd06", status: "active" }, parentItem) + let currentTask: object | undefined = { taskId: "child-rpd06" } const provider = makeProviderStub({ contextProxy: { globalStorageUri: { fsPath: "/tmp" } }, getTaskWithId: vi.fn().mockResolvedValue({ historyItem: parentItem }), emit: emitSpy, - getCurrentTask: vi.fn(() => ({ taskId: "child-rpd06" })), - removeClineFromStack: vi.fn().mockResolvedValue(undefined), - createTaskWithHistoryItem: vi.fn().mockResolvedValue(parentInstance), + getCurrentTask: vi.fn(() => currentTask), + removeClineFromStack: vi.fn(async () => { + currentTask = undefined + }), + createTaskWithHistoryItem: vi.fn(async () => (currentTask = parentInstance)), taskHistoryStore, } as any) @@ -824,6 +873,7 @@ describe("History resume delegation - parent metadata transitions", () => { completionResultSummary: "Subtask finished despite overwrite failures", }), ).resolves.toBe(true) + await vi.waitFor(() => expect(parentInstance.resumeAfterDelegation).toHaveBeenCalledTimes(1)) expect(parentInstance.overwriteClineMessages).toHaveBeenCalledTimes(1) expect(parentInstance.overwriteApiConversationHistory).toHaveBeenCalledTimes(1) @@ -843,6 +893,144 @@ describe("History resume delegation - parent metadata transitions", () => { expect(resumedIdx).toBeGreaterThan(completedIdx) }) + it("keeps a failed scheduled parent resume visible and resumable without emitting resumed success", async () => { + const resumeError = new Error("provider stream failed") + const emitSpy = vi.fn() + const log = vi.fn() + const parentInstance = { + taskId: "parent-resume-failure", + resumeAfterDelegation: vi.fn().mockRejectedValue(resumeError), + overwriteClineMessages: vi.fn().mockResolvedValue(undefined), + overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined), + } + const parentItem = { + id: "parent-resume-failure", + status: "delegated", + awaitingChildId: "child-resume-failure", + childIds: ["child-resume-failure"], + ts: 900, + task: "Parent resume failure", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + } + const taskHistoryStore = makeTaskHistoryStoreStub({ id: "child-resume-failure", status: "active" }, parentItem) + let scheduled: Promise | undefined + const provider = makeProviderStub({ + contextProxy: { globalStorageUri: { fsPath: "/tmp" } }, + getTaskWithId: vi.fn().mockResolvedValue({ historyItem: parentItem }), + emit: emitSpy, + log, + getCurrentTask: vi.fn(() => parentInstance), + removeClineFromStack: vi.fn().mockResolvedValue(undefined), + createTaskWithHistoryItem: vi.fn().mockResolvedValue(parentInstance), + taskScheduler: { + schedule: vi.fn((_task, run) => { + scheduled = run() + return scheduled + }), + }, + taskHistoryStore, + }) + + vi.mocked(readTaskMessages).mockResolvedValue([]) + vi.mocked(readApiMessages).mockResolvedValue([]) + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}) + + await expect( + ClineProvider.prototype.reopenParentFromDelegation.call(provider, { + parentTaskId: "parent-resume-failure", + childTaskId: "child-resume-failure", + completionResultSummary: "Child completed", + }), + ).resolves.toBe(true) + await expect(scheduled).rejects.toThrow(resumeError) + + expect(log).toHaveBeenCalledWith(expect.stringContaining("provider stream failed")) + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + expect.stringContaining("Open the task from history to retry"), + ) + expect(emitSpy).not.toHaveBeenCalledWith( + RooCodeEventName.TaskDelegationResumed, + "parent-resume-failure", + "child-resume-failure", + ) + expect(parentInstance.taskId).toBe("parent-resume-failure") + consoleError.mockRestore() + }) + + it("releases the shared parent queue when scheduler admission rejects", async () => { + const scheduleError = new Error("scheduler unavailable") + const parentInstance = { + taskId: "parent-scheduler-rejection", + resumeAfterDelegation: vi.fn().mockResolvedValue(undefined), + overwriteClineMessages: vi.fn().mockResolvedValue(undefined), + overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined), + } + const parentItem = { + id: parentInstance.taskId, + status: "delegated", + awaitingChildId: "child-scheduler-rejection", + childIds: ["child-scheduler-rejection"], + ts: 901, + task: "Parent scheduler rejection", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + } + const taskHistoryStore = makeTaskHistoryStoreStub( + { id: "child-scheduler-rejection", status: "active" }, + parentItem, + ) + const emit = vi.fn() + const provider = makeProviderStub({ + contextProxy: { globalStorageUri: { fsPath: "/tmp" } }, + getTaskWithId: vi.fn().mockResolvedValue({ historyItem: parentItem }), + emit, + getCurrentTask: vi.fn(() => parentInstance), + removeClineFromStack: vi.fn().mockResolvedValue(undefined), + createTaskWithHistoryItem: vi.fn().mockResolvedValue(parentInstance), + taskScheduler: { + schedule: vi.fn().mockRejectedValue(scheduleError), + }, + taskHistoryStore, + }) + const laterTransition = vi.fn() + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}) + + vi.mocked(readTaskMessages).mockResolvedValue([]) + vi.mocked(readApiMessages).mockResolvedValue([]) + await expect( + ClineProvider.prototype.reopenParentFromDelegation.call(provider, { + parentTaskId: parentInstance.taskId, + childTaskId: "child-scheduler-rejection", + completionResultSummary: "Child completed", + }), + ).resolves.toBe(true) + await ( + ClineProvider.prototype as unknown as { + runDelegationTransition: ( + this: ClineProvider, + parentTaskId: string, + fn: () => Promise, + ) => Promise + } + ).runDelegationTransition.call(provider, parentInstance.taskId, async () => laterTransition()) + + expect(parentInstance.resumeAfterDelegation).not.toHaveBeenCalled() + expect(laterTransition).toHaveBeenCalledTimes(1) + expect(emit).not.toHaveBeenCalledWith( + RooCodeEventName.TaskDelegationResumed, + parentInstance.taskId, + "child-scheduler-rejection", + ) + expect(consoleError).toHaveBeenCalledWith( + `[reopenParentFromDelegation] taskScheduler.schedule failed:`, + scheduleError, + ) + consoleError.mockRestore() + }) + it("reopenParentFromDelegation does NOT emit TaskPaused or TaskUnpaused (new flow only)", async () => { const emitSpy = vi.fn() const parentItem = { @@ -888,7 +1076,7 @@ describe("History resume delegation - parent metadata transitions", () => { expect(eventNames).not.toContain(RooCodeEventName.TaskSpawned) }) - it("reopenParentFromDelegation skips child close when current task differs and still reopens parent (RPD-02)", async () => { + it("reopenParentFromDelegation skips stale resume when another task remains current (RPD-02)", async () => { const parentInstance = { resumeAfterDelegation: vi.fn().mockResolvedValue(undefined), overwriteClineMessages: vi.fn().mockResolvedValue(undefined), @@ -949,9 +1137,312 @@ describe("History resume delegation - parent metadata transitions", () => { }), { startTask: false }, ) + await vi.waitFor(() => expect(taskHistoryStore.invalidate).toHaveBeenCalledWith("parent-rpd02")) + expect(parentInstance.resumeAfterDelegation).not.toHaveBeenCalled() + }) + + it("serializes real cross-provider completion and redelegation through resume invocation", async () => { + let releaseResume!: () => void + const resumeBlocked = new Promise((resolve) => { + releaseResume = resolve + }) + let releaseChildRun!: () => void + const childRunBlocked = new Promise((resolve) => { + releaseChildRun = resolve + }) + const parentInstanceA = { + taskId: "parent-shared-transition", + abort: false, + abandoned: false, + resumeAfterDelegation: vi.fn(() => resumeBlocked), + overwriteClineMessages: vi.fn().mockResolvedValue(undefined), + overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined), + } + const parentItem = { + id: parentInstanceA.taskId, + status: "delegated", + awaitingChildId: "child-c1", + delegatedToId: "child-c1", + childIds: ["child-c1"], + ts: 1, + task: "Parent", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + mode: "code", + } as HistoryItem + const taskHistoryStore = makeStatefulTaskHistoryStore(parentItem, { + id: "child-c1", + status: "active", + parentTaskId: parentItem.id, + } as HistoryItem) + let currentTaskA: object | undefined = { taskId: "child-c1" } + let runScheduledContinuation!: () => Promise + let settleScheduledContinuation!: () => void + const scheduledContinuationSettled = new Promise((resolve) => { + settleScheduledContinuation = resolve + }) + let scheduledContinuation: Promise | undefined + const emitA = vi.fn() + const providerA = makeProviderStub({ + contextProxy: { globalStorageUri: { fsPath: "/tmp" } }, + getTaskWithId: vi.fn(async (id: string) => ({ historyItem: taskHistoryStore.get(id)! })), + emit: emitA, + getCurrentTask: vi.fn(() => currentTaskA), + removeClineFromStack: vi.fn(async () => { + currentTaskA = undefined + }), + createTaskWithHistoryItem: vi.fn(async () => (currentTaskA = parentInstanceA)), + taskScheduler: { + schedule: vi.fn((_task, run) => { + runScheduledContinuation = () => (scheduledContinuation ??= run()) + return scheduledContinuationSettled + }), + }, + taskHistoryStore, + }) + const parentInstanceB = { + taskId: parentItem.id, + apiConfiguration: { apiProvider: providerIdentifiers.anthropic, anthropicApiKey: "task-local-key" }, + getTaskMode: vi.fn().mockResolvedValue("code"), + getTaskApiConfigName: vi.fn().mockResolvedValue("task-local-profile"), + flushPendingToolResultsToHistory: vi.fn().mockResolvedValue(true), + retrySaveApiConversationHistory: vi.fn(), + } + const childC2 = { + taskId: "child-c2", + run: vi.fn(async () => { + expect(taskHistoryStore.get(parentItem.id)).toMatchObject({ + status: "delegated", + awaitingChildId: "child-c2", + }) + await childRunBlocked + }), + } + let currentTaskB: object | undefined = parentInstanceB + let childRun: Promise | undefined + const removeParentB = vi.fn(async () => { + currentTaskB = undefined + }) + const createChildC2 = vi.fn(async () => { + currentTaskB = childC2 + return childC2 + }) + const providerB = makeProviderStub({ + taskHistoryStore, + getCurrentTask: vi.fn(() => currentTaskB), + removeClineFromStack: removeParentB, + createTask: createChildC2, + taskScheduler: { + schedule: vi.fn((_task, run) => (childRun = run())), + }, + emit: vi.fn(), + log: vi.fn(), + isViewLaunched: false, + }) + + vi.mocked(readTaskMessages).mockResolvedValue([]) + vi.mocked(readApiMessages).mockResolvedValue([]) + await ClineProvider.prototype.reopenParentFromDelegation.call(providerA, { + parentTaskId: parentItem.id, + childTaskId: "child-c1", + completionResultSummary: "C1 done", + }) + expect(taskHistoryStore.get("child-c1")).toMatchObject({ status: "completed" }) + expect(taskHistoryStore.get(parentItem.id)).toMatchObject({ + status: "active", + completedByChildId: "child-c1", + awaitingChildId: undefined, + }) + + const providerBTransition = ClineProvider.prototype.delegateParentAndOpenChild.call(providerB, { + parentTaskId: parentItem.id, + message: "Run C2", + initialTodos: [], + mode: "code", + }) + + await Promise.resolve() + expect(removeParentB).not.toHaveBeenCalled() + expect(createChildC2).not.toHaveBeenCalled() + expect(taskHistoryStore.atomicReadAndUpdate).not.toHaveBeenCalled() + + const continuationRun = runScheduledContinuation() + await vi.waitFor(() => expect(parentInstanceA.resumeAfterDelegation).toHaveBeenCalledTimes(1)) + await expect(providerBTransition).resolves.toBe(childC2) + expect(removeParentB).toHaveBeenCalledTimes(1) + expect(createChildC2).toHaveBeenCalledTimes(1) + expect(taskHistoryStore.atomicReadAndUpdate).toHaveBeenCalledTimes(1) + expect(taskHistoryStore.get(parentItem.id)).toMatchObject({ + status: "delegated", + awaitingChildId: "child-c2", + delegatedToId: "child-c2", + childIds: ["child-c1", "child-c2"], + }) + await vi.waitFor(() => expect(childC2.run).toHaveBeenCalledTimes(1)) + expect(parentInstanceA.resumeAfterDelegation).toHaveBeenCalledTimes(1) + expect(emitA).not.toHaveBeenCalledWith(RooCodeEventName.TaskDelegationResumed, parentItem.id, "child-c1") + + releaseChildRun() + await childRun + releaseResume() + await continuationRun + settleScheduledContinuation() + await scheduledContinuationSettled + expect(emitA).toHaveBeenCalledWith(RooCodeEventName.TaskDelegationResumed, parentItem.id, "child-c1") + }) + + it("allows resumed work to re-enter the same parent transition queue", async () => { + const parentInstance = { + taskId: "parent-reentrant", + abort: false, + abandoned: false, + resumeAfterDelegation: vi.fn<() => Promise>(), + overwriteClineMessages: vi.fn().mockResolvedValue(undefined), + overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined), + } + const parentItem = { + id: parentInstance.taskId, + status: "delegated", + awaitingChildId: "child-reentrant", + childIds: ["child-reentrant"], + ts: 1, + task: "Parent", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + } + const taskHistoryStore = makeTaskHistoryStoreStub({ id: "child-reentrant", status: "active" }, parentItem) + let currentTask: object | undefined = { taskId: "child-reentrant" } + let runScheduledContinuation!: () => Promise + const provider = makeProviderStub({ + contextProxy: { globalStorageUri: { fsPath: "/tmp" } }, + getTaskWithId: vi.fn().mockResolvedValue({ historyItem: parentItem }), + emit: vi.fn(), + getCurrentTask: vi.fn(() => currentTask), + removeClineFromStack: vi.fn(async () => { + currentTask = undefined + }), + createTaskWithHistoryItem: vi.fn(async () => (currentTask = parentInstance)), + taskScheduler: { + schedule: vi.fn((_task, run) => { + runScheduledContinuation = run + return new Promise(() => {}) + }), + }, + taskHistoryStore, + }) + const reentrantTransition = vi.fn() + parentInstance.resumeAfterDelegation.mockImplementation(async () => { + await ( + ClineProvider.prototype as unknown as { + runDelegationTransition: ( + this: ClineProvider, + parentTaskId: string, + fn: () => Promise, + ) => Promise + } + ).runDelegationTransition.call(provider, parentInstance.taskId, async () => reentrantTransition()) + }) + + vi.mocked(readTaskMessages).mockResolvedValue([]) + vi.mocked(readApiMessages).mockResolvedValue([]) + await ClineProvider.prototype.reopenParentFromDelegation.call(provider, { + parentTaskId: parentInstance.taskId, + childTaskId: "child-reentrant", + completionResultSummary: "done", + }) + await runScheduledContinuation() + expect(parentInstance.resumeAfterDelegation).toHaveBeenCalledTimes(1) + expect(reentrantTransition).toHaveBeenCalledTimes(1) }) + it.each([ + { name: "cancelled child", cancelled: true }, + { name: "aborted parent instance", instance: { abort: true } }, + { name: "abandoned parent instance", instance: { abandoned: true } }, + { name: "current parent instance mismatch", currentMismatch: true }, + { name: "missing persisted parent", missingPersisted: true }, + { name: "non-active persisted parent", persisted: { status: "delegated" as const } }, + { name: "wrong completing child", persisted: { completedByChildId: "child-c2" } }, + { name: "new awaited child", persisted: { awaitingChildId: "child-c2" } }, + { name: "new delegated child", persisted: { delegatedToId: "child-c2" } }, + ])( + "skips resume for $name during ownership revalidation", + async ({ cancelled, instance, currentMismatch, missingPersisted, persisted }) => { + const emit = vi.fn() + const log = vi.fn() + const parentInstance = { + taskId: "parent-stale-owner", + abort: instance?.abort ?? false, + abandoned: instance?.abandoned ?? false, + resumeAfterDelegation: vi.fn().mockResolvedValue(undefined), + overwriteClineMessages: vi.fn().mockResolvedValue(undefined), + overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined), + } + const parentItem = { + id: parentInstance.taskId, + status: "delegated", + awaitingChildId: "child-c1", + childIds: ["child-c1"], + ts: 1, + task: "Parent", + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + } + const taskHistoryStore = makeTaskHistoryStoreStub({ id: "child-c1", status: "active" }, parentItem) + let currentTask: object | undefined = { taskId: "child-c1" } + let runScheduledContinuation!: () => Promise + const cancelledDelegationChildIds = new Set() + const provider = makeProviderStub({ + contextProxy: { globalStorageUri: { fsPath: "/tmp" } }, + getTaskWithId: vi.fn().mockResolvedValue({ historyItem: parentItem }), + emit, + log, + getCurrentTask: vi.fn(() => currentTask), + removeClineFromStack: vi.fn(async () => { + currentTask = undefined + }), + createTaskWithHistoryItem: vi.fn(async () => (currentTask = parentInstance)), + taskScheduler: { + schedule: vi.fn((_task, run) => { + runScheduledContinuation = run + return new Promise(() => {}) + }), + }, + taskHistoryStore, + cancelledDelegationChildIds, + }) + + vi.mocked(readTaskMessages).mockResolvedValue([]) + vi.mocked(readApiMessages).mockResolvedValue([]) + await ClineProvider.prototype.reopenParentFromDelegation.call(provider, { + parentTaskId: parentInstance.taskId, + childTaskId: "child-c1", + completionResultSummary: "C1 done", + }) + const completedParent = taskHistoryStore.get(parentInstance.taskId) + if (cancelled) cancelledDelegationChildIds.add("child-c1") + if (currentMismatch) currentTask = { taskId: "other-parent-instance" } + taskHistoryStore.get.mockImplementation((id: string) => + id === parentInstance.taskId && !missingPersisted + ? ({ ...completedParent, ...persisted } as HistoryItem) + : undefined, + ) + + await runScheduledContinuation() + expect(parentInstance.resumeAfterDelegation).not.toHaveBeenCalled() + expect(emit).not.toHaveBeenCalledWith( + RooCodeEventName.TaskDelegationResumed, + parentInstance.taskId, + "child-c1", + ) + expect(log).toHaveBeenCalledWith(expect.stringContaining("Skipping stale parent continuation")) + }, + ) + it("reopenParentFromDelegation propagates atomicUpdatePair failure — parent not reopened (RPD-04)", async () => { const parentItem = { id: "parent-rpd04", @@ -1464,18 +1955,30 @@ describe("History resume delegation - parent metadata transitions", () => { // history item (createTaskWithHistoryItem passes historyItem.status through as // initialStatus), so the resumed task instance still reports "interrupted". let currentActiveId: string | undefined = "child-566" + let currentTask: + | { + taskId: string + resumeAfterDelegation?: ReturnType + overwriteClineMessages?: ReturnType + overwriteApiConversationHistory?: ReturnType + } + | undefined = { + taskId: "child-566", + } const emitSpy = vi.fn() const removeClineFromStack = vi.fn().mockImplementation(async () => { currentActiveId = undefined + currentTask = undefined }) const createTaskWithHistoryItem = vi.fn().mockImplementation(async (historyItem: any) => { currentActiveId = historyItem.id - return { + currentTask = { taskId: historyItem.id, resumeAfterDelegation: vi.fn().mockResolvedValue(undefined), overwriteClineMessages: vi.fn().mockResolvedValue(undefined), overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined), } + return currentTask }) const getTaskWithId = vi.fn(async (id: string) => { const item = id === "child-566" ? childItem : id === "parent-566" ? parentItem : undefined @@ -1504,7 +2007,7 @@ describe("History resume delegation - parent metadata transitions", () => { contextProxy: { globalStorageUri: { fsPath: "/tmp" } }, getTaskWithId, emit: emitSpy, - getCurrentTask: vi.fn(() => (currentActiveId ? ({ taskId: currentActiveId } as any) : undefined)), + getCurrentTask: vi.fn(() => currentTask as unknown as Task | undefined), removeClineFromStack, createTaskWithHistoryItem, taskHistoryStore, @@ -1555,6 +2058,7 @@ describe("History resume delegation - parent metadata transitions", () => { askFinishSubTaskApproval: vi.fn(async () => true), toolDescription: () => "desc", } as any) + await vi.waitFor(() => expect(currentTask?.resumeAfterDelegation).toHaveBeenCalledTimes(1)) // The parent must regain control — this is the exact behavior issue #566 reported as broken. expect(currentActiveId).toBe("parent-566") diff --git a/src/__tests__/helpers/provider-stub.ts b/src/__tests__/helpers/provider-stub.ts index 59dde33933..852e2f5a67 100644 --- a/src/__tests__/helpers/provider-stub.ts +++ b/src/__tests__/helpers/provider-stub.ts @@ -3,10 +3,10 @@ import { TaskRegistry } from "../../core/task/TaskRegistry" import { type Task } from "../../core/task/Task" type ProviderStubFields = { - delegationTransitionLocks?: Map> cancelledDelegationChildIds?: Set log?: ReturnType - taskHistoryStore?: { get: (id: string) => unknown } + taskHistoryStore?: { get: (id: string) => unknown; invalidate?: (id: string) => Promise } + taskScheduler?: { schedule: (task: Task, run: () => Promise) => Promise } taskRegistry?: TaskRegistry clineStack?: Task[] tasks?: Task[] @@ -24,7 +24,7 @@ type PrivateProviderMethods = { /** * Augments a plain stub object with the instance fields and bound methods that * ClineProvider methods read from `this` (runDelegationTransition, - * delegationTransitionLocks, cancelledDelegationChildIds, cancellingDelegationChildIds), + * cancelledDelegationChildIds and taskHistoryStore), * so tests can call private ClineProvider methods against a plain object * without instantiating a real ClineProvider. * @@ -34,10 +34,11 @@ type PrivateProviderMethods = { export function makeProviderStub(stub: T): ClineProvider { const s = stub as T & ProviderStubFields const proto = ClineProvider.prototype as unknown as PrivateProviderMethods - s.delegationTransitionLocks ??= new Map() s.cancelledDelegationChildIds ??= new Set() s.log ??= vi.fn() s.taskHistoryStore ??= { get: () => undefined } + s.taskHistoryStore.invalidate ??= async () => {} + s.taskScheduler ??= { schedule: async (_task, run) => run() } // Convert legacy clineStack array into a TaskRegistry if (!s.taskRegistry) { diff --git a/src/__tests__/nested-delegation-resume.spec.ts b/src/__tests__/nested-delegation-resume.spec.ts index 9b06ad4162..57b831b4a2 100644 --- a/src/__tests__/nested-delegation-resume.spec.ts +++ b/src/__tests__/nested-delegation-resume.spec.ts @@ -65,6 +65,16 @@ describe("Nested delegation resume (A → B → C)", () => { it("C completes → reopens B; then B completes → reopens A; emits correct events; no resume_task asks", async () => { // Track which task is "current" to satisfy provider.reopenParentFromDelegation() child-close logic let currentActiveId: string | undefined = "C" + let currentTask: + | { + taskId: string + resumeAfterDelegation?: ReturnType + overwriteClineMessages?: ReturnType + overwriteApiConversationHistory?: ReturnType + } + | undefined = { + taskId: "C", + } // History index: A is parent of B, B is parent of C const historyIndex: Record = { @@ -116,6 +126,7 @@ describe("Nested delegation resume (A → B → C)", () => { const removeClineFromStack = vi.fn().mockImplementation(async () => { // Simulate closing current child currentActiveId = undefined + currentTask = undefined }) const createTaskWithHistoryItem = vi .fn() @@ -125,12 +136,13 @@ describe("Nested delegation resume (A → B → C)", () => { // Reopen the parent currentActiveId = historyItem.id // Return minimal parent instance with resumeAfterDelegation - return { + currentTask = { taskId: historyItem.id, resumeAfterDelegation: vi.fn().mockResolvedValue(undefined), overwriteClineMessages: vi.fn().mockResolvedValue(undefined), overwriteApiConversationHistory: vi.fn().mockResolvedValue(undefined), } + return currentTask }) const getTaskWithId = vi.fn(async (id: string) => { @@ -167,13 +179,14 @@ describe("Nested delegation resume (A → B → C)", () => { }, ), get: vi.fn((id: string) => historyIndex[id]), + invalidate: vi.fn().mockResolvedValue(undefined), } const provider = makeProviderStub({ contextProxy: { globalStorageUri: { fsPath: "/tmp" } }, getTaskWithId, emit: emitSpy, - getCurrentTask: vi.fn(() => (currentActiveId ? ({ taskId: currentActiveId } as any) : undefined)), + getCurrentTask: vi.fn(() => currentTask as unknown as Task | undefined), removeClineFromStack, createTaskWithHistoryItem, updateTaskHistory, @@ -231,6 +244,7 @@ describe("Nested delegation resume (A → B → C)", () => { askFinishSubTaskApproval, toolDescription: () => "desc", } as any) + await vi.waitFor(() => expect(currentTask?.resumeAfterDelegation).toHaveBeenCalledTimes(1)) // After C completes, B must be current expect(currentActiveId).toBe("B") @@ -274,6 +288,7 @@ describe("Nested delegation resume (A → B → C)", () => { askFinishSubTaskApproval, toolDescription: () => "desc", } as any) + await vi.waitFor(() => expect(currentTask?.resumeAfterDelegation).toHaveBeenCalledTimes(1)) // After B completes, A should become current // Note: delegation resume may fall back to a non-tool_result user message when the parent history diff --git a/src/core/config/ProviderSettingsManager.ts b/src/core/config/ProviderSettingsManager.ts index 51f79cff35..3fcc0e6e43 100644 --- a/src/core/config/ProviderSettingsManager.ts +++ b/src/core/config/ProviderSettingsManager.ts @@ -618,7 +618,7 @@ export class ProviderSettingsManager { const content = await this.context.secrets.get(this.secretsKey) if (!content) { - return this.defaultProviderProfiles + return structuredClone(this.defaultProviderProfiles) } const providerProfiles = providerProfilesSchema diff --git a/src/core/config/__tests__/ProviderSettingsManager.spec.ts b/src/core/config/__tests__/ProviderSettingsManager.spec.ts index be0cbfec92..7bedfc1a25 100644 --- a/src/core/config/__tests__/ProviderSettingsManager.spec.ts +++ b/src/core/config/__tests__/ProviderSettingsManager.spec.ts @@ -627,6 +627,21 @@ describe("ProviderSettingsManager", () => { ) }) + it("keeps nested defaults pristine when the initial save fails", async () => { + mockSecrets.store.mockRejectedValueOnce(new Error("Storage failed")) + + await expect( + providerSettingsManager.saveConfig("default", { + apiProvider: providerIdentifiers.anthropic, + apiKey: "test-key", + }), + ).rejects.toThrow("Storage failed") + + await expect(providerSettingsManager.listConfig()).resolves.toEqual([ + { name: "default", id: expect.any(String), apiProvider: undefined }, + ]) + }) + it("should preserve full fields including legacy provider-specific keys when saving retired provider profiles", async () => { mockSecrets.get.mockResolvedValue( JSON.stringify({ diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index fae796db6b..92ee8184d6 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -138,6 +138,7 @@ import { validateAndFixToolResultIds } from "./validateToolResultIds" import { mergeConsecutiveApiMessages } from "./mergeConsecutiveApiMessages" import { prepareApiConversationMessage } from "./apiConversationHistory" import { shouldAddUserMessageToHistory } from "./messageCounting" +import { type TaskExecutionContext } from "./providerHandoff" const MAX_EXPONENTIAL_BACKOFF_SECONDS = 600 // 10 minutes const DEFAULT_USAGE_COLLECTION_TIMEOUT_MS = 5000 // 5 seconds @@ -193,6 +194,8 @@ export interface TaskOptions extends CreateTaskOptions { initialStatus?: "active" | "delegated" | "completed" | "interrupted" rateLimitClock?: RateLimitClock diffFuzzyThreshold?: number + /** Explicit task-local execution context for a delegated child. */ + handoffExecutionContext?: TaskExecutionContext } type AssistantMessagePersistenceResult = boolean @@ -524,6 +527,7 @@ export class Task extends EventEmitter implements TaskLike { initialStatus, rateLimitClock, diffFuzzyThreshold, + handoffExecutionContext, }: TaskOptions) { super() this.resetAssistantMessagePersistence() @@ -573,7 +577,7 @@ export class Task extends EventEmitter implements TaskLike { console.error("Failed to initialize RooIgnoreController:", error) }) - this.apiConfiguration = apiConfiguration + this.apiConfiguration = handoffExecutionContext?.apiConfiguration ?? apiConfiguration this.api = buildApiHandler(this.apiConfiguration) this.rateLimitClock = rateLimitClock ?? createRateLimitClock() this.autoApprovalHandler = new AutoApprovalHandler() @@ -593,7 +597,13 @@ export class Task extends EventEmitter implements TaskLike { // Store the task's mode and API config name when it's created. // For history items, use the stored values; for new tasks, we'll set them // after getting state. - if (historyItem) { + if (handoffExecutionContext) { + this._taskMode = handoffExecutionContext.mode + this._taskApiConfigName = handoffExecutionContext.apiConfigName + this.taskModeReady = Promise.resolve() + this.taskApiConfigReady = Promise.resolve() + TelemetryService.instance.captureTaskCreated(this.taskId) + } else if (historyItem) { this._taskMode = historyItem.mode || defaultModeSlug this._taskApiConfigName = historyItem.apiConfigName this.taskModeReady = Promise.resolve() diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 7418920cb1..1bcacd459c 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -11,6 +11,7 @@ import { providerIdentifiers, RooCodeEventName, type GlobalState, + type HistoryItem, type ProviderSettings, type ModelInfo, type TaskLike, @@ -647,6 +648,64 @@ describe("Cline", () => { }) describe("constructor", () => { + it.each([{ apiConfigName: "parent-local-profile" }, { apiConfigName: undefined }])( + "uses an explicit delegated-child context without shared state or startup persistence", + async ({ apiConfigName }) => { + const captureTaskCreated = vi.spyOn(TelemetryService.instance, "captureTaskCreated") + const captureTaskRestarted = vi.spyOn(TelemetryService.instance, "captureTaskRestarted") + const getState = vi.spyOn(mockProvider, "getState") + const updateTaskHistory = vi.spyOn(mockProvider, "updateTaskHistory") + const localConfiguration: ProviderSettings = { + apiProvider: providerIdentifiers.openrouter, + openRouterModelId: "openai/gpt-4", + } + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "delegated child", + startTask: false, + handoffExecutionContext: { + mode: "ask", + apiConfigName, + apiConfiguration: localConfiguration, + }, + }) + + await expect(task.getTaskMode()).resolves.toBe("ask") + await expect(task.getTaskApiConfigName()).resolves.toBe(apiConfigName) + expect(task.apiConfiguration).toEqual(localConfiguration) + expect(getState).not.toHaveBeenCalled() + expect(updateTaskHistory).not.toHaveBeenCalled() + expect(captureTaskCreated).toHaveBeenCalledWith(task.taskId) + expect(captureTaskRestarted).not.toHaveBeenCalled() + }, + ) + + it("keeps history-task initialization distinct from delegated-child initialization", async () => { + const captureTaskRestarted = vi.spyOn(TelemetryService.instance, "captureTaskRestarted") + const historyItem = { + id: "history-task", + number: 1, + task: "history", + ts: Date.now(), + tokensIn: 0, + tokensOut: 0, + totalCost: 0, + mode: "architect", + apiConfigName: "history-profile", + } satisfies HistoryItem + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + historyItem, + startTask: false, + }) + + await expect(task.getTaskMode()).resolves.toBe("architect") + await expect(task.getTaskApiConfigName()).resolves.toBe("history-profile") + expect(captureTaskRestarted).toHaveBeenCalledWith("history-task") + }) + it("should always have diff strategy defined", async () => { const cline = new Task({ provider: mockProvider, diff --git a/src/core/task/__tests__/providerHandoff.spec.ts b/src/core/task/__tests__/providerHandoff.spec.ts new file mode 100644 index 0000000000..3f6ad713cc --- /dev/null +++ b/src/core/task/__tests__/providerHandoff.spec.ts @@ -0,0 +1,68 @@ +import { providerIdentifiers, type ProviderSettings } from "@roo-code/types" + +import { getEffectiveTaskApiConfiguration, selectHandoffExecutionContext } from "../providerHandoff" + +const parentConfiguration: ProviderSettings = { + apiProvider: providerIdentifiers.anthropic, + consecutiveMistakeLimit: 3, +} +const savedConfiguration: ProviderSettings = { + apiProvider: providerIdentifiers.openrouter, + consecutiveMistakeLimit: 7, +} +const parent = { mode: "code", apiConfigName: undefined, apiConfiguration: parentConfiguration } + +describe("provider handoff decisions", () => { + it.each([ + { name: "unsaved", locked: false, saved: undefined, expected: parentConfiguration }, + { + name: "saved", + locked: false, + saved: { name: "ask-profile", apiConfiguration: savedConfiguration }, + expected: savedConfiguration, + }, + { + name: "locked", + locked: true, + saved: { name: "ask-profile", apiConfiguration: savedConfiguration }, + expected: parentConfiguration, + }, + { name: "stale", locked: false, saved: undefined, expected: parentConfiguration }, + ])("selects the $name profile path without mutating the parent", ({ locked, saved, expected }) => { + const selected = selectHandoffExecutionContext(parent, "ask", "code", locked, saved) + + expect(selected.apiConfiguration).toEqual(expected) + expect(selected.apiConfiguration).not.toBe(expected) + expect(parent.apiConfiguration).toBe(parentConfiguration) + }) + + it("derives task limits from the effective handoff configuration", () => { + const handoff = selectHandoffExecutionContext(parent, "ask", "code", false, { + name: "ask-profile", + apiConfiguration: savedConfiguration, + }) + + expect(getEffectiveTaskApiConfiguration(parentConfiguration, handoff).consecutiveMistakeLimit).toBe(7) + expect(getEffectiveTaskApiConfiguration(parentConfiguration).consecutiveMistakeLimit).toBe(3) + }) + + it("keeps the parent configuration when a configured saved profile is locked", () => { + const selected = selectHandoffExecutionContext(parent, "ask", "code", true, { + name: "ask-profile", + apiConfiguration: savedConfiguration, + }) + + expect(selected.apiConfigName).toBeUndefined() + expect(selected.apiConfiguration).toEqual(parentConfiguration) + }) + + it("keeps the parent configuration when the requested mode is unchanged", () => { + const selected = selectHandoffExecutionContext(parent, "code", "code", false, { + name: "ask-profile", + apiConfiguration: savedConfiguration, + }) + + expect(selected.apiConfigName).toBeUndefined() + expect(selected.apiConfiguration).toEqual(parentConfiguration) + }) +}) diff --git a/src/core/task/providerHandoff.ts b/src/core/task/providerHandoff.ts new file mode 100644 index 0000000000..a4cb1f349c --- /dev/null +++ b/src/core/task/providerHandoff.ts @@ -0,0 +1,41 @@ +import type { ProviderSettings } from "@roo-code/types" + +export type TaskExecutionContext = { + mode: string + apiConfigName: string | undefined + apiConfiguration: ProviderSettings +} + +export type SavedModeProfile = { + name?: string + apiConfiguration: ProviderSettings +} + +export function selectHandoffExecutionContext( + parent: TaskExecutionContext, + requestedMode: string, + parentMode: string, + lockApiConfigAcrossModes: boolean, + savedModeProfile?: SavedModeProfile, +): TaskExecutionContext { + if (requestedMode !== parentMode && !lockApiConfigAcrossModes && savedModeProfile?.apiConfiguration.apiProvider) { + return { + mode: requestedMode, + apiConfigName: savedModeProfile.name, + apiConfiguration: structuredClone(savedModeProfile.apiConfiguration), + } + } + + return { + mode: requestedMode, + apiConfigName: parent.apiConfigName, + apiConfiguration: structuredClone(parent.apiConfiguration), + } +} + +export function getEffectiveTaskApiConfiguration( + apiConfiguration: ProviderSettings, + handoffExecutionContext?: TaskExecutionContext, +): ProviderSettings { + return handoffExecutionContext?.apiConfiguration ?? apiConfiguration +} diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 87a899344c..495fe454b7 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -60,6 +60,11 @@ import { import { RateLimitClock, createRateLimitClock } from "../task/RateLimitClock" import { TaskRegistry } from "../task/TaskRegistry" import { TaskScheduler } from "../task/TaskScheduler" +import { + getEffectiveTaskApiConfiguration, + selectHandoffExecutionContext, + type TaskExecutionContext, +} from "../task/providerHandoff" import { aggregateTaskCostsRecursive, type AggregatedCosts } from "./aggregateTaskCosts" import { TelemetryService } from "@roo-code/telemetry" import { CloudService, getRooCodeApiUrl } from "@roo-code/cloud" @@ -137,6 +142,8 @@ export type ClineProviderEvents = { clineCreated: [cline: Task] } +type DelegatedChildContext = TaskExecutionContext + function runDelegationTransition( locks: Map>, parentTaskId: string, @@ -163,9 +170,14 @@ function runDelegationTransition( return current } -function scheduleTask(scheduler: TaskScheduler, task: Task, source: string): void { +function scheduleTask( + scheduler: TaskScheduler, + task: Task, + source: string, + run: () => Promise = () => task.run(), +): void { void scheduler - .schedule(task, () => task.run()) + .schedule(task, run) .catch((error) => console.error(`[${source}] taskScheduler.schedule failed:`, error)) } @@ -197,7 +209,7 @@ export class ClineProvider private view?: vscode.WebviewView | vscode.WebviewPanel private taskRegistry = new TaskRegistry() private taskScheduler = new TaskScheduler() - private delegationTransitionLocks?: Map> + private static readonly delegationTransitionLocks = new Map>() private cancelledDelegationChildIds = new Set() private codeIndexStatusSubscription?: vscode.Disposable private codeIndexManager?: CodeIndexManager @@ -237,8 +249,7 @@ export class ClineProvider private historyTaskCreationQueue = Promise.resolve() private runDelegationTransition(parentTaskId: string, fn: () => Promise): Promise { - this.delegationTransitionLocks ??= new Map() - return runDelegationTransition(this.delegationTransitionLocks, parentTaskId, fn) + return runDelegationTransition(ClineProvider.delegationTransitionLocks, parentTaskId, fn) } private enqueueProviderProfileMutation(fn: (signal: AbortSignal) => Promise): Promise { @@ -1347,7 +1358,6 @@ export class ClineProvider taskSyncEnabled, diffFuzzyThreshold, } = await this.getState() - const task = new Task({ provider: this, apiConfiguration, @@ -2704,6 +2714,14 @@ export class ClineProvider const mergedDeniedCommands = this.mergeDeniedCommands(deniedCommands) const cwd = this.cwd const currentTask = this.getCurrentTask() + let currentTaskMode: string | undefined + try { + currentTaskMode = currentTask?.taskMode + } catch { + // A just-created task may still be initializing its mode; retain the persisted projection for this post. + } + const currentTaskApiConfigName = currentTask?.taskApiConfigName + const currentTaskApiConfiguration = currentTask?.apiConfiguration let zooCodeState: { zooCodeIsAuthenticated: boolean zooCodeUserName: string | undefined @@ -2738,7 +2756,7 @@ export class ClineProvider return { version: this.context.extension?.packageJSON?.version ?? "", - apiConfiguration, + apiConfiguration: currentTaskApiConfiguration ?? apiConfiguration, customInstructions, alwaysAllowReadOnly: alwaysAllowReadOnly ?? false, alwaysAllowReadOnlyOutsideWorkspace: alwaysAllowReadOnlyOutsideWorkspace ?? false, @@ -2787,10 +2805,10 @@ export class ClineProvider terminalZdotdir: terminalZdotdir ?? false, terminalProfile, mcpEnabled: mcpEnabled ?? true, - currentApiConfigName: currentApiConfigName ?? "default", + currentApiConfigName: currentTask ? currentTaskApiConfigName : currentApiConfigName, listApiConfigMeta: listApiConfigMeta ?? [], pinnedApiConfigs: pinnedApiConfigs ?? {}, - mode: mode ?? defaultModeSlug, + mode: currentTaskMode ?? mode ?? defaultModeSlug, customModePrompts: customModePrompts ?? {}, customSupportPrompts: customSupportPrompts ?? {}, enhancementApiConfigId, @@ -3417,7 +3435,7 @@ export class ClineProvider text?: string, images?: string[], parentTask?: Task, - options: CreateTaskOptions = {}, + options: CreateTaskOptions & { handoffExecutionContext?: DelegatedChildContext } = {}, configuration: RooCodeSettings = {}, ): Promise { if (configuration) { @@ -3468,6 +3486,10 @@ export class ClineProvider organizationAllowList, diffFuzzyThreshold, } = await this.getState() + const effectiveApiConfiguration = getEffectiveTaskApiConfiguration( + apiConfiguration, + options.handoffExecutionContext, + ) // Single-open-task invariant: always enforce for user-initiated top-level tasks. if (!parentTask) { @@ -3476,7 +3498,7 @@ export class ClineProvider }) } - if (!ProfileValidator.isProfileAllowed(apiConfiguration, organizationAllowList)) { + if (!ProfileValidator.isProfileAllowed(effectiveApiConfiguration, organizationAllowList)) { throw new OrganizationAllowListViolationError(t("common:errors.violated_organization_allowlist")) } @@ -3485,7 +3507,7 @@ export class ClineProvider apiConfiguration, enableCheckpoints, checkpointTimeout, - consecutiveMistakeLimit: apiConfiguration.consecutiveMistakeLimit, + consecutiveMistakeLimit: effectiveApiConfiguration.consecutiveMistakeLimit, task: text, images, experiments, @@ -3826,6 +3848,18 @@ export class ClineProvider initialTodos: TodoItem[] mode: string pendingActionId?: string + }): Promise { + return runDelegationTransition(ClineProvider.delegationTransitionLocks, params.parentTaskId, () => + ClineProvider.prototype.delegateParentAndOpenChildUnlocked.call(this, params), + ) + } + + private async delegateParentAndOpenChildUnlocked(params: { + parentTaskId: string + message: string + initialTodos: TodoItem[] + mode: string + pendingActionId?: string }): Promise { const { parentTaskId, message, initialTodos, mode, pendingActionId } = params @@ -3841,6 +3875,19 @@ export class ClineProvider `[delegateParentAndOpenChild] Parent mismatch: expected ${parentTaskId}, current ${parent.taskId}`, ) } + + // A different provider may have delegated this parent while this call + // waited on the shared lock. Refresh before mutating either task stack. + await this.taskHistoryStore.invalidate(parentTaskId) + const authoritativeParent = this.taskHistoryStore.get(parentTaskId) + if (authoritativeParent?.status === "delegated") { + const awaitedChildId = authoritativeParent.awaitingChildId + if (!awaitedChildId) throw new Error("Cannot re-delegate a parent with no awaited child") + await this.taskHistoryStore.invalidate(awaitedChildId) + if (this.taskHistoryStore.get(awaitedChildId)?.status !== "interrupted") { + throw new Error("Cannot re-delegate while the awaited child is not interrupted") + } + } if (pendingActionId) { const parentHistory = this.taskHistoryStore.get(parentTaskId) if (parentHistory?.pendingAction?.actionId !== pendingActionId) { @@ -3849,6 +3896,42 @@ export class ClineProvider ) } } + + const parentExecutionContext: DelegatedChildContext = { + mode, + apiConfigName: await parent.getTaskApiConfigName(), + apiConfiguration: structuredClone(parent.apiConfiguration), + } + const parentMode = await parent.getTaskMode() + const lockApiConfigAcrossModes = + mode !== parentMode && this.context.workspaceState.get("lockApiConfigAcrossModes", false) + let savedModeProfile: { name?: string; apiConfiguration: ProviderSettings } | undefined + if (mode !== parentMode && !lockApiConfigAcrossModes) { + const savedConfigId = await this.providerSettingsManager.getModeConfigId(mode as Mode) + if (savedConfigId) { + try { + const { + name, + id: _id, + ...savedConfiguration + } = await this.providerSettingsManager.getProfile({ + id: savedConfigId, + }) + savedModeProfile = { name, apiConfiguration: savedConfiguration } + } catch (error) { + this.log( + `[delegateParentAndOpenChild] Saved profile ${savedConfigId} for mode '${mode}' could not be loaded for parent ${parentTaskId}: ${error instanceof Error ? error.message : String(error)}. Using the parent task configuration.`, + ) + } + } + } + const handoffExecutionContext = selectHandoffExecutionContext( + parentExecutionContext, + mode, + parentMode, + lockApiConfigAcrossModes, + savedModeProfile, + ) // 2) Flush pending tool results to API history BEFORE disposing the parent. // This is critical: when tools are called before new_task, // their tool_result blocks are in userMessageContent but not yet saved to API history. @@ -3897,21 +3980,9 @@ export class ClineProvider // Non-fatal: proceed with child creation even if parent cleanup had issues } - // 3) Switch provider mode to child's requested mode BEFORE creating the child task - // This ensures the child's system prompt and configuration are based on the correct mode. - // The mode switch must happen before createTask() because the Task constructor - // initializes its mode from provider.getState() during initializeTaskMode(). - try { - await this.handleModeSwitch(mode as any) - } catch (e) { - this.log( - `[delegateParentAndOpenChild] handleModeSwitch failed for mode '${mode}': ${ - (e as Error)?.message ?? String(e) - }`, - ) - } - - // 4) Create child as sole active (parent reference preserved for lineage) + // 4) Bind the child directly to the delegating task's local provider + // context. Delegation never mutates shared profile/global state. + // Create child as sole active (parent reference preserved for lineage) // Pass initialStatus: "active" to ensure the child task's historyItem is created // with status from the start, avoiding race conditions where the task might // call attempt_completion before status is persisted separately. @@ -3926,6 +3997,7 @@ export class ClineProvider initialTodos, initialStatus: "active", startTask: false, + handoffExecutionContext, }) // 5) Persist parent delegation metadata BEFORE the child starts writing. @@ -4282,15 +4354,67 @@ export class ClineProvider // non-fatal } - // Auto-resume parent without ask("resume_task") - await parentInstance.resumeAfterDelegation() - } + let admitContinuation!: () => void + const continuationAdmitted = new Promise((resolve) => { + admitContinuation = resolve + }) + let schedulerAdmitted = false + // Reserve the continuation's place in the shared parent queue before this + // completion transition releases. Its body waits until scheduler admission, + // so the completing child can release its permit without deadlocking. + const continuation = this.runDelegationTransition(parentTaskId, async () => { + await continuationAdmitted + if (!schedulerAdmitted) return {} + await this.taskHistoryStore.invalidate(parentTaskId) + const persistedParent = this.taskHistoryStore.get(parentTaskId) + const currentTask = this.getCurrentTask() + if ( + this.cancelledDelegationChildIds.has(childTaskId) || + parentInstance.abort || + parentInstance.abandoned || + currentTask !== parentInstance || + persistedParent?.status !== "active" || + persistedParent.completedByChildId !== childTaskId || + persistedParent.awaitingChildId !== undefined || + persistedParent.delegatedToId !== undefined + ) { + this.log( + `[reopenParentFromDelegation] Skipping stale parent continuation for ${parentTaskId} after child ${childTaskId}`, + ) + return {} + } - // 9) Emit TaskDelegationResumed (provider-level) - try { - this.emit(RooCodeEventName.TaskDelegationResumed, parentTaskId, childTaskId) - } catch { - // non-fatal + // Keep the run promise inside an object so the transition queue does not + // assimilate it and retain the parent key for the full resumed task loop. + return { runPromise: parentInstance.resumeAfterDelegation() } + }) + void this.taskScheduler + .schedule(parentInstance, async () => { + schedulerAdmitted = true + admitContinuation() + const { runPromise } = await continuation + if (!runPromise) return + try { + await runPromise + try { + this.emit(RooCodeEventName.TaskDelegationResumed, parentTaskId, childTaskId) + } catch { + // non-fatal + } + } catch (error) { + const message = `Failed to resume parent task ${parentTaskId} after subtask ${childTaskId}: ${error instanceof Error ? error.message : String(error)}` + this.log(`[reopenParentFromDelegation] ${message}`) + await vscode.window.showErrorMessage(`${message}. Open the task from history to retry.`) + throw error + } + }) + .then(admitContinuation, (error) => { + admitContinuation() + console.error( + `[${ClineProvider.prototype.reopenParentFromDelegation.name}] taskScheduler.schedule failed:`, + error, + ) + }) } this.cancelledDelegationChildIds.delete(childTaskId) diff --git a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts index 0365283222..2bbf0736c6 100644 --- a/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts +++ b/src/core/webview/__tests__/ClineProvider.taskHistory.spec.ts @@ -2,10 +2,12 @@ import * as vscode from "vscode" import type { HistoryItem, ExtensionMessage } from "@roo-code/types" -import { RooCodeEventName } from "@roo-code/types" +import { providerIdentifiers, RooCodeEventName } from "@roo-code/types" import { TelemetryService } from "@roo-code/telemetry" import { ContextProxy } from "../../config/ContextProxy" +import { Task } from "../../task/Task" +import { ProfileValidator } from "../../../shared/ProfileValidator" import { ClineProvider } from "../ClineProvider" // Mock setup @@ -640,6 +642,91 @@ describe("ClineProvider Task History Synchronization", () => { }) describe("task history includes all workspaces", () => { + it("uses the default profile name when no task is active and no profile is saved", async () => { + await provider["updateGlobalState"]("currentApiConfigName", undefined) + + const state = await provider.getStateToPostToWebview() + + expect(state.currentTaskId).toBeUndefined() + expect(state.currentApiConfigName).toBe("default") + }) + + it("projects the active task's local mode and provider profile", async () => { + const activeTask = { + taskId: "task-local-context", + taskMode: "ask", + taskApiConfigName: undefined, + apiConfiguration: { + apiProvider: providerIdentifiers.openrouter, + openRouterModelId: "task-local-model", + }, + clineMessages: [], + todoList: [], + messageQueueService: { messages: [] }, + } + // The module-level Task mock intentionally implements only the fields this provider-state test reads. + provider["taskRegistry"].push(activeTask as unknown as Task) + await provider.updateTaskHistory( + createHistoryItem({ + id: activeTask.taskId, + task: "Task-local context", + mode: "ask", + apiConfigName: undefined, + }), + { broadcast: false }, + ) + + const state = await provider.getStateToPostToWebview() + + expect(state.mode).toBe("ask") + expect(state.currentApiConfigName).toBeUndefined() + expect(state.apiConfiguration).toEqual(activeTask.apiConfiguration) + expect(state.currentTaskId).toBe(activeTask.taskId) + }) + + it("validates and applies the delegated child's effective profile", async () => { + const effectiveConfiguration = { + apiProvider: providerIdentifiers.openrouter, + openRouterModelId: "allowed-child-model", + consecutiveMistakeLimit: 7, + } + const isProfileAllowed = vi.spyOn(ProfileValidator, "isProfileAllowed").mockReturnValue(true) + const parentTask = { taskId: "parent", workspacePath: "/test/workspace" } as Task + + await provider.createTask("child", undefined, parentTask, { + startTask: false, + handoffExecutionContext: { + mode: "ask", + apiConfigName: "allowed-child", + apiConfiguration: effectiveConfiguration, + }, + }) + + expect(isProfileAllowed).toHaveBeenCalledWith(effectiveConfiguration, expect.anything()) + expect(vi.mocked(Task)).toHaveBeenCalledWith(expect.objectContaining({ consecutiveMistakeLimit: 7 })) + }) + + it("rejects a delegated child when its effective profile is not allowed", async () => { + const effectiveConfiguration = { + apiProvider: providerIdentifiers.openrouter, + openRouterModelId: "blocked-child-model", + } + vi.spyOn(ProfileValidator, "isProfileAllowed").mockReturnValue(false) + const parentTask = { taskId: "parent", workspacePath: "/test/workspace" } as Task + + await expect( + provider.createTask("child", undefined, parentTask, { + startTask: false, + handoffExecutionContext: { + mode: "ask", + apiConfigName: "blocked-child", + apiConfiguration: effectiveConfiguration, + }, + }), + ).rejects.toThrow("errors.violated_organization_allowlist") + expect(vi.mocked(Task)).not.toHaveBeenCalled() + }) + it("getStateToPostToWebview returns tasks from all workspaces", async () => { await provider.resolveWebviewView(mockWebviewView) diff --git a/src/eslint-suppressions.json b/src/eslint-suppressions.json index 381cf0c1e0..d90272962b 100644 --- a/src/eslint-suppressions.json +++ b/src/eslint-suppressions.json @@ -4,6 +4,16 @@ "count": 4 } }, + "__tests__/ClineProvider.delegation.spec.ts": { + "@typescript-eslint/no-explicit-any": { + "count": 9 + } + }, + "__tests__/ClineProvider.history-resume-delegation.spec.ts": { + "@typescript-eslint/no-explicit-any": { + "count": 70 + } + }, "__tests__/abandonSubtask.spec.ts": { "@typescript-eslint/no-explicit-any": { "count": 20 @@ -24,11 +34,6 @@ "count": 6 } }, - "__tests__/history-resume-delegation.spec.ts": { - "@typescript-eslint/no-explicit-any": { - "count": 71 - } - }, "__tests__/migrateSettings.spec.ts": { "@typescript-eslint/no-explicit-any": { "count": 4 @@ -36,7 +41,7 @@ }, "__tests__/nested-delegation-resume.spec.ts": { "@typescript-eslint/no-explicit-any": { - "count": 20 + "count": 19 } }, "__tests__/new-task-delegation.spec.ts": { @@ -44,11 +49,6 @@ "count": 9 } }, - "__tests__/provider-delegation.spec.ts": { - "@typescript-eslint/no-explicit-any": { - "count": 9 - } - }, "activate/CodeActionProvider.ts": { "@typescript-eslint/no-explicit-any": { "count": 1 @@ -1026,7 +1026,7 @@ }, "core/webview/ClineProvider.ts": { "@typescript-eslint/no-explicit-any": { - "count": 8 + "count": 7 } }, "core/webview/__tests__/ClineProvider.apiHandlerRebuild.spec.ts": {