Skip to content

feat(contextProxy): add setValue and setValues methods to simplify#1396

Merged
cte merged 1 commit intoRooCodeInc:mainfrom
samhvw8:refactor-context-proxy-more
Mar 5, 2025
Merged

feat(contextProxy): add setValue and setValues methods to simplify#1396
cte merged 1 commit intoRooCodeInc:mainfrom
samhvw8:refactor-context-proxy-more

Conversation

@samhvw8
Copy link
Contributor

@samhvw8 samhvw8 commented Mar 5, 2025

  • Added new setValue method to ContextProxy to route keys to either secrets or global state
  • Added setValues method to process multiple key-value pairs at once
  • Updated ClineProvider to use new methods, reducing code duplication
  • Added comprehensive test coverage for new methods

This change is part of the larger ClineProvider refactoring effort to improve state management and reduce complexity, as outlined in the refactoring plan documents.

Context

Follow up @cte on previous context proxy PR

#1235 (comment)

Implementation

Screenshots

before after

How to Test

Get in Touch


Important

Add setValue and setValues methods to ContextProxy for streamlined state management, integrated into ClineProvider with comprehensive tests.

  • Behavior:
    • Add setValue and setValues methods to ContextProxy to route keys to secrets or global state.
    • setValue handles single key-value pairs; setValues handles multiple pairs.
    • Unknown keys default to global state with a warning.
  • Integration:
    • Update ClineProvider to use setValues for API configuration, reducing code duplication.
  • Testing:
    • Add tests for setValue and setValues in contextProxy.test.ts to ensure correct routing and caching behavior.

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

…ate management

- Added new setValue method to ContextProxy to route keys to either secrets or global state
- Added setValues method to process multiple key-value pairs at once
- Updated ClineProvider to use new methods, reducing code duplication
- Added comprehensive test coverage for new methods

This change is part of the larger ClineProvider refactoring effort to improve state management and reduce complexity, as outlined in the refactoring plan documents.
@samhvw8 samhvw8 requested review from cte and mrubens as code owners March 5, 2025 17:03
@changeset-bot
Copy link

changeset-bot bot commented Mar 5, 2025

⚠️ No Changeset found

Latest commit: b1b51f8

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 Mar 5, 2025
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 5, 2025
promises.push(this.setValue(key, value))
}

return Promise.all(promises)
Copy link
Contributor

Choose a reason for hiding this comment

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

The setValues method uses Promise.all which may fail fast if one operation errors. Consider using Promise.allSettled if partial success is acceptable. (Development Standards)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, TIL about Promise.allSettled - thanks @ellipsis-dev!

Copy link
Contributor

Choose a reason for hiding this comment

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

Would you like me to update the setValues method to use Promise.allSettled instead of Promise.all for handling multiple key-value pairs?

@cte cte merged commit e1fb929 into RooCodeInc:main Mar 5, 2025
17 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement New feature or request lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants