Skip to content

Conversation

@Cdddo
Copy link

@Cdddo Cdddo commented Apr 11, 2025

Context

I split the setting to auto-approve subtask creation and completion into two separate settings. Having them be combined is a nuisance because it's generally fine to let Roo create subtasks automatically, but unfortunately too often it tries to complete them when they're still incomplete or buggy. This way we can allow it to make all the subtasks it wants but give the user control on when they are truly completed.

Implementation

I added a new setting "AlwaysAllowSubtaskCompletion" and also renamed the older setting from "AlwaysAllowSubtasks" to "AlwaysAllowSubtaskCreation" for clarity. This means that the settings users had saved for subtasks will be now set to the default values.

Screenshots

Highlight_20250410205659_761x463 Highlight_20250410205752_800x416

How to Test

Use boomerang mode. Play with the auto-approve settings.

Get in Touch

My discord handle in the RooCode server is Hyper.


Important

Splits subtask approval into separate settings for creation and completion, updating code and UI components accordingly.

  • Behavior:
    • Splits AlwaysAllowSubtasks into AlwaysAllowSubtaskCreation and AlwaysAllowSubtaskCompletion for separate control over subtask creation and completion.
    • Updates default settings to reflect this change, affecting user-saved settings.
  • Code Changes:
    • Updates subtasks.test.ts, roo-code-defaults.ts, and roo-code.ts to use new settings.
    • Modifies ClineProvider.ts and webviewMessageHandler.ts to handle new settings.
    • Adjusts UI components in AutoApproveMenu.tsx, ChatView.tsx, and AutoApproveSettings.tsx to reflect new settings.
  • Localization:
    • Updates localization files across multiple languages to include new setting labels and descriptions.

This description was created by Ellipsis for 762ec17. It will automatically update as commits are pushed.

@changeset-bot
Copy link

changeset-bot bot commented Apr 11, 2025

⚠️ No Changeset found

Latest commit: 1f30f1a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. enhancement New feature or request labels Apr 11, 2025
mode: "ask",
alwaysAllowModeSwitch: true,
alwaysAllowSubtasks: true,
alwaysAllowSubtaskCreation: true,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only feedback on this - I wonder if we should stick to the alwaysAllowSubtasks key for the creation, and preserve their current setting. Otherwise the settings are going to get cleared out for everyone right?

I guess we could also add migration logic that sets the value of both new keys to the value of the old key - that might not be so bad. What do you think?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially I didn't change it for the same reason, but as I was working on it I thought it would be important to maintain clarity in the code going forward (already too many references to "subtasks".)

Adding some migration logic I suppose would be easy, just have no idea where exactly to put it. Even easier would be to just put a notice in the changelog that this setting will now be reset to defaults and people should double check their config after the update. People do read changelogs, right? That way there's no need to leave some migration code hanging around.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think people read in general :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do have a migrateSettings method called from extension.ts

@hannesrudolph hannesrudolph moved this from New to PR [Pre Approval Review] in Roo Code Roadmap Apr 14, 2025
@hannesrudolph hannesrudolph moved this from New to PR [Pre Approval Review] in Roo Code Roadmap May 20, 2025
@hannesrudolph hannesrudolph moved this from PR [Needs Review] to TEMP in Roo Code Roadmap May 26, 2025
@daniel-lxs daniel-lxs moved this from TEMP to PR [Needs Review] in Roo Code Roadmap May 26, 2025
@hannesrudolph
Copy link
Collaborator

@Cdddo We're going to close this because it seems to add complexity for an edge case that may not be broadly applicable. If the user must manually approve task completion, it would effectively shift Roo into a semi-automatic state, defeating much of the advantage of automatic orchestration. Typically, orchestration should either fully automate or require consistent manual oversight, not sit in-between.

We believe the scenario you're addressing could be better handled through custom modes or tailored prompting.

That said, we're happy to explore this further if you'd like to open a detailed issue proposal following our issue-first approach.

We appreciate your effort. Sorry for the delay on this.

@github-project-automation github-project-automation bot moved this from PR [Pre Approval Review] to Done in Roo Code Roadmap May 27, 2025
@github-project-automation github-project-automation bot moved this from Needs Preliminary Review to Done in Roo Code Roadmap May 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request PR - Needs Preliminary Review size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants