-
Notifications
You must be signed in to change notification settings - Fork 532
Feat: Computer tool - add invoke method with RunContext #761
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
Conversation
🦋 Changeset detectedLatest commit: 69748c6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Thanks for sharing this implementation idea. However, I don't think adding new "invoke" method is the only and best way to achieve the goal to separate different computer runs. We prefer a simpler fix like detecting simultaneous executions using the same computer instance or anything else, which does not require existing computer implementations to change their internals. |
The main goal was to add user context to agent method executions, similar to how regular tools work. |
|
Here is my approach for resolving this issue: #771 If you have any comments, please feel free to share them! |
|
Thanks again for sharing this idea. Closing this PR in favor of #771 |
This PR resolves issue #663.
Right now Computer tool should implement each UI action as a separate method. These methods also don't have RunContext parameter opposite to regular tools. It makes impossible passing user context when executing UI actions.
The fix adds:
One common method for all UI actions with
RunContext(contains user context) andComputerUseCallItem(contains UI actions).The fix keeps backward compatibility with the regular methods definition.
Invoke method implementation example: