Merged
Conversation
Add login_method attribute to StormContext and use it throughout the codebase instead of directly accessing settings. Update error messages to dynamically show "Login" or "Create profiles" based on the configured login method.
Add a new /add_account endpoint that allows users to log in with additional Google accounts to the existing environment. Includes backend support in Profiles and UndetectedDrivers classes, and a UI button in the CookieLogin component for triggering account addition.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis pull request introduces support for adding multiple Google accounts to an environment. It adds a new API endpoint, backend account-management logic, updates login-method tracking, enhances channel selection, and extends the frontend UI with account-addition controls and documentation. Changes
Sequence DiagramsequenceDiagram
participant User as User (UI)
participant CookieLogin as CookieLogin.jsx
participant ProfileAPI as ProfileRouter<br/>(add_account endpoint)
participant Profiles as Profiles<br/>(add_account method)
participant UndetectedDrivers as UndetectedDrivers<br/>(youtube_login)
participant Google as Google<br/>(AddSession)
User->>CookieLogin: Click "Add Another Google Account"
activate CookieLogin
CookieLogin->>CookieLogin: Set addAccount.loading = true
CookieLogin->>ProfileAPI: POST /environment/profiles/add_account
deactivate CookieLogin
activate ProfileAPI
ProfileAPI->>ProfileAPI: Set busy state
ProfileAPI->>Profiles: Call add_account() in threadpool
deactivate ProfileAPI
activate Profiles
Profiles->>Profiles: Retrieve cookies via get_cookies()
alt Cookies not found
Profiles-->>ProfileAPI: Raise ValueError
else Cookies found
Profiles->>UndetectedDrivers: Instantiate with base_profile_dir
Profiles->>UndetectedDrivers: Call initiate_base_profile()
Profiles->>UndetectedDrivers: Call youtube_login(add_account=True)
end
deactivate Profiles
activate UndetectedDrivers
alt add_account=True
UndetectedDrivers->>Google: Navigate to AddSession URL
Google-->>UndetectedDrivers: Display account selection
end
deactivate UndetectedDrivers
alt Success path
UndetectedDrivers-->>ProfileAPI: Return success
ProfileAPI-->>CookieLogin: Return 200 with success message
activate CookieLogin
CookieLogin->>CookieLogin: Update notification
CookieLogin->>CookieLogin: Log analytics event
CookieLogin->>CookieLogin: Set addAccount.loading = false
deactivate CookieLogin
else ValueError (missing cookies)
Profiles-->>ProfileAPI: ValueError
ProfileAPI-->>CookieLogin: Return 400 with error message
activate CookieLogin
CookieLogin->>CookieLogin: Display addAccount.error
CookieLogin->>CookieLogin: Set addAccount.loading = false
deactivate CookieLogin
else Other exception
ProfileAPI-->>CookieLogin: Return 500 with failure message
activate CookieLogin
CookieLogin->>CookieLogin: Set addAccount.loading = false
deactivate CookieLogin
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Adds the /environment/profiles/add_account endpoint to enable adding additional Google accounts to environment profiles.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Deploying streamstorm-ui with
|
| Latest commit: |
424d6cb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://00206bf6.streamstorm.pages.dev |
| Branch Preview URL: | https://feat-multiple-google-account.streamstorm.pages.dev |
Add mock_logged_in and mock_get_cookies fixtures to ensure API tests behave consistently in CI where cookie files don't exist on disk. Also update atatus dependency to 2.3.0.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Engine/lib/core/UndetectedDrivers.py (1)
27-38:⚠️ Potential issue | 🟡 MinorAdd
add_account_urlto__slots__for consistency.Line 38 assigns
self.add_account_url, but it is not declared in the__slots__tuple on line 27. While this will work (becauseBrowserAutomatorlacks__slots__, providing a__dict__), it contradicts the other URL attributes (youtube_url,youtube_login_url) which are properly declared in__slots__. This inconsistency defeats the memory optimization intent of__slots__and creates a fragile dependency on the parent class structure.Proposed fix
- __slots__: tuple[str, ...] = ('base_profile_dir', 'youtube_login_url', 'data_json_path', 'youtube_url') + __slots__: tuple[str, ...] = ('add_account_url', 'base_profile_dir', 'youtube_login_url', 'data_json_path', 'youtube_url')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Engine/lib/core/UndetectedDrivers.py` around lines 27 - 38, The class defines __slots__ but forgets to include 'add_account_url'; update the __slots__ tuple in the class (where __slots__ is declared) to include 'add_account_url' so it matches attributes set in __init__ (youtube_url, youtube_login_url, add_account_url), preserving the intent of using __slots__ for memory optimization and avoiding accidental __dict__ usage.
🧹 Nitpick comments (5)
src/Engine/lib/api/routers/ProfileRouter.py (1)
253-266: Uselogger.exceptionfor richer error diagnostics.Per Ruff TRY400,
logger.exceptioncaptures the traceback automatically, aiding debugging. The ValueError handler also benefits from this.Proposed fix
except ValueError as e: - logger.error(f"Error while adding account: {e}") + logger.exception(f"Error while adding account: {e}") EngineContext.reset()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Engine/lib/api/routers/ProfileRouter.py` around lines 253 - 266, In the except ValueError block handling profiles.add_account, replace the logger.error call with logger.exception so the traceback is captured (keep the same formatted message), then proceed to call EngineContext.reset() and return the JSONResponse as before; specifically update the except handler that references profiles.add_account, logger, EngineContext.reset, and JSONResponse to use logger.exception for richer diagnostics.src/Engine/lib/core/Profiles.py (1)
120-127: Unused walrus-operator variablecookies.Ruff correctly flags
cookiesas assigned but never used (F841). Sinceget_cookies()is only called to verify cookies exist, simplify:♻️ Proposed fix
def add_account(self) -> None: - if (cookies:= get_cookies()) is None: + if get_cookies() is None: raise ValueError("No cookies found. Login first.") UD: UndetectedDrivers = UndetectedDrivers(self.base_profile_dir) UD.initiate_base_profile() # Here we are not passing cookies because the base profile already has the required cookies. UD.youtube_login(add_account=True)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Engine/lib/core/Profiles.py` around lines 120 - 127, The add_account method assigns cookies with the walrus operator but never uses it, causing an unused-variable lint error; change the check to call get_cookies() directly (e.g., if get_cookies() is None: raise ValueError(...)) and remove the unused "cookies" binding so add_account only verifies existence of cookies before instantiating UndetectedDrivers and calling UD.initiate_base_profile() and UD.youtube_login(add_account=True).src/Engine/lib/core/StormContext.py (1)
39-39:login_methodis absent fromget()and could use aLiteraltypeTwo minor observations:
login_methodis not included in theget()method (lines 64–87). If any client consuming the context snapshot (e.g., an MCP tool or the UI via/storm/context) needs to know the login method, it would silently receive a stale default.- Only
"cookies"and"profiles"appear to be valid values; narrowing the annotation toLiteral["cookies", "profiles"]makes invalid states immediately visible to type checkers.♻️ Proposed refinement
- self.login_method: str = "cookies" + self.login_method: Literal["cookies", "profiles"] = "cookies"And in
get():context["start_time"] = self.start_time + context["login_method"] = self.login_method return contextAlso update the import at the top of the file:
-from typing import Any, Literal +from typing import Any, Literal # Literal already imported — no change needed🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Engine/lib/core/StormContext.py` at line 39, The context's login_method field is currently typed as a generic str and omitted from the get() snapshot; update the declaration of login_method to use typing.Literal with the two allowed values (Literal["cookies","profiles"]) and add login_method to the dictionary returned by the get() method so consumers receive the current value; also add the Literal import at the top of the file (from typing import Literal or adjust existing imports) and ensure any places constructing the snapshot (method get) include the login_method key.src/UI/src/Components/Modals/ManageProfiles/Sections/CookieLogin.jsx (1)
147-186:handleAddAccountduplicates ~90% ofhandleCookieLogin's fetch/error/analytics patternThe two handlers share identical shape: clear error → set loading → log start event → fetch → handle success/failure with notification + analytics → catch with atatus.notify + analytics → finally clear loading. Extracting a shared helper reduces the copy-paste surface:
♻️ Suggested refactor
+ const handleProfileAction = ({ url, analyticsPrefix, onSuccess }) => { + setErrorText(""); // caller can override per-state via onSuccess/onFailure callbacks + return fetch(url, { method: "POST" }) + .then((r) => r.json()) + .then((data) => { + if (data.success) { + onSuccess(data); + logEvent(analytics, `${analyticsPrefix}_success`); + } else { + notifications.show(`Failed.`, { severity: "error" }); + if (data.error) atatus.notify(new Error(data.error), { response: data }, [`${analyticsPrefix}_failed`]); + logEvent(analytics, `${analyticsPrefix}_failed`); + return data; // propagate for per-caller error state + } + }) + .catch((error) => { + notifications.show("An error occurred. Try again.", { severity: "error" }); + atatus.notify(error, {}, [`${analyticsPrefix}_error`]); + logEvent(analytics, `${analyticsPrefix}_error`); + }); + };Each caller then wraps this with its own loading state and error state updates, passing only what differs (URL, analytics prefix, success callback).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/UI/src/Components/Modals/ManageProfiles/Sections/CookieLogin.jsx` around lines 147 - 186, Both handleAddAccount and handleCookieLogin repeat the same fetch/notification/analytics/error flow; extract that shared behavior into a single helper (e.g., performFetchWithAnalytics or sharedFetchHandler) that accepts the URL, analytics event names (start/success/failed/error), success message/handler, and optional error handlers; update handleAddAccount and handleCookieLogin to call this helper while retaining their own state setters (setAddAccountLoading/setAddAccountError and setCookieLoginLoading/setCookieLoginError) before/after the helper call so each caller only manages its loading/error state and any unique success callback.src/Engine/lib/api/routers/StormRouter.py (1)
417-417: Extract conditional to variable for clarityThe nested f-string on line 417 uses double quotes inside a double-quoted f-string. While this is valid syntax for Python 3.12+, the project targets Python 3.13, so it works fine in the current CI environment. However, extracting the conditional expression to a separate variable improves code clarity and readability.
The same pattern also appears in
src/Engine/lib/core/StreamStorm.pyaround line 181 ("create Profiles" if self.context.login_method == "profiles" else "Login").Suggested refactoring
+ action = "Create profiles" if settings.login_method == "profiles" else "Login" return JSONResponse( status_code=404, content={ "success": False, - "message": f"Config file not found. {"Create profiles" if settings.login_method == "profiles" else "Login"} first.", + "message": f"Config file not found. {action} first.", } )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Engine/lib/api/routers/StormRouter.py` at line 417, Extract the inline conditional used in the f-string to a clearly named variable to improve readability: for the message in StormRouter (the f-string that currently contains {"Create profiles" if settings.login_method == "profiles" else "Login"}), create a variable like action_text = "Create profiles" if settings.login_method == "profiles" else "Login" and then use f"Config file not found. {action_text} first."; apply the same refactor in StreamStorm where the expression "create Profiles" if self.context.login_method == "profiles" else "Login" is used, assigning it to a local variable (e.g., action_text) and replacing the inline conditional in the string with that variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Engine/lib/api/routers/ProfileRouter.py`:
- Around line 268-277: The catch-all exception handler in the add-account flow
currently returns str(e) to the client; instead, keep detailed error info in
server logs and return a generic message. In the except Exception as e block,
continue calling logger.error(...) (include the exception and stack trace), call
EngineContext.reset(), and change the JSONResponse content to a non-sensitive
message such as {"success": False, "message": "An internal error occurred while
adding the account"} while preserving status_code=500; do not include str(e) in
the response body.
In `@src/Engine/lib/core/SeparateInstance.py`:
- Around line 134-144: The __click_channel method constructs an XPath by
directly interpolating the channel name which breaks for names containing single
quotes (e.g., "John's Channel"); modify __click_channel to escape quotes in the
name before building xpath or construct a safe XPath using XPath concat()
semantics so names with single or double quotes don't produce malformed
expressions, then pass the safe xpath into find_and_click_element (referenced
symbol: __click_channel and find_and_click_element) and ensure all branches
(name is not None and else) still behave the same.
In `@src/Engine/lib/core/StreamStorm.py`:
- Around line 61-68: The method load_cookies currently declares cookies as list
but get_cookies() returns Optional[list], and it may leave self.cookies as None
when login_method != "cookies"; change the local variable to use Optional[list]
(e.g., cookies: Optional[list] = get_cookies()) or omit the explicit type and
then ensure you assign a non-None value to self.cookies by setting self.cookies
= cookies or [] so self.cookies is always a list; reference the load_cookies
method, get_cookies() call, self.context.login_method check, and self.cookies
assignment when making this change to keep behavior consistent with the
Playwright parent.
- Around line 61-68: The tests fail because StreamStorm.load_cookies() calls
get_cookies() which returns None in CI while StormContext.login_method defaults
to "cookies"; update the ss_instance fixture in tests to either mock
get_cookies() to return a valid cookie list or set the StormContext.login_method
to a non-"cookies" value before creating StreamStorm(data) so load_cookies()
does not raise SystemError; locate the ss_instance fixture in conftest.py and
modify it to patch get_cookies() (or set context.login_method) so that
StreamStorm.load_cookies() receives valid cookies or skips the cookies check.
In `@src/Engine/lib/core/UndetectedDrivers.py`:
- Around line 210-216: In youtube_login, the add_account flow can be clobbered
by the login-loop redirect that forces self.driver.get(self.youtube_login_url)
when current_url doesn't start with the v3 signin path; update the loop so that
the redirect-to-regular-login check is skipped when add_account is True (i.e.,
if add_account: do not replace the current page with self.youtube_login_url),
preserving the initial navigation to self.add_account_url and only reverting to
self.youtube_login_url when add_account is False or after the add-account flow
has definitively failed; modify the logic around the
current_url.startswith("https://accounts.google.com/v3/signin") check inside
youtube_login accordingly.
---
Outside diff comments:
In `@src/Engine/lib/core/UndetectedDrivers.py`:
- Around line 27-38: The class defines __slots__ but forgets to include
'add_account_url'; update the __slots__ tuple in the class (where __slots__ is
declared) to include 'add_account_url' so it matches attributes set in __init__
(youtube_url, youtube_login_url, add_account_url), preserving the intent of
using __slots__ for memory optimization and avoiding accidental __dict__ usage.
---
Nitpick comments:
In `@src/Engine/lib/api/routers/ProfileRouter.py`:
- Around line 253-266: In the except ValueError block handling
profiles.add_account, replace the logger.error call with logger.exception so the
traceback is captured (keep the same formatted message), then proceed to call
EngineContext.reset() and return the JSONResponse as before; specifically update
the except handler that references profiles.add_account, logger,
EngineContext.reset, and JSONResponse to use logger.exception for richer
diagnostics.
In `@src/Engine/lib/api/routers/StormRouter.py`:
- Line 417: Extract the inline conditional used in the f-string to a clearly
named variable to improve readability: for the message in StormRouter (the
f-string that currently contains {"Create profiles" if settings.login_method ==
"profiles" else "Login"}), create a variable like action_text = "Create
profiles" if settings.login_method == "profiles" else "Login" and then use
f"Config file not found. {action_text} first."; apply the same refactor in
StreamStorm where the expression "create Profiles" if self.context.login_method
== "profiles" else "Login" is used, assigning it to a local variable (e.g.,
action_text) and replacing the inline conditional in the string with that
variable.
In `@src/Engine/lib/core/Profiles.py`:
- Around line 120-127: The add_account method assigns cookies with the walrus
operator but never uses it, causing an unused-variable lint error; change the
check to call get_cookies() directly (e.g., if get_cookies() is None: raise
ValueError(...)) and remove the unused "cookies" binding so add_account only
verifies existence of cookies before instantiating UndetectedDrivers and calling
UD.initiate_base_profile() and UD.youtube_login(add_account=True).
In `@src/Engine/lib/core/StormContext.py`:
- Line 39: The context's login_method field is currently typed as a generic str
and omitted from the get() snapshot; update the declaration of login_method to
use typing.Literal with the two allowed values (Literal["cookies","profiles"])
and add login_method to the dictionary returned by the get() method so consumers
receive the current value; also add the Literal import at the top of the file
(from typing import Literal or adjust existing imports) and ensure any places
constructing the snapshot (method get) include the login_method key.
In `@src/UI/src/Components/Modals/ManageProfiles/Sections/CookieLogin.jsx`:
- Around line 147-186: Both handleAddAccount and handleCookieLogin repeat the
same fetch/notification/analytics/error flow; extract that shared behavior into
a single helper (e.g., performFetchWithAnalytics or sharedFetchHandler) that
accepts the URL, analytics event names (start/success/failed/error), success
message/handler, and optional error handlers; update handleAddAccount and
handleCookieLogin to call this helper while retaining their own state setters
(setAddAccountLoading/setAddAccountError and
setCookieLoginLoading/setCookieLoginError) before/after the helper call so each
caller only manages its loading/error state and any unique success callback.
Owner
Author
|
ok |
Owner
Author
|
@coderabbitai generate changelogs in markdown format |
|
Tip For best results, initiate chat on the files or code changes.
ChangelogFeaturesMultiple Google Accounts Support (
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #147
Closes #148
Closes #150
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Chores