-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: Cannot report sub-task results in Orchestrator mode #6658
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
fix: Cannot report sub-task results in Orchestrator mode #6658
Conversation
The `rootTask` and `parentTask` properties in the `Task` class were incorrectly initialized, preventing sub-tasks from properly reporting their results back to the main Orchestrator task. This commit fixes the initialization to ensure correct task hierarchy and result propagation. The issue is with how TypeScript handles `readonly` properties when they have explicit initializers. Let me demonstrate: ```typescript readonly rootTask: Task | undefined readonly parentTask: Task | undefined ``` ```typescript readonly rootTask: Task | undefined = undefined readonly parentTask: Task | undefined = undefined ``` When you declare a `readonly` property with an explicit initializer like `= undefined`, TypeScript treats this as a **definitive assignment** that happens at the class field level, **before** the constructor runs. Here's what happens: 1. Class is instantiated 2. Constructor runs 3. `this.parentTask = parentTask` successfully assigns the parent task 4. Property now contains the actual parent task reference 1. Class is instantiated 2. **TypeScript immediately sets `this.parentTask = undefined` due to the field initializer** 3. Constructor runs 4. `this.parentTask = parentTask` tries to assign, but TypeScript may optimize this away or the field initializer takes precedence 5. Property remains `undefined` even though we tried to assign it Fixes RooCodeInc#1742
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.
Thank you for your contribution! I've reviewed the changes and the fix correctly addresses the TypeScript initialization issue. The PR description provides excellent context explaining the problem and solution.
Suggestions:
-
Add unit tests: Consider adding tests for the
rootTaskandparentTaskfunctionality to prevent regression of this bug. Currently, there are no tests covering these properties, and having tests would ensure sub-task result reporting continues to work correctly in Orchestrator mode. -
Documentation: Consider updating any relevant documentation about the Orchestrator mode to ensure it reflects that sub-task result reporting is now working correctly.
|
|
||
| readonly rootTask: Task | undefined = undefined | ||
| readonly parentTask: Task | undefined = undefined | ||
| readonly rootTask: Task | undefined |
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.
Good fix! This correctly addresses the TypeScript initialization issue. However, consider adding a brief comment explaining why these properties don't have explicit initializers, as this subtle behavior might not be obvious to all developers:
| readonly rootTask: Task | undefined | |
| // Note: No explicit initializers to allow constructor assignment | |
| readonly rootTask: Task | undefined | |
| readonly parentTask: Task | undefined |
|
Hey @hassoncs I'm not entirely sure what problem is being solved here, I tested both main and production and the subtask result seems to be visible to the parent task, am I missing something? |
|
Hmm, the bug we were seeing is that Orchestrator dispatches a sub-task, but that sub-task is unable to report completion to the parent. When I debugged it, the problem was the Maybe this was only a bug on our side?? I'll do some more testing and close this if I can't reproduce it! Sorry for the confusion |
|
@daniel-lxs, yeah I can't seem to reproduce it!! I assumed the bug would be the same 😬 Closing this! Sorry 😅 |
|
@hassoncs No problem, thank you for taking the time to fix a potential issue! |
The
rootTaskandparentTaskproperties in theTaskclass were incorrectly initialized, preventing sub-tasks from properly reporting their results back to the main Orchestrator task. This commit fixes the initialization to ensure correct task hierarchy and result propagation.The issue is with how TypeScript handles
readonlyproperties when they have explicit initializers. Let me demonstrate:When you declare a
readonlyproperty with an explicit initializer like= undefined, TypeScript treats this as a definitive assignment that happens at the class field level, before the constructor runs.Here's what happens:
Class is instantiated
Constructor runs
this.parentTask = parentTasksuccessfully assigns the parent taskProperty now contains the actual parent task reference
Class is instantiated
TypeScript immediately sets
this.parentTask = undefineddue to the field initializerConstructor runs
this.parentTask = parentTasktries to assign, but TypeScript may optimize this away or the field initializer takes precedenceProperty remains
undefinedeven though we tried to assign itGif of the working fix:

Fixes: Kilo-Org/kilocode#1742
Important
Fixes incorrect initialization of
rootTaskandparentTaskinTaskclass to ensure proper sub-task result reporting in Orchestrator mode.rootTaskandparentTaskinTaskclass inTask.ts, ensuring sub-tasks report results correctly in Orchestrator mode.readonly rootTaskandreadonly parentTaskproperties to prevent TypeScript from overriding constructor assignments.This description was created by
for 42bd8b9. You can customize this summary. It will automatically update as commits are pushed.