Repository navigation
fix(orchestration): reconcile held delegated tasks after child completion #15170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
jakeleventhal
wants to merge
1
commit into
pingdotgg:main
from
jakeleventhal:fix/terminal-subagent-lineage-queue
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
234 changes: 234 additions & 0 deletions
234
apps/server/src/mcp/OrchestratorMcpService.queued-tasks.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,234 @@ | ||
| import * as NodeServices from "@effect/platform-node/NodeServices"; | ||
| import { assert, it } from "@effect/vitest"; | ||
| import { | ||
| EnvironmentId, | ||
| MessageId, | ||
| NodeId, | ||
| type OrchestrationV2ThreadProjection, | ||
| type OrchestrationV2ServerCommand, | ||
| ProjectId, | ||
| ProviderInstanceId, | ||
| RunId, | ||
| ThreadId, | ||
| } from "@t3tools/contracts"; | ||
| import * as DateTime from "effect/DateTime"; | ||
| import * as Effect from "effect/Effect"; | ||
| import * as Layer from "effect/Layer"; | ||
|
|
||
| import * as ProviderAdapterRegistry from "../orchestration-v2/ProviderAdapterRegistry.ts"; | ||
| import * as ThreadManagementService from "../orchestration-v2/ThreadManagementService.ts"; | ||
| import * as ProviderRegistry from "../provider/Services/ProviderRegistry.ts"; | ||
| import * as ScheduledTaskService from "../scheduledTasks/ScheduledTaskService.ts"; | ||
| import type { McpInvocationScope } from "./McpInvocationContext.ts"; | ||
| import * as OrchestratorMcpService from "./OrchestratorMcpService.ts"; | ||
|
|
||
| const parentThreadId = ThreadId.make("thread:queued-task-parent"); | ||
| const childThreadId = ThreadId.make("thread:queued-task-child"); | ||
| const taskId = NodeId.make("node:queued-task"); | ||
| const now = DateTime.makeUnsafe("2026-10-03T11:00:00Z"); | ||
| const providerInstanceId = ProviderInstanceId.make("claude_two"); | ||
| const scope: McpInvocationScope = { | ||
| environmentId: EnvironmentId.make("environment:queued-task"), | ||
| threadId: parentThreadId, | ||
| providerSessionId: "provider-session:queued-task", | ||
| providerInstanceId, | ||
| capabilities: new Set(["orchestration"]), | ||
| issuedAt: 1, | ||
| }; | ||
|
|
||
| function fixture( | ||
| options: { | ||
| published?: boolean; | ||
| executing?: boolean; | ||
| failCancellation?: boolean; | ||
| secondQueued?: boolean; | ||
| deliveryPending?: boolean; | ||
| } = {}, | ||
| ) { | ||
| const projectId = ProjectId.make("project:queued-task"); | ||
| const thread = { | ||
| id: childThreadId, | ||
| projectId, | ||
| title: "Child", | ||
| modelSelection: { instanceId: providerInstanceId, model: "claude-opus-5-5" }, | ||
| lineage: { parentThreadId, relationshipToParent: "subagent", rootThreadId: parentThreadId }, | ||
| createdAt: now, | ||
| updatedAt: now, | ||
| deletedAt: null, | ||
| archivedAt: null, | ||
| settledOverride: null, | ||
| settledAt: null, | ||
| runtimeMode: "full-access", | ||
| interactionMode: "default", | ||
| }; | ||
| const run = (ordinal: number, status: string) => ({ | ||
| id: RunId.make(`run:queued-task:${ordinal}`), | ||
| threadId: childThreadId, | ||
| ordinal, | ||
| status, | ||
| userMessageId: MessageId.make(`message:queued-task:${ordinal}`), | ||
| requestedAt: now, | ||
| startedAt: status === "queued" ? null : now, | ||
| completedAt: status === "failed" || status === "completed" ? now : null, | ||
| queueHeld: status === "queued", | ||
| modelSelection: thread.modelSelection, | ||
| providerInstanceId, | ||
| }); | ||
| const child = { | ||
| thread, | ||
| runs: [ | ||
| run(1, "failed"), | ||
| run(2, "queued"), | ||
| run(3, "completed"), | ||
| run(4, "completed"), | ||
| ...(options.executing ? [run(5, "running")] : []), | ||
| ...(options.secondQueued ? [run(6, "queued")] : []), | ||
| ], | ||
| messages: [ | ||
| { | ||
| id: MessageId.make("result:queued-task:4"), | ||
| runId: RunId.make("run:queued-task:4"), | ||
| role: "assistant", | ||
| text: "Latest completed follow-up result", | ||
| updatedAt: now, | ||
| }, | ||
| ], | ||
| turnItems: [], | ||
| providerThreads: [{ status: "idle", pendingBackgroundTasks: [] }], | ||
| runtimeRequests: [], | ||
| contextTransfers: [], | ||
| subagents: [], | ||
| } as unknown as OrchestrationV2ThreadProjection; | ||
| const parent = { | ||
| thread: { | ||
| ...thread, | ||
| id: parentThreadId, | ||
| lineage: { parentThreadId: null, relationshipToParent: null }, | ||
| }, | ||
| runs: [], | ||
| contextTransfers: [], | ||
| subagents: [ | ||
| { | ||
| id: taskId, | ||
| threadId: parentThreadId, | ||
| childThreadId, | ||
| origin: "app_owned", | ||
| status: options.published ? "completed" : "running", | ||
| providerInstanceId, | ||
| model: "claude-opus-5-5", | ||
| result: options.published ? "Published original result" : null, | ||
| completionDelivery: { state: options.deliveryPending ? "pending" : "disposed" }, | ||
| }, | ||
| ], | ||
| } as unknown as OrchestrationV2ThreadProjection; | ||
| const commands: OrchestrationV2ServerCommand[] = []; | ||
| const layer = OrchestratorMcpService.layer.pipe( | ||
| Layer.provide( | ||
| Layer.mergeAll( | ||
| NodeServices.layer, | ||
| Layer.mock(ThreadManagementService.ThreadManagementService)({ | ||
| getThreadRecords: (id) => Effect.succeed(id === parentThreadId ? parent : child), | ||
| getProjectThreadRecords: () => Effect.succeed(child), | ||
| getTimelinePage: () => Effect.succeed({ items: [], totalItems: 0, hasMore: false }), | ||
| dispatch: (command) => | ||
| Effect.suspend(() => { | ||
| commands.push(command); | ||
| return options.failCancellation && command.type === "queued-run.cancel" | ||
| ? Effect.fail(new Error("Cancellation failed") as never) | ||
| : Effect.succeed({} as never); | ||
| }), | ||
| }), | ||
| Layer.mock(ProviderRegistry.ProviderRegistry)({ getProviders: Effect.succeed([]) }), | ||
| Layer.mock(ProviderAdapterRegistry.ProviderAdapterRegistryV2)({ | ||
| list: () => Effect.succeed([]), | ||
| }), | ||
| Layer.mock(ScheduledTaskService.ScheduledTaskService)({}), | ||
| ), | ||
| ), | ||
| ); | ||
| return { child, parent, commands, layer }; | ||
| } | ||
|
|
||
| it.effect("reports an old held continuation as queued and keeps the latest result readable", () => { | ||
| const { commands, layer } = fixture(); | ||
| return Effect.gen(function* () { | ||
| const service = yield* OrchestratorMcpService.OrchestratorMcpService; | ||
| const task = yield* service.taskStatus(scope, taskId); | ||
| assert.equal(task.status, "queued"); | ||
| assert.equal(task.workState, "working"); | ||
| assert.equal(task.hasPendingChildRuns, true); | ||
| assert.equal(task.latestTerminalRunId, RunId.make("run:queued-task:4")); | ||
| assert.equal(task.latestTerminalSummary, "Latest completed follow-up result"); | ||
| assert.isNull(task.summary); | ||
| const read = yield* service.readThread(scope, { threadId: childThreadId, runLimit: 1 }); | ||
| assert.equal(read.thread.status, "completed"); | ||
| assert.isNull(read.thread.activeRunId); | ||
| assert.equal(read.thread.pendingRequestCount, 0); | ||
| assert.equal(read.thread.queuedRunCount, 1); | ||
| assert.equal(read.thread.heldQueuedRunCount, 1); | ||
| assert.equal(read.recentRuns.length, 1); | ||
| assert.deepEqual(commands, []); | ||
| }).pipe(Effect.provide(layer)); | ||
| }); | ||
|
|
||
| it.effect("cancels queued-only unpublished work without a provider interruption", () => { | ||
| const { commands, child, layer } = fixture({ secondQueued: true }); | ||
| return Effect.gen(function* () { | ||
| const service = yield* OrchestratorMcpService.OrchestratorMcpService; | ||
| const result = yield* service.cancelTask(scope, { taskId, clientRequestId: "cancel-held" }); | ||
| assert.equal(result.status, "cancel_requested"); | ||
| assert.deepEqual( | ||
| commands.map((command) => command.type), | ||
| ["queued-run.cancel"], | ||
| ); | ||
| const command = commands[0]!; | ||
| assert.equal(command.type, "queued-run.cancel"); | ||
| if (command.type !== "queued-run.cancel") | ||
| return yield* Effect.die("Expected queue cancellation"); | ||
| assert.equal(command.threadId, childThreadId); | ||
| assert.equal(command.runId, child.runs[1]!.id); | ||
| assert.deepEqual(command.additionalRunIds, [RunId.make("run:queued-task:6")]); | ||
| }).pipe(Effect.provide(layer)); | ||
| }); | ||
|
|
||
| it.effect("keeps truly executing child work running ahead of held queued work", () => { | ||
| const { layer } = fixture({ executing: true }); | ||
| return Effect.gen(function* () { | ||
| const service = yield* OrchestratorMcpService.OrchestratorMcpService; | ||
| const result = yield* service.taskStatus(scope, taskId); | ||
| assert.equal(result.status, "running"); | ||
| assert.equal(result.hasPendingChildRuns, true); | ||
| }).pipe(Effect.provide(layer)); | ||
| }); | ||
|
|
||
| it.effect("does not resurrect a published task or cancel its later backing-thread work", () => { | ||
| const { commands, layer } = fixture({ published: true, executing: true }); | ||
| return Effect.gen(function* () { | ||
| const service = yield* OrchestratorMcpService.OrchestratorMcpService; | ||
| const status = yield* service.taskStatus(scope, taskId); | ||
| assert.equal(status.status, "completed"); | ||
| assert.equal(status.summary, "Published original result"); | ||
| assert.equal(status.hasPendingChildRuns, true); | ||
| const cancel = yield* service.cancelTask(scope, { | ||
| taskId, | ||
| clientRequestId: "cancel-published", | ||
| }); | ||
| assert.equal(cancel.status, "completed"); | ||
| assert.deepEqual(commands, []); | ||
| }).pipe(Effect.provide(layer)); | ||
| }); | ||
|
|
||
| it.effect("does not dispose completion delivery when queue cancellation fails", () => { | ||
| const { commands, layer } = fixture({ failCancellation: true, deliveryPending: true }); | ||
| return Effect.gen(function* () { | ||
| const service = yield* OrchestratorMcpService.OrchestratorMcpService; | ||
| const error = yield* service | ||
| .cancelTask(scope, { taskId, clientRequestId: "cancel-fails" }) | ||
| .pipe(Effect.flip); | ||
| assert.equal(error.code, "task_not_cancellable"); | ||
| assert.deepEqual( | ||
| commands.map((command) => command.type), | ||
| ["queued-run.cancel"], | ||
| ); | ||
| }).pipe(Effect.provide(layer)); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 High
mcp/OrchestratorMcpService.ts:1551Cancelling one delegated task also cancels unrelated queued work in the same child thread.
queuedRunsincludes every queued run, soadditionalRunIdssends runs created by latersendToThreadmessages toqueued-run.cancelwithout checking that they belong to this task's continuation cohort. Restrict the list to the task-owned cohort before constructingadditionalRunIds.🤖 Copy this AI Prompt to have your agent fix this: