Skip to content

Conversation

@ink-developer
Copy link
Collaborator

@ink-developer ink-developer commented Dec 31, 2025

Описание

Кратко, что делает этот PR. Например, добавляет новый метод, исправляет баг, улучшает документацию.

Новые методы

set_password

Измененные методы

login_with_code

И так же кучу системных методов поменял + фикс сокета частичный

Тип изменений

  • Исправление бага
  • Новая функциональность
  • Улучшение документации
  • Рефакторинг

Связанные задачи / Issue

Ссылка на issue, если есть: #

Тестирование

Покажите пример кода, который проверяет изменения:

import pymax

# пример использования нового функционала

Summary by CodeRabbit

  • New Features

    • Two-factor authentication (2FA) with password challenges, email code verification and recovery hints
    • Account password management (set password, hint, optional recovery email)
    • WebSocket protocol update to support ver=11 messages
  • Bug Fixes

    • Improved socket/WebSocket sequence handling and pending-message behavior
    • Better connection checks and error handling for disconnected sockets and telemetry
  • Other

    • Release bumped to v1.2.5

✏️ Tip: You can customize this high-level summary in your review settings.

… поддержку двухфакторной аутентификации и исправления для работы с сокетами.
…ые функции и исправления. Добавлены пометки для методов аутентификации в enum.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 31, 2025

📝 Walkthrough

Walkthrough

Adds full two-factor authentication flows (password challenges, email codes, recovery hints) and account password management, extends WebSocket/protocol handling (ver=11, seq masking, timeout and connection checks), introduces new payloads and Capability enum, and bumps version to v1.2.5.

Changes

Cohort / File(s) Summary
Release notes
redocs/source/release_notes.rst, redocs/source/release_notes_archive.rst
Version bump to v1.2.5; documents new 2FA flows, password management, WebSocket/socket improvements; archives prior release notes.
Enums & constants
src/pymax/static/enum.py, src/pymax/static/constant.py
Adds Capability enum (2FA capabilities) and many new Opcode members; adds private _Unset and public sentinel UNSET.
Payload models
src/pymax/payloads.py
Introduces 7 new payload classes: CheckPasswordChallengePayload, CreateTrackPayload, SetPasswordPayload, SetHintPayload, SetTwoFactorPayload, RequestEmailCodePayload, SendEmailCodePayload.
Authentication logic
src/pymax/core.py, src/pymax/mixins/auth.py
login_with_code updated to detect passwordChallenge; adds _check_password, _two_factor_auth, _set_password, _set_hint, _set_email, and public set_password workflow for interactive 2FA and account setup.
Socket / WebSocket / telemetry
src/pymax/interfaces.py, src/pymax/mixins/socket.py, src/pymax/mixins/websocket.py, src/pymax/mixins/telemetry.py
Masks sequence numbers to 0–255 for pending keys; cancels prior futures for same 1‑byte seq; adds explicit asyncio.TimeoutError handling in send-and-wait; checks connection before sending notifications; handles SocketNotConnectedError; adds ver=11 in WS messages.
Project metadata
pyproject.toml
Project version bumped from 1.2.4 to 1.2.5.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Client as MaxClient
    participant Server as AuthServer

    rect rgb(220,235,255)
    Note over User,Server: Login request returns passwordChallenge
    User->>Client: login_with_code(code)
    Client->>Server: send login request
    Server-->>Client: { ..., passwordChallenge: {...}, tokenAttrs: {...} }
    end

    rect rgb(235,220,255)
    Note over Client,User: Two‑factor interactive flow
    Client->>Server: CreateTrackPayload(type=0)
    Server-->>Client: { track_id }
    Client->>User: prompt password
    User-->>Client: password
    Client->>Server: CheckPasswordChallengePayload(track_id, password)
    Server-->>Client: tokenAttrs or error
    alt tokenAttrs present
        Client->>User: optional hint/email prompts
        User-->>Client: hint / email / verify_code
        Client->>Server: SetHintPayload / RequestEmailCodePayload / SendEmailCodePayload
        Client->>Server: SetTwoFactorPayload(expected_capabilities, track_id, password, hint?)
        Server-->>Client: 2FA setup success
    else error
        Client-->>User: invalid password / retry or fail
    end
    end

    rect rgb(220,255,230)
    Note over Client: Finalize login
    Client->>Client: extract LOGIN token from tokenAttrs or proceed with token
    Client-->>User: login successful
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • 1.1.21 #27: Overlaps on authentication flow and AuthMixin changes (verification, 2FA flows, payloads).
  • Dev/1.2.1 #30: Shares modifications to authentication and WebSocket/socket handling and related opcode/enum updates.

Poem

🐰 A track and a token I found in the night,
Passwords and hints to keep things tight.
Bytes now confined to a single small frame,
WebSocket whispers and a sturdier name.
thumps nose ✨ Two‑factor hops into sight!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 2 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.70% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Dev/1.2.5' is generic and vague, using a branch/version name rather than describing the main changes; it does not convey meaningful information about what was modified. Replace with a descriptive title summarizing the primary change, such as 'Add two-factor authentication support with set_password method and login improvements' or 'Implement 2FA authentication and password management features'.
Description check ❓ Inconclusive The description is partially filled with checkboxes and lists of changed methods but lacks substantive explanations; the testing section contains only a placeholder comment without actual test code, leaving key sections incomplete. Expand the 'Описание' section with clear, detailed explanations of the 2FA implementation and socket fixes; provide concrete test examples demonstrating set_password usage and login flow validation in the testing section.
✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (8)
src/pymax/mixins/telemetry.py (1)

106-113: Exception handling looks good; consider adding exc_info for debugging.

The SocketNotConnectedError handling correctly exits the telemetry loop on disconnect. The generic Exception catch is appropriate here to prevent telemetry failures from crashing the client.

Consider adding exc_info=True to the warning on line 112 for easier debugging:

             except Exception:
-                    self.logger.warning("Failed to send random navigation")
+                    self.logger.warning("Failed to send random navigation", exc_info=True)
src/pymax/static/constant.py (1)

92-96: Sentinel pattern is well-implemented.

Using a dedicated _Unset class is the correct approach for distinguishing between "not provided" and None. Consider adding __repr__ for debugging clarity:

class _Unset:
    def __repr__(self) -> str:
        return "UNSET"
src/pymax/mixins/websocket.py (1)

137-144: Fix exception chaining per static analysis hints.

Use raise ... from None to suppress exception chaining noise, and consider using logger.warning instead of logger.exception for TimeoutError (no useful traceback).

🔎 Proposed fix
         except asyncio.TimeoutError:
-            self.logger.exception("Send and wait failed (opcode=%s, seq=%s)", opcode, msg["seq"])
-            raise RuntimeError("Send and wait failed")
+            self.logger.warning("Send and wait timed out (opcode=%s, seq=%s)", opcode, msg["seq"])
+            raise RuntimeError("Send and wait failed") from None
         except Exception:
             self.logger.exception("Send and wait failed (opcode=%s, seq=%s)", opcode, msg["seq"])
-            raise RuntimeError("Send and wait failed")
+            raise RuntimeError("Send and wait failed") from None
         finally:
             self._pending.pop(seq_key, None)
src/pymax/static/enum.py (1)

226-231: Replace Cyrillic characters with ASCII in the comment.

The comment on line 227 contains Cyrillic characters that could cause encoding issues or confusion. Static analysis flagged ambiguous characters (В and а). Consider rewriting the comment in English or ensuring it uses only ASCII-compatible Cyrillic if Russian is intentional.

Suggested fix
 class Capability(int, Enum):
-    DEFAULT = 0  # В душе не чаю что это такое но при первой установке 2фа там 0 3 4 так что пусть будет дефолт
+    DEFAULT = 0  # Unknown purpose; observed values 0, 3, 4 during initial 2FA setup
     ESIA_VERIFIED_FLAG = 1
     SECOND_FACTOR_PASSWORD_ENABLED = 2
     SECOND_FACTOR_HAS_EMAIL = 3
     SECOND_FACTOR_HAS_HINT = 4
src/pymax/mixins/auth.py (4)

389-400: Consider propagating errors rather than silently returning None.

_check_password returns None on error without logging or re-raising. This makes it harder to distinguish between "wrong password" and other server errors. Consider logging the specific error for debugging purposes.

Suggested improvement
     async def _check_password(self, password: str, track_id: str) -> dict[str, Any] | None:
         payload = CheckPasswordChallengePayload(
             track_id=track_id,
             password=password,
         ).model_dump(by_alias=True)

         data = await self._send_and_wait(opcode=Opcode.AUTH_LOGIN_CHECK_PASSWORD, payload=payload)

         token_attrs = data.get("payload", {}).get("tokenAttrs", {})
         if data.get("payload", {}).get("error"):
+            self.logger.debug("Password check failed: %s", data.get("payload", {}).get("error"))
             return None
         return token_attrs

433-441: Variable shadowing: payload is reassigned from request to response.

The variable payload is used for both the request payload (line 434) and reassigned to the response payload (line 440). This reduces readability and can lead to confusion.

Suggested fix
     async def _set_password(self, password: str, track_id: str) -> bool:
         payload = SetPasswordPayload(
             track_id=track_id,
             password=password,
         ).model_dump(by_alias=True)

         data = await self._send_and_wait(opcode=Opcode.AUTH_VALIDATE_PASSWORD, payload=payload)
-        payload = data.get("payload", {})
-        return not payload
+        response_payload = data.get("payload", {})
+        return not response_payload

443-451: Same variable shadowing issue as _set_password.

Suggested fix
     async def _set_hint(self, hint: str, track_id: str) -> bool:
         payload = SetHintPayload(
             track_id=track_id,
             hint=hint,
         ).model_dump(by_alias=True)

         data = await self._send_and_wait(opcode=Opcode.AUTH_VALIDATE_HINT, payload=payload)
-        payload = data.get("payload", {})
-        return not payload
+        response_payload = data.get("payload", {})
+        return not response_payload

575-593: Variable shadowing: payload reassigned from request to response.

Same issue as in other methods. Also, the expected_capabilities list hardcodes SECOND_FACTOR_HAS_HINT and SECOND_FACTOR_HAS_EMAIL even when hint/email might not have been set successfully.

Suggested fix
+        capabilities = [Capability.DEFAULT]
+        if hint and isinstance(hint, str):
+            capabilities.append(Capability.SECOND_FACTOR_HAS_HINT)
+        if email:
+            capabilities.append(Capability.SECOND_FACTOR_HAS_EMAIL)
+
         payload = SetTwoFactorPayload(
-            expected_capabilities=[
-                Capability.DEFAULT,
-                Capability.SECOND_FACTOR_HAS_HINT,
-                Capability.SECOND_FACTOR_HAS_EMAIL,
-            ],
+            expected_capabilities=capabilities,
             track_id=track_id,
             password=password,
             hint=hint if isinstance(hint, (str, type(None))) else None,
         )

         data = await self._send_and_wait(
             opcode=Opcode.AUTH_SET_2FA,
             payload=payload.model_dump(by_alias=True),
         )
-        payload = data.get("payload", {})
-        if payload and payload.get("error"):
+        response_payload = data.get("payload", {})
+        if response_payload and response_payload.get("error"):
             MixinsUtils.handle_error(data)

         return True
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a6e69af and d932b56.

📒 Files selected for processing (11)
  • redocs/source/release_notes.rst
  • redocs/source/release_notes_archive.rst
  • src/pymax/core.py
  • src/pymax/interfaces.py
  • src/pymax/mixins/auth.py
  • src/pymax/mixins/socket.py
  • src/pymax/mixins/telemetry.py
  • src/pymax/mixins/websocket.py
  • src/pymax/payloads.py
  • src/pymax/static/constant.py
  • src/pymax/static/enum.py
🧰 Additional context used
🧬 Code graph analysis (6)
src/pymax/mixins/telemetry.py (1)
src/pymax/exceptions.py (2)
  • Error (60-86)
  • SocketNotConnectedError (23-30)
src/pymax/interfaces.py (1)
src/pymax/exceptions.py (2)
  • SocketNotConnectedError (23-30)
  • WebSocketNotConnectedError (13-20)
src/pymax/mixins/auth.py (4)
src/pymax/static/enum.py (2)
  • Capability (226-231)
  • Opcode (4-146)
src/pymax/payloads.py (7)
  • CheckPasswordChallengePayload (370-372)
  • CreateTrackPayload (375-376)
  • RequestEmailCodePayload (396-398)
  • SendEmailCodePayload (401-403)
  • SetHintPayload (384-386)
  • SetPasswordPayload (379-381)
  • SetTwoFactorPayload (389-393)
src/pymax/protocols.py (2)
  • ClientProtocol (32-123)
  • _send_and_wait (95-102)
src/pymax/static/constant.py (1)
  • _Unset (92-93)
src/pymax/mixins/socket.py (2)
src/pymax/interfaces.py (1)
  • _handle_pending (259-266)
src/pymax/exceptions.py (1)
  • SocketSendError (33-39)
src/pymax/core.py (1)
src/pymax/mixins/auth.py (1)
  • _two_factor_auth (402-431)
src/pymax/payloads.py (1)
src/pymax/static/enum.py (1)
  • Capability (226-231)
🪛 Ruff (0.14.10)
src/pymax/mixins/telemetry.py

111-111: Do not catch blind exception: Exception

(BLE001)

src/pymax/interfaces.py

213-213: Do not catch blind exception: Exception

(BLE001)

src/pymax/mixins/auth.py

408-408: Avoid specifying long messages outside the exception class

(TRY003)


430-430: Avoid specifying long messages outside the exception class

(TRY003)


527-527: Avoid specifying long messages outside the exception class

(TRY003)

src/pymax/mixins/websocket.py

139-139: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


139-139: Avoid specifying long messages outside the exception class

(TRY003)


142-142: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling

(B904)


142-142: Avoid specifying long messages outside the exception class

(TRY003)

src/pymax/static/enum.py

227-227: Comment contains ambiguous В (CYRILLIC CAPITAL LETTER VE). Did you mean B (LATIN CAPITAL LETTER B)?

(RUF003)


227-227: Comment contains ambiguous а (CYRILLIC SMALL LETTER A). Did you mean a (LATIN SMALL LETTER A)?

(RUF003)

🔇 Additional comments (15)
src/pymax/interfaces.py (3)

191-191: Protocol version bump looks correct.

The ver=11 aligns with the WebSocket protocol improvements mentioned in the release notes.


210-214: Socket disconnect handling in ping loop is correct.

Breaking out of the loop on SocketNotConnectedError and logging at debug level is appropriate. Removing exc_info from the generic exception warning is fine since the message is self-explanatory.


289-296: Potential logic inversion — verify notification behavior.

The early return skips sending NOTIF_MESSAGE when the socket is connected. This appears inverted: typically you'd want to send read notifications when connected, not skip them. If intentional (e.g., socket clients handle notifications differently), please add a clarifying comment.

 async def _send_notification_response(self, chat_id: int, message_id: str) -> None:
-    if self._socket is not None and self.is_connected:
-        return
+    # Socket clients handle notifications via a different path
+    if self._socket is not None and self.is_connected:
+        return
     await self._send_and_wait(
src/pymax/core.py (1)

137-137: Sequence initialization at 255 is correct for 1-byte wrapping.

With seq % 256 used in packing, starting at 255 means the first message increments to 256, which wraps to 0. This aligns with the 8-bit sequence handling in socket.py.

src/pymax/mixins/websocket.py (1)

114-120: Good improvement: seq_key and pending future cancellation.

Using seq_key for consistent cleanup and canceling stale pending futures prevents memory leaks when sequence numbers wrap around.

src/pymax/mixins/socket.py (4)

87-87: Correct 1-byte sequence packing.

Masking seq % 256 ensures the sequence fits in the 1-byte wire format.


220-223: Sequence normalization in recv loop aligns with send path.

Using seq % 256 (with None guard) ensures pending lookups match the keys used when registering futures.


252-258: Future cancellation prevents stale pending entries.

Canceling an existing future before registering a new one for the same seq_key is essential for correctness when sequences wrap.


291-299: Timeout and cleanup handling looks correct.

Using SocketSendError with proper exception chaining (from None / from exc) and cleaning up with msg["seq"] % 256 maintains consistency.

redocs/source/release_notes.rst (1)

1-61: Release notes accurately document the changes.

The documentation covers all key changes: 2FA support, set_password method, sequence number fixes, and protocol version bump. The new payload classes and Capability enum are properly listed.

src/pymax/payloads.py (1)

370-403: New 2FA payload classes are well-structured.

The payload models follow the existing CamelModel pattern consistently. SetTwoFactorPayload correctly uses list[Capability] for type safety.

src/pymax/static/enum.py (1)

5-146: LGTM on Opcode additions.

The new opcode values and inline checkmark annotations are consistent with the existing pattern. The new opcodes for 2FA (AUTH_VALIDATE_PASSWORD, AUTH_LOGIN_CHECK_PASSWORD, etc.) align with the authentication features being added in this PR.

src/pymax/mixins/auth.py (2)

10-26: LGTM on imports.

The new imports align with the 2FA functionality being added and are properly organized.


209-223: LGTM on login flow enhancement.

The conditional handling for password challenges vs direct token extraction is clear and properly structured for 2FA support.

redocs/source/release_notes_archive.rst (1)

1-141: Documentation structure is correct—v1.2.5 release notes already exist.

The archive file properly contains only previous versions (v1.2.3 and v1.2.4). The v1.2.5 release notes are already documented in redocs/source/release_notes.rst with comprehensive coverage of the new 2FA functionality, including set_password(), Capability enum, and all related payload classes. No additional documentation is needed.

Comment on lines +402 to +431
async def _two_factor_auth(self, password_challenge: dict[str, Any]) -> None:
self.logger.info("Starting two-factor authentication flow")

track_id = password_challenge.get("trackId")
if not track_id:
self.logger.critical("Password challenge missing track ID")
raise ValueError("Password challenge missing track ID")

hint = password_challenge.get("hint", "No hint provided")

while True:
password = await asyncio.to_thread(
lambda: input(f"Введите пароль (Подсказка: {hint}): ").strip()
)
if not password:
self.logger.warning("Password is empty, please try again")
continue

token_attrs = await self._check_password(password, track_id)
if not token_attrs:
self.logger.error("Incorrect password, please try again")
continue

login_attrs = token_attrs.get("LOGIN", {})
if login_attrs:
token = login_attrs.get("token")
if not token:
self.logger.critical("Login response did not contain tokenAttrs.LOGIN.token")
raise ValueError("Login response did not contain tokenAttrs.LOGIN.token")
return token
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Return type annotation mismatch: method returns str but is annotated as None.

The method is annotated to return None but actually returns a token string on line 431. This will cause type checking issues.

Proposed fix
-    async def _two_factor_auth(self, password_challenge: dict[str, Any]) -> None:
+    async def _two_factor_auth(self, password_challenge: dict[str, Any]) -> str:
         self.logger.info("Starting two-factor authentication flow")
         # ... rest of method
🧰 Tools
🪛 Ruff (0.14.10)

408-408: Avoid specifying long messages outside the exception class

(TRY003)


430-430: Avoid specifying long messages outside the exception class

(TRY003)

🤖 Prompt for AI Agents
In src/pymax/mixins/auth.py around lines 402 to 431, the _two_factor_auth method
is annotated to return None but actually returns a token string; change the
return type annotation to str (or Optional[str] if you prefer a nullable
contract) on the method signature, update any necessary typing imports, and
ensure any callers/types are compatible with the new return type.

opcode=Opcode.AUTH_CREATE_TRACK,
payload=payload,
)
print(data)
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove debug print statement.

This print(data) statement appears to be debug code that should be removed before merging.

Proposed fix
-        print(data)
         if data.get("payload", {}).get("error"):
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
print(data)
if data.get("payload", {}).get("error"):
🤖 Prompt for AI Agents
In src/pymax/mixins/auth.py around line 520, remove the leftover debug statement
`print(data)` so no debug output is emitted; simply delete that line (or replace
it with appropriate logging at debug level if needed) and ensure surrounding
code and imports remain unchanged.

Comment on lines +529 to +541
while True:
if not password:
password = await asyncio.to_thread(lambda: input("Введите пароль: ").strip())
if not password:
self.logger.warning("Password is empty, please try again")
continue

success = await self._set_password(password, track_id)
if success:
self.logger.info("Password set successfully")
break
else:
self.logger.error("Failed to set password, please try again")
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Potential infinite loop if password validation repeatedly fails.

If _set_password keeps returning False (e.g., due to server-side validation rules the user can't satisfy), the loop never exits. Consider adding a retry limit or allowing the user to abort.

Suggested fix with retry limit
+        max_retries = 5
+        attempts = 0
         while True:
+            if attempts >= max_retries:
+                self.logger.error("Maximum password set attempts exceeded")
+                raise ValueError("Failed to set password after maximum retries")
             if not password:
                 password = await asyncio.to_thread(lambda: input("Введите пароль: ").strip())
                 if not password:
                     self.logger.warning("Password is empty, please try again")
                     continue

             success = await self._set_password(password, track_id)
             if success:
                 self.logger.info("Password set successfully")
                 break
             else:
+                attempts += 1
+                password = ""  # Reset to prompt again
                 self.logger.error("Failed to set password, please try again")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
while True:
if not password:
password = await asyncio.to_thread(lambda: input("Введите пароль: ").strip())
if not password:
self.logger.warning("Password is empty, please try again")
continue
success = await self._set_password(password, track_id)
if success:
self.logger.info("Password set successfully")
break
else:
self.logger.error("Failed to set password, please try again")
max_retries = 5
attempts = 0
while True:
if attempts >= max_retries:
self.logger.error("Maximum password set attempts exceeded")
raise ValueError("Failed to set password after maximum retries")
if not password:
password = await asyncio.to_thread(lambda: input("Введите пароль: ").strip())
if not password:
self.logger.warning("Password is empty, please try again")
continue
success = await self._set_password(password, track_id)
if success:
self.logger.info("Password set successfully")
break
else:
attempts += 1
password = "" # Reset to prompt again
self.logger.error("Failed to set password, please try again")
🤖 Prompt for AI Agents
In src/pymax/mixins/auth.py around lines 529 to 541, the password prompt loop
can become infinite if _set_password keeps returning False; add a retry limit
and allow user abort: introduce a max_retries counter (e.g., 3-5), increment it
on each failed attempt from _set_password, and after exceeding the limit log an
error and break/raise so the flow can continue or surface the failure;
additionally, when prompting for the password accept a special abort token
(e.g., empty input interpreted after first prompt or a 'q' input) to let the
user cancel immediately and handle that case by logging/wrapping up instead of
retrying indefinitely.

Comment on lines +561 to +573
while True:
if not email:
email = await asyncio.to_thread(
lambda: input("Введите email для восстановления пароля: ").strip()
)
if not email:
self.logger.warning("Email is empty, please try again")
continue

success = await self._set_email(email, track_id)
if success:
self.logger.info("Recovery email set successfully")
break
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Potential infinite loop if email validation repeatedly fails.

Similar to the password loop, if _set_email keeps failing, there's no exit condition. Additionally, if the user provides an empty email and email parameter was not provided, the loop continues indefinitely asking for email with no way to skip.

Suggested fix allowing user to skip email
         while True:
             if not email:
                 email = await asyncio.to_thread(
-                    lambda: input("Введите email для восстановления пароля: ").strip()
+                    lambda: input("Введите email для восстановления пароля (пустой - пропустить): ").strip()
                 )
                 if not email:
-                    self.logger.warning("Email is empty, please try again")
-                    continue
+                    self.logger.info("Email recovery skipped by user")
+                    break

             success = await self._set_email(email, track_id)
             if success:
                 self.logger.info("Recovery email set successfully")
                 break
+            else:
+                email = ""  # Reset to prompt again
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
while True:
if not email:
email = await asyncio.to_thread(
lambda: input("Введите email для восстановления пароля: ").strip()
)
if not email:
self.logger.warning("Email is empty, please try again")
continue
success = await self._set_email(email, track_id)
if success:
self.logger.info("Recovery email set successfully")
break
while True:
if not email:
email = await asyncio.to_thread(
lambda: input("Введите email для восстановления пароля (пустой - пропустить): ").strip()
)
if not email:
self.logger.info("Email recovery skipped by user")
break
success = await self._set_email(email, track_id)
if success:
self.logger.info("Recovery email set successfully")
break
else:
email = "" # Reset to prompt again
🤖 Prompt for AI Agents
In src/pymax/mixins/auth.py around lines 561 to 573, the while loop that prompts
for an email can spin indefinitely if _set_email repeatedly fails or the user
keeps submitting empty input; update the loop to enforce a bounded number of
attempts (e.g., max_retries), accept an explicit skip option (e.g., empty input
or 'skip') to exit gracefully when no email is desired, and on repeated
_set_email failures return or raise a clear error after max_retries instead of
looping forever; also log the reason when aborting so callers can act
accordingly.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/pymax/static/enum.py (1)

226-231: Consider revising the comment to use only Latin characters.

The new Capability enum is well-structured for 2FA features. However, the comment on line 227 contains ambiguous Cyrillic characters (В, а) that visually resemble Latin characters (B, a), which can cause confusion.

🔎 Proposed fix
-    DEFAULT = 0  # В душе не чаю что это такое но при первой установке 2фа там 0 3 4 так что пусть будет дефолт
+    DEFAULT = 0  # Default capability value (observed as 0, 3, 4 during initial 2FA setup)
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d932b56 and aa736c9.

📒 Files selected for processing (3)
  • pyproject.toml
  • src/pymax/core.py
  • src/pymax/static/enum.py
✅ Files skipped from review due to trivial changes (1)
  • pyproject.toml
🧰 Additional context used
🧬 Code graph analysis (1)
src/pymax/core.py (1)
src/pymax/mixins/auth.py (1)
  • _two_factor_auth (402-431)
🪛 Ruff (0.14.10)
src/pymax/static/enum.py

227-227: Comment contains ambiguous В (CYRILLIC CAPITAL LETTER VE). Did you mean B (LATIN CAPITAL LETTER B)?

(RUF003)


227-227: Comment contains ambiguous а (CYRILLIC SMALL LETTER A). Did you mean a (LATIN SMALL LETTER A)?

(RUF003)

🔇 Additional comments (1)
src/pymax/core.py (1)

255-262: LGTM! The token extraction bug has been fixed.

The previous issue where line 262 called .get("LOGIN", {}) again on login_attrs (which was already the LOGIN dict) has been correctly resolved. The code now properly extracts the token directly from login_attrs.get("token").

The new 2FA flow logic is sound:

  • If a passwordChallenge is present and login_attrs is empty, the code triggers two-factor authentication.
  • Otherwise, it retrieves the token directly from login_attrs.

@ink-developer ink-developer merged commit b325c95 into main Dec 31, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants