fix(modes): prevent missing role definition when selecting default mode

[devxpain]

Related GitHub Issue

Closes: #4285

Description

• Previously, getModeSelection could pick promptComponent as the active mode even if it lacked a roleDefinition.
• This resulted in an empty roleDefinition being returned, overriding potentially valid role definitions from built-in modes.
• The logic now only considers promptComponent if it provides a roleDefinition, ensuring that a proper role definition is always selected if available from custom, prompt, or built-in modes.

Test Procedure

Same as the Issue: #4285

Type of Change

• 🐛 Bug Fix: Non-breaking change that fixes an issue.
• ✨ New Feature: Non-breaking change that adds functionality.
• 💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
• ♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
• 💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
• 📚 Documentation: Updates to documentation files.
• ⚙️ Build/CI: Changes to the build process or CI configuration.
• 🧹 Chore: Other changes that don't modify src or test files.

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.
• Code Quality:
  • My code adheres to the project's style guidelines.
  • There are no new linting errors or warnings (npm run lint).
  • All debug code (e.g., console.log) has been removed.
• Testing:
  • New and/or updated tests have been added to cover my changes.
  • All tests pass locally (npm test).
  • The application builds successfully with my changes.
• Branch Hygiene: My branch is up-to-date (rebased) with the main branch.
• Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
• Changeset: A changeset has been created using npm run changeset if this PR includes user-facing changes or dependency updates.
• Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

BEFORE FIX

AFTER_FIX

Documentation Updates

Get in Touch

---

Important

Fixes getModeSelection in modes.ts to ensure promptComponent is only considered if it has a roleDefinition, preventing empty role definitions from overriding valid ones.

• Behavior:
  • Fixes getModeSelection in modes.ts to only consider promptComponent if it has a roleDefinition.
  • Prevents empty roleDefinition from overriding valid ones from custom or built-in modes.
• Misc:
  • Closes issue missing role definition when selecting default mode #4285.

^{This description was created by for 89ffd9bcce14efcce9aabc958126b2d5ffb1d10c. You can customize this summary. It will automatically update as commits are pushed.}

[mrubens]

Did this break in #3791? Maybe we should revert that. I'm not sure that only considering promptComponents with roleDefinitions is the right fix.

[devxpain]

Did this break in #3791? Maybe we should consider reverting it. I'm not sure that filtering only promptComponents with roleDefinition is the right fix.

The only difference in my change is the use of (promptComponent?.roleDefinition ? promptComponent : undefined), compared to the original. I don’t believe this breaks #3791.
As for whether #3791 itself needs to be reverted — I’m not sure. I’d prefer not to review or recommend anything unless it causes another issue later.
I think the better approach is to ask @ChuKhaLi, the original author of #3791, to take a look.

[daniel-lxs]

To clarify how this fix works: when a promptComponent lacks a roleDefinition, it's completely skipped. So if someone has a promptComponent with only customInstructions, those instructions are lost and we use the built-in mode entirely.

This is an all-or-nothing approach - either the promptComponent is complete (has roleDefinition) and we use it, or it's incomplete and we skip it.

An alternative approach could be to merge them - take the roleDefinition from the built-in mode when missing, but preserve any customInstructions from the promptComponent. Something like:

const customMode = findModeBySlug(mode, customModes)
const builtInMode = findModeBySlug(mode, modes)

if (customMode) return customMode
if (promptComponent?.roleDefinition) return promptComponent
if (promptComponent?.customInstructions && builtInMode) {
  // Merge: use built-in roleDefinition with prompt's customInstructions
  return {
    roleDefinition: builtInMode.roleDefinition || "",
    customInstructions: promptComponent.customInstructions
  }
}
return builtInMode

But the current fix is simpler and solves the immediate issue of empty role definitions for certain modes.

[devxpain]

To clarify how this fix works: when a promptComponent lacks a roleDefinition, it's completely skipped. So if someone has a promptComponent with only customInstructions, those instructions are lost and we use the built-in mode entirely.

This is an all-or-nothing approach - either the promptComponent is complete (has roleDefinition) and we use it, or it's incomplete and we skip it.

An alternative approach could be to merge them - take the roleDefinition from the built-in mode when missing, but preserve any customInstructions from the promptComponent. Something like:

const customMode = findModeBySlug(mode, customModes)
const builtInMode = findModeBySlug(mode, modes)

if (customMode) return customMode
if (promptComponent?.roleDefinition) return promptComponent
if (promptComponent?.customInstructions && builtInMode) {
  // Merge: use built-in roleDefinition with prompt's customInstructions
  return {
    roleDefinition: builtInMode.roleDefinition || "",
    customInstructions: promptComponent.customInstructions
  }
}
return builtInMode

But the current fix is simpler and solves the immediate issue of empty role definitions for certain modes.

Thanks @daniel-lxs for clarifying the issue — that was really helpful to me. Sorry I’ve been quite busy with my own projects, so I’ve been aiming to provide the minimal fixes and features that at least won’t break anything on my end.

Let me know what you guys decide. I’m happy to switch to your approach or keep mine.
Once again, thanks for taking the time to look into it!

[daniel-lxs]

No problem, just moved it to draft to avoid accidental merging until we decide.

In what situations would a promptComponent not have a role definition? Can we consider that invalid?

Maybe we can define a more generic role definition, I think the code mode role definition is quite generic, it could work for promptComponents that are incomplete.

Let me know what you think.

cc: @mrubens

[hannesrudolph]

stale