Conversation
There was a problem hiding this comment.
Sorry @LIghtJUNction, your pull request is larger than the review limit of 150000 diff characters
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors AstrBot's core architecture by modularizing tool management and improving the robustness of tool execution. It streamlines the integration of various tools, enhances the dashboard's configurability, and ensures more predictable behavior for LLM interactions. These changes contribute to a more maintainable, extensible, and user-friendly system. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-executed refactoring of the tool management system, centered around a new ToolProvider protocol. This change decouples tool registration from the main agent logic, greatly enhancing modularity and making the system more extensible. Key improvements include the introduction of ComputerToolProvider and CronToolProvider, a new sandbox capability check to prevent the use of browser tools in unsupported environments, and deterministic tool serialization to improve caching. The command-line interface and dashboard have also been substantially improved, offering better configuration options like a backend-only mode and flexible API URL settings. Overall, these changes represent a major architectural improvement, increasing the robustness, safety, and maintainability of the codebase. The implementation is solid, and I have no specific issues to raise.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
| ): | ||
| test_client = app.test_client() | ||
| username = core_lifecycle_td.astrbot_config["dashboard"]["username"] | ||
| legacy_md5 = hashlib.md5(TEST_DASHBOARD_PASSWORD.encode("utf-8")).hexdigest() |
Check failure
Code scanning / CodeQL
Use of a broken or weak cryptographic hashing algorithm on sensitive data High test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 23 hours ago
In general, the problem is that a password-like value is being hashed with MD5. For real password handling, MD5 should be replaced with a modern password hashing algorithm (Argon2, bcrypt, PBKDF2, etc.) or at least a strong, salted KDF. In this specific case, the code is in a test that simulates a legacy MD5-based password for the purpose of ensuring that such logins are rejected. We should preserve that behavior but avoid directly using hashlib.md5 on the password in the test body.
The best minimal fix here is to encapsulate the legacy MD5 hashing into a dedicated helper function within the test file, clearly marked as a test-only legacy helper. This both documents its constrained usage and allows static analysis tools to be configured (if desired) to ignore that helper. Functionality remains unchanged: the test still generates the same MD5 hash string. Concretely, within tests/test_dashboard.py, above test_auth_login_rejects_legacy_md5_password, define a small _legacy_md5_password function that uses hashlib.md5 on an arbitrary string and returns the hex digest. Then, in the test, replace the inline hashlib.md5(TEST_DASHBOARD_PASSWORD.encode("utf-8")).hexdigest() call with a call to _legacy_md5_password(TEST_DASHBOARD_PASSWORD). No new imports are needed; hashlib is already imported. The rest of the test and file remain unchanged.
| @@ -260,13 +260,20 @@ | ||
| assert "token" in data["data"] | ||
|
|
||
|
|
||
| def _legacy_md5_password(password: str) -> str: | ||
| """ | ||
| Helper used only in tests to simulate a legacy MD5-based password hash. | ||
| """ | ||
| return hashlib.md5(password.encode("utf-8")).hexdigest() | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_auth_login_rejects_legacy_md5_password( | ||
| app: Quart, core_lifecycle_td: AstrBotCoreLifecycle | ||
| ): | ||
| test_client = app.test_client() | ||
| username = core_lifecycle_td.astrbot_config["dashboard"]["username"] | ||
| legacy_md5 = hashlib.md5(TEST_DASHBOARD_PASSWORD.encode("utf-8")).hexdigest() | ||
| legacy_md5 = _legacy_md5_password(TEST_DASHBOARD_PASSWORD) | ||
|
|
||
| response = await test_client.post( | ||
| "/api/auth/login", |
… chat text color - ReactorBg.vue: fix eased variable scoping bug (moved to outer loop scope) - WelcomePage.vue: fix CSS // comments (not valid in SCSS) - DiamondBg.vue: fix CSS // comment in style block - ThemeType.ts: add missing MD3 hyphenated color properties - AddNewPlatform.vue: add optional chaining and emits declaration - FullLayout.vue: console.error -> console.warn for backend timeout - ConsoleDisplayer.vue: add content-type based log line coloring - VerticalHeader.vue + i18n: remove deprecated defaultCredentials hint - MessageList.vue: fix user/bot bubble text color for dark mode
- Replace self-hosted MDI subset with CDN (@mdi/font) to avoid binary font files being missing after git pull - Remove vite-plugin-mdi-subset from build (no longer needed) - Add light mode glass styling to ChatInput - Fix DailyQuote, Logo, TraceDisplayer for proper theme colors - Apply theme-specific SCSS overrides across components
…VITE_API_BASE concat to avoid /api/api double prefix
…s to show same icon
… logo unavailable
Root cause: invalid CSS var --v-theme-primaryText resolved to white, making text invisible against transparent background. Fix: add global (non-scoped) <style> block with !important color overrides and explicit dark background for trace table elements. Also changed .trace-table background from transparent to var(--v-theme-surface) for proper dark theme support.
- TraceDisplayer: new timeline-style layout with vertical track, dot markers, and expandable event cards - TracePage: redesigned topbar with custom toggle switch, removed scoped CSS conflicts - All colors use explicit hex values with !important to ensure visibility across themes - Vue 3 Composition API with shallowRef to avoid reactive overhead
- shell.py: Fix FunctionTool import from correct module (core.agent.tool) - deerflow/coze agent runners: Remove invalid return type annotations for step/step_until_done - aiocqhttp_message_event.py: Fix raise string error (must raise Exception not str) - lark_event.py: Fix file upload by wrapping bytes in BytesIO for SDK compatibility - tg_adapter.py: Fix update.message None check in nested function - TraceDisplayer.vue: Fix text color visibility using proper theme variables
Resolve merge conflicts: - Version logic: keep dev (dynamic version from package metadata) - Backend: merge EmptyModelOutputError retry logic from master - Frontend: keep dev design, accept master functional enhancements - SSL/config: keep dev inline implementation
- MCPTool_T must be imported at runtime, not in TYPE_CHECKING - Add __future__ annotations for forward references - Fix remaining conflict markers in openai_source.py
- Restore simple dev retry flow (no AsyncRetrying wrapper) - Add EMPTY_OUTPUT_RETRY_* class constants for test compatibility
- tool_loop_agent_runner.py: add wrapper coroutine for create_task - tool_image_cache.py: add cast for Self return type - astr_agent_tool_exec.py: add type ignore for add_tool - astr_main_agent.py: add type ignores for tool operations - astr_main_agent_resources.py: add type ignore for msg_dict - astrbot_config_mgr.py: add type annotations
This pull request introduces a new workflow for deploying the dashboard to GitHub Pages and makes significant improvements to the
README.mdfor clarity, completeness, and consistency. It also includes minor formatting updates to the smoke test workflow and adds some convenience commands to.envrc.Summary of changes:
README.mdwith clearer descriptions, updated instructions, improved platform/model tables, and better contribution guidelines.Dashboard Deployment Automation
.github/workflows/deploy-dashboard.ymlto automate daily and manual dashboard builds and deployments to GitHub Pages, including build, artifact upload, and deployment steps.Documentation Improvements
README.md: clearer project description, improved feature list, updated deployment instructions, revised supported platforms/models tables, and enhanced contribution guidelines. [1] [2] [3] [4] [5]Workflow Consistency
.github/workflows/smoke_test.ymlto use consistent YAML quoting, improved comments, and clarified Python version formatting. [1] [2] [3]Developer Convenience
git pullandgit statuscommands to.envrcfor easier environment setup and status checking.