browser configuration, proxy configuration and user profile for browser session

[wintonzheng]

Important

Add proxy and user profile configurations to browser sessions, updating database schema, models, and session management.

• Database Schema:
  • Adds proxy_configuration, browser_configuration, and user_profile columns to persistent_browser_sessions table in 2025_07_17_0427-05c722f2f859_add_browser_configuration_proxy_.py.
• Models:
  • Updates PersistentBrowserSessionModel in models.py to include proxy_configuration, browser_configuration, and user_profile.
• Session Management:
  • Updates create_persistent_browser_session() in client.py to accept new configuration parameters.
  • Modifies create_session() in persistent_sessions_manager.py to handle new configurations.
• API Changes:
  • Updates create_browser_session() in browser_sessions.py to accept proxy_location from request.
• Schemas:
  • Updates PersistentBrowserSession in persistent_browser_sessions.py to include new fields.
  • Adds proxy_location to CreateBrowserSessionRequest in browser_sessions.py.

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

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to 2bf24bb in 1 minute and 41 seconds. Click for details.

• Reviewed 150 lines of code in 7 files
• Skipped 0 files when reviewing.
• Skipped posting 5 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. skyvern/forge/sdk/schemas/persistent_browser_sessions.py:15

• Draft comment:
  The Pydantic model now includes new fields (proxy_configuration, browser_configuration, user_profile). Consider adding example values in the Field definitions to improve API documentation consistency.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% 1. The comment is about the changed code (new fields). 2. The suggestion would improve documentation. 3. However, none of the other fields use Field definitions with examples. 4. Adding examples only to these fields would be inconsistent. 5. Without seeing how these dicts are used, we can't know what good examples would be. We don't know if Field examples are used anywhere else in the codebase - this could be a valid pattern in other files. Also, examples could be genuinely helpful for complex dict structures. Without seeing the broader codebase patterns and knowing the expected structure of these dicts, adding example values could be misleading or inconsistent. Delete the comment. While documentation improvements are good, suggesting examples only for these fields without broader context could lead to inconsistency.

2. skyvern/schemas/browser_sessions.py:17

• Draft comment:
  The CreateBrowserSessionRequest schema only accepts proxy_location. If desired, consider extending it to accept browser_configuration and user_profile, or document why they are omitted.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. skyvern/webeye/persistent_sessions_manager.py:119

• Draft comment:
  In create_session, the default proxy_configuration is constructed using the proxy_location. Confirm this is the intended behavior and consider allowing more customization if needed.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 50% <= threshold 50% The comment is asking the author to confirm their intention regarding the use of proxy_location in constructing the default proxy_configuration. This violates the rule against asking the author to confirm their intention. However, it also suggests considering more customization, which is a valid suggestion. The comment is partially useful but needs to be rephrased to focus on the suggestion rather than confirmation.

4. skyvern/webeye/persistent_sessions_manager.py:73

• Draft comment:
  In get_browser_address, the address string is split on ':'. Consider adding error handling in case the expected colon-delimited format is not met.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

5. skyvern/webeye/schemas.py:32

• Draft comment:
  Good inclusion of the proxy_configuration field in the BrowserSessionResponse model. Verify that the examples provided meet client expectations and consider adding examples for browser_configuration and user_profile if these are to be exposed later.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 20% <= threshold 50% The comment starts with a purely informative statement about the inclusion of a field, which is not allowed. It then asks the PR author to verify if examples meet client expectations, which is against the rules. However, it also suggests considering adding examples for other fields if they are to be exposed later, which is a valid suggestion. Overall, the comment mostly violates the rules.

Workflow ID: wflow_45cntQa3NL4Ht6sZ

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

New JSON columns (proxy_configuration, browser_configuration, user_profile) have been added to PersistentBrowserSessionModel. Ensure a database migration is provided and that clients are aware of these new fields.

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 2bf24bb in 2 minutes and 6 seconds. Click for details.

• Reviewed 150 lines of code in 7 files
• Skipped 0 files when reviewing.
• Skipped posting 6 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. skyvern/forge/sdk/db/models.py:697

• Draft comment:
  New JSON columns (proxy_configuration, browser_configuration, user_profile) are added to PersistentBrowserSessionModel. Consider whether defaults or additional validation/sanitization are needed if these may contain sensitive or user-supplied data.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While the comment raises a valid security consideration about data validation, it's somewhat speculative ("Consider whether...") and doesn't point to a specific issue that needs fixing. The columns are already properly typed as JSON and nullable. Without more context about the actual data that will be stored or specific security requirements, this is more of a general suggestion than an actionable issue. The comment does raise an important security consideration that could be relevant. Perhaps there are specific validation requirements that I'm not aware of. However, the comment is speculative and doesn't point to any specific validation requirements or security issues that need addressing. It's asking the author to "consider" something rather than pointing out a concrete problem. The comment should be removed as it is speculative and doesn't point to a specific issue that needs fixing. If there are specific validation requirements, they should be raised as concrete suggestions.

2. skyvern/forge/sdk/routes/browser_sessions.py:46

• Draft comment:
  The create_browser_session endpoint only passes proxy_location from the request. If clients should also configure browser_configuration and user_profile, consider extending the CreateBrowserSessionRequest schema and mapping these parameters.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

3. skyvern/schemas/browser_sessions.py:10

• Draft comment:
  CreateBrowserSessionRequest does not include fields for browser_configuration or user_profile. If clients are meant to supply these settings, add them to the request model.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

4. skyvern/webeye/persistent_sessions_manager.py:119

• Draft comment:
  In create_session, browser_configuration and user_profile are passed unmodified to the database call. Ensure these inputs are validated as needed for consistency and security.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is asking the author to ensure that inputs are validated for consistency and security. This falls under asking the author to ensure something is done, which is against the rules. The comment does not provide a specific suggestion or point out a specific issue with the code.

5. skyvern/webeye/schemas.py:32

• Draft comment:
  BrowserSessionResponse exposes proxy_configuration but omits browser_configuration and user_profile. Confirm whether these should be included in the API response.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

6. skyvern/webeye/schemas.py:39

• Draft comment:
  Typo: Consider changing 'Url' to 'URL' for consistency and clarity.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_4ysPea7mZh6fXcjO

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed aa051d1 in 52 seconds. Click for details.

• Reviewed 41 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 3 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. alembic/versions/2025_07_17_0427-05c722f2f859_add_browser_configuration_proxy_.py:1

• Draft comment:
  File name includes a trailing underscore. Remove it if unintentional for consistency.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

2. alembic/versions/2025_07_17_0427-05c722f2f859_add_browser_configuration_proxy_.py:24

• Draft comment:
  Using sa.JSON() may be DB-specific. Ensure your target database supports JSON columns.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 0% <= threshold 50% The comment is suggesting to ensure compatibility with the target database, which is not allowed by the rules. It doesn't provide a specific code suggestion or ask for a specific test to be written.

3. alembic/versions/2025_07_17_0427-05c722f2f859_add_browser_configuration_proxy_.py:23

• Draft comment:
  Consider removing or clarifying the auto-generated Alembic comment blocks to keep the migration clean.
• Reason this comment was not posted:
  Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_05uLj68NCLA7V4WA

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}