Mask MT5 passwords and fix stale docs#97
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81ab51670c
ℹ️ 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".
- Refactor initialize() tests: combine test_initialize_with_parameters, test_initialize_with_credentials_without_path, and test_initialize_unwraps_secret_password into parametrized test_initialize_with_password_types covering both path variations and password types (str and SecretStr). All cases verify MT5 receives unwrapped plain password strings. - Refactor login() tests: combine test_login_success, test_login_unwraps_secret_password, and test_login_without_timeout into parametrized test_login_with_password_types covering timeout variations and password types (str and SecretStr). All cases verify MT5 receives unwrapped plain password strings. Keep test_login_failure as separate test for failure path. - Parametrize Mt5Config password coercion: combine test_custom_config and test_config_accepts_secretstr_input into parametrized test_config_password_coercion covering both str and SecretStr inputs. Keep test_config_masks_password_in_string_representations and test_config_immutable as separate tests for clarity. - Maintain 100% branch coverage and explicit assertions that MT5 boundary calls receive unwrapped plain password strings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
dceoy
left a comment
dceoy
left a comment
There was a problem hiding this comment.
Reviewed the password masking and documentation updates.
No blocking issues found. Mt5Config.password is normalized to SecretStr, printable/model dump paths are covered, and both Mt5DataClient.initialize_and_login_mt5 and direct Mt5Client.initialize/login boundaries unwrap the real password before calling MetaTrader5. The stale README/AGENTS guidance is also aligned with the current API and structure.
CI is green on the latest head commit.
1 participant
Closes #93
Closes #95
Summary
Mt5Config.passwordtopydantic.SecretStrsorepr(),str(), and model dumps mask MT5 passwordsinitialize_and_login_mt5docstringMt5RuntimeErrorAPItrading.py/ dry-run contributor guidance fromAGENTS.mdandCLAUDE.mdValidation
uv run pytest --no-cov tests/test_dataframe.py -k 'Mt5Config or masks_nested_password or unwraps_secret_password_override or context_manager_uses_config_and_retries or initialize_and_login'.agents/skills/local-qa/scripts/qa.shNotes
CLAUDE.mdis a symlink toAGENTS.md, so the contributor-guide update applies to both.