Skip to content

Conversation

@roomote
Copy link
Contributor

@roomote roomote bot commented Aug 21, 2025

Related GitHub Issue

Closes: #7277

Roo Code Task Context (Optional)

This PR was created with assistance from Roo Code to address the reported issue.

Description

This PR fixes an issue where mode instructions and rules were not being included in the environment details when the Power Steering feature was disabled.

Key changes:

  • Removed the conditional check that only included mode roleDefinition and customInstructions when Power Steering was enabled
  • These critical mode details are now always included in environment details, ensuring the LLM has proper context regardless of Power Steering status
  • Updated tests to verify that mode instructions are included in both Power Steering enabled and disabled states

The implementation is minimal and focused - it simply removes the unnecessary conditional wrapper around the mode instruction inclusion logic.

Test Procedure

Testing performed:

  1. Updated the existing test case to verify mode instructions are included with Power Steering both enabled and disabled
  2. Ran the test suite: cd src && npx vitest run core/environment/__tests__/getEnvironmentDetails.spec.ts
  3. All tests pass successfully (15 tests passing)
  4. Linting and type checking pass without warnings

To verify the fix:

  1. Set Power Steering feature to OFF in settings
  2. Switch to a custom mode (e.g., Orchestrator)
  3. Export task history and verify that mode instructions and rules are included in the environment_details section
  4. Repeat with Power Steering ON to confirm it still works

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Testing: New and/or updated tests have been added to cover my changes (if applicable).
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

Not applicable - this is a backend fix that affects how mode instructions are included in prompts.

Documentation Updates

  • No documentation updates are required.

This is a bug fix that restores expected behavior. The mode instructions should always be included regardless of experimental features.

Additional Notes

This was a straightforward fix that addresses the core issue reported by the user. The mode instructions are essential context for the LLM to understand what is expected of each mode, so they should not be gated behind an experimental feature flag.

Get in Touch

Available for any questions about this PR.


Important

Fixes issue where mode instructions were not included when Power Steering was disabled by always including them in getEnvironmentDetails.ts.

  • Behavior:
    • Removes conditional check in getEnvironmentDetails.ts to always include roleDefinition and customInstructions in environment details, regardless of Power Steering status.
    • Ensures LLM has proper context by including mode details in all cases.
  • Tests:
    • Updates test in getEnvironmentDetails.spec.ts to verify mode instructions are included with Power Steering both enabled and disabled.
  • Misc:

This description was created by Ellipsis for 6f4f2c0. You can customize this summary. It will automatically update as commits are pushed.

…tting

- Mode roleDefinition and customInstructions are now always included in environment details
- Previously these were only included when Power Steering was enabled
- Updated tests to verify mode instructions are included in both cases

Fixes #7277
@roomote roomote bot requested review from cte, jr and mrubens as code owners August 21, 2025 13:26
@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. bug Something isn't working labels Aug 21, 2025
Copy link
Contributor Author

@roomote roomote bot left a comment

Choose a reason for hiding this comment

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

Reviewing my own code is like debugging in a mirror - everything looks backward but the bugs are still mine.

details += `<slug>${currentMode}</slug>\n`
details += `<name>${modeDetails.name}</name>\n`
details += `<model>${modelId}</model>\n`
details += `<role>${modeDetails.roleDefinition}</role>\n`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Clean fix! This correctly ensures mode instructions are always included. Though after this change, you might want to check if the EXPERIMENT_IDS import is still needed in this file since we're no longer checking for POWER_STEERING.

})

it("should include experiment-specific details when Power Steering is enabled", async () => {
it("should always include mode role and custom instructions regardless of Power Steering", async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good test coverage for both scenarios! The test name clearly indicates the behavior we're verifying - that mode instructions should always be included regardless of experimental features.

@hannesrudolph hannesrudolph added the Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. label Aug 21, 2025
@jeepshop
Copy link

I don't know the code base all that well, does this fix only effect the first instructions send to the LLM or all instructions sent to the LLM? I thought Power Steering was used to occasionally include more details (not always) but the first instructions sent should always include all the environment details in the instruction.

I'm not asking the BOT to make code changes, but to make sure the proposed change does what I thought the intent and purpose of power steering was.

@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Aug 22, 2025
@hannesrudolph hannesrudolph added PR - Needs Preliminary Review and removed Issue/PR - Triage New issue. Needs quick review to confirm validity and assign labels. labels Aug 22, 2025
@daniel-lxs
Copy link
Member

#7277 (comment)

@daniel-lxs daniel-lxs closed this Aug 26, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Aug 26, 2025
@github-project-automation github-project-automation bot moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Aug 26, 2025
@daniel-lxs daniel-lxs deleted the feature/fix-mode-rules-without-power-steering branch August 26, 2025 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working PR - Needs Preliminary Review size:S This PR changes 10-29 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

No Mode rules without Power Steering

5 participants