- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 964
 
统一路径类,旧函数已弃用 #3279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
统一路径类,旧函数已弃用 #3279
Conversation
.env文件保存统一的路径 静态文件一起打包而不是从上上上级目录找... Co-Authored-By: 赵天乐(tyler zhao) <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `astrbot/dashboard/routes/config.py:464-466` </location>
<code_context>
-                    get_astrbot_path(),
-                    "samples",
-                    "stt_health_check.wav",
+                sample_audio_path = str(
+                    importlib.resources.files("astrbot")
+                    / "samples"
+                    / "stt_health_check.wav"
                 )
</code_context>
<issue_to_address>
**issue (bug_risk):** Switching to importlib.resources for file access may break if the resource is not packaged.
Confirm that 'samples/stt_health_check.wav' is packaged and accessible via importlib.resources, or add error handling for missing resources.
</issue_to_address>
### Comment 2
<location> `astrbot/base/paths.py:34-40` </location>
<code_context>
+    @classmethod
+    def getPaths(cls, name: str) -> AstrbotPaths:
+        """返回Paths实例,用于访问模块的各类目录."""
+        normalized_name: NormalizedName = canonicalize_name(name)
+        instance: AstrbotPaths = cls(normalized_name)
+        instance.name = normalized_name
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Using canonicalize_name for directory names may introduce unexpected normalization.
Verify that canonicalize_name does not cause naming conflicts or break compatibility with existing directory naming conventions used by plugins or modules.
```suggestion
    @classmethod
    def getPaths(cls, name: str) -> AstrbotPaths:
        """返回Paths实例,用于访问模块的各类目录."""
        instance: AstrbotPaths = cls(name)
        instance.name = name
        return instance
```
</issue_to_address>
### Comment 3
<location> `astrbot/base/paths.py:45-46` </location>
<code_context>
+    @property
+    def root(self) -> Path:
+        """返回根目录."""
+        return (
+            self.astrbot_root if self.astrbot_root.exists() else Path.cwd() / ".astrbot"
+        )
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Fallback to Path.cwd()/.astrbot may not be expected.
Switching to the working directory if astrbot_root is missing may cause unexpected results. Instead, raise an error or notify the user to ensure clarity.
</issue_to_address>
### Comment 4
<location> `astrbot/base/paths.py:21` </location>
<code_context>
+    from collections.abc import AsyncGenerator, Generator
+
+
+class AstrbotPaths(IAstrbotPaths):
+    """Class to manage and provide paths used by Astrbot Canary."""
+
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the class to use helper functions and unified logic for path properties, directory changes, and environment reloads to reduce boilerplate and nested abstractions.
```markdown
You can significantly cut down boilerplate and nested abstractions by
  1. removing `getPaths` and normalizing the name in `__init__`  
  2. generating all “mkdir + return Path” properties from one helper  
  3. unifying sync/async `chdir` logic  
  4. moving the env-reload logic into a small helper
For example:
```python
from contextlib import contextmanager, asynccontextmanager
from pathlib import Path
from os import chdir, getenv
from dotenv import load_dotenv
from packaging.utils import canonicalize_name
def _path_property(subdir: str):
    @property
    def prop(self) -> Path:
        p = self.astrbot_root / subdir / self.name
        p.mkdir(parents=True, exist_ok=True)
        return p
    return prop
class AstrbotPaths:
    home   = _path_property("home")
    config = _path_property("config")
    data   = _path_property("data")
    log    = _path_property("logs")
    def __init__(self, name: str):
        # normalize once
        self.name = canonicalize_name(name)
        load_dotenv()  # ensure env is loaded
        self.astrbot_root = Path(
            getenv("ASTRBOT_ROOT", Path.home() / ".astrbot")
        ).absolute()
        self.astrbot_root.mkdir(parents=True, exist_ok=True)
    @contextmanager
    def chdir(self, key: str = "home"):
        original = Path.cwd()
        target = getattr(self, key)
        chdir(target)
        try:
            yield target
        finally:
            chdir(original)
    @asynccontextmanager
    async def achdir(self, key: str = "home"):
        # reuse the sync chdir under the hood
        with self.chdir(key) as target:
            yield target
    def reload_env(self) -> None:
        load_dotenv()
        self.astrbot_root = Path(
            getenv("ASTRBOT_ROOT", Path.home() / ".astrbot")
        ).absolute()
```
Key benefits:
- **No more** duplicated `mkdir` blocks: one helper covers all subdirs.  
- **Drop** `getPaths` and duplicate `.name` sets; `__init__` does normalization.  
- **Single** sync `chdir`, async version just wraps it.  
- **Env-reload** is now a focused `reload_env()` method.  
- **All** original behavior remains intact.
```
</issue_to_address>
### Comment 5
<location> `astrbot/base/paths.py:89-91` </location>
<code_context>
    @classmethod
    def is_root(cls, path: Path) -> bool:
        """检查路径是否为 Astrbot 根目录."""
        if not path.exists() or not path.is_dir():
            return False
        # 检查此目录内是是否包含.astrbot标记文件
        if not (path / ".astrbot").exists():
            return False
        return True
</code_context>
<issue_to_address>
**suggestion (code-quality):** We've found these issues:
- Lift code into else after jump in control flow ([`reintroduce-else`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/reintroduce-else/))
- Replace if statement with if expression ([`assign-if-exp`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/assign-if-exp/))
- Simplify boolean if expression ([`boolean-if-exp-identity`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/boolean-if-exp-identity/))
```suggestion
        return bool((path / ".astrbot").exists())
```
</issue_to_address>
### Comment 6
<location> `astrbot/core/pipeline/respond/stage.py:152` </location>
<code_context>
    async def process(
        self,
        event: AstrMessageEvent,
    ) -> None:
        result = event.get_result()
        if result is None:
            return
        if result.result_content_type == ResultContentType.STREAMING_FINISH:
            return
        logger.info(
            f"Prepare to send - {event.get_sender_name()}/{event.get_sender_id()}: {event._outline_chain(result.chain)}",
        )
        if result.result_content_type == ResultContentType.STREAMING_RESULT:
            if result.async_stream is None:
                logger.warning("async_stream 为空,跳过发送。")
                return
            # 流式结果直接交付平台适配器处理
            use_fallback = self.config.get("provider_settings", {}).get(
                "streaming_segmented",
                False,
            )
            logger.info(f"应用流式输出({event.get_platform_id()})")
            await event.send_streaming(result.async_stream, use_fallback)
            return
        if len(result.chain) > 0:
            # 检查路径映射
            if mappings := self.platform_settings.get("path_mapping", []):
                for idx, component in enumerate(result.chain):
                    if isinstance(component, Comp.File) and component.file:
                        # 支持 File 消息段的路径映射。
                        component.file = path_Mapping(mappings, component.file)
                        event.get_result().chain[idx] = component
            # 检查消息链是否为空
            try:
                if await self._is_empty_message_chain(result.chain):
                    logger.info("消息为空,跳过发送阶段")
                    return
            except Exception as e:
                logger.warning(f"空内容检查异常: {e}")
            # 将 Plain 为空的消息段移除
            result.chain = [
                comp
                for comp in result.chain
                if not (
                    isinstance(comp, Comp.Plain)
                    and (not comp.text or not comp.text.strip())
                )
            ]
            # 发送消息链
            # Record 需要强制单独发送
            need_separately = {ComponentType.Record}
            if self.is_seg_reply_required(event):
                header_comps = self._extract_comp(
                    result.chain,
                    {ComponentType.Reply, ComponentType.At},
                    modify_raw_chain=True,
                )
                if not result.chain or len(result.chain) == 0:
                    # may fix #2670
                    logger.warning(
                        f"实际消息链为空, 跳过发送阶段。header_chain: {header_comps}, actual_chain: {result.chain}",
                    )
                    return
                async with session_lock_manager.acquire_lock(event.unified_msg_origin):
                    for comp in result.chain:
                        i = await self._calc_comp_interval(comp)
                        await asyncio.sleep(i)
                        try:
                            if comp.type in need_separately:
                                await event.send(MessageChain([comp]))
                            else:
                                await event.send(MessageChain([*header_comps, comp]))
                                header_comps.clear()
                        except Exception as e:
                            logger.error(
                                f"发送消息链失败: chain = {MessageChain([comp])}, error = {e}",
                                exc_info=True,
                            )
            else:
                if all(
                    comp.type in {ComponentType.Reply, ComponentType.At}
                    for comp in result.chain
                ):
                    # may fix #2670
                    logger.warning(
                        f"消息链全为 Reply 和 At 消息段, 跳过发送阶段。chain: {result.chain}",
                    )
                    return
                sep_comps = self._extract_comp(
                    result.chain,
                    need_separately,
                    modify_raw_chain=True,
                )
                for comp in sep_comps:
                    chain = MessageChain([comp])
                    try:
                        await event.send(chain)
                    except Exception as e:
                        logger.error(
                            f"发送消息链失败: chain = {chain}, error = {e}",
                            exc_info=True,
                        )
                chain = MessageChain(result.chain)
                if result.chain and len(result.chain) > 0:
                    try:
                        await event.send(chain)
                    except Exception as e:
                        logger.error(
                            f"发送消息链失败: chain = {chain}, error = {e}",
                            exc_info=True,
                        )
        if await call_event_hook(event, EventType.OnAfterMessageSentEvent):
            return
        event.clear_result()
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Remove redundant conditional [×2] ([`remove-redundant-if`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-redundant-if/))
- Simplify sequence length comparison [×2] ([`simplify-len-comparison`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/simplify-len-comparison/))
- Low code quality found in RespondStage.process - 12% ([`low-code-quality`](https://docs.sourcery.ai/Reference/Default-Rules/comments/low-code-quality/))
<br/><details><summary>Explanation</summary>
The quality score for this function is below the quality threshold of 25%.
This score is a combination of the method length, cognitive complexity and working memory.
How can you solve this?
It might be worth refactoring this function to make it shorter and more readable.
- Reduce the function length by extracting pieces of functionality out into
  their own functions. This is the most important thing you can do - ideally a
  function should be less than 10 lines.
- Reduce nesting, perhaps by introducing guard clauses to return early.
- Ensure that variables are tightly scoped, so that code using related concepts
  sits together within the function rather than being scattered.</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| 
           我蓄谋已久的路径统一  | 
    
Co-Authored-By: 赵天乐(tyler zhao) <[email protected]>
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Co-authored-by: 赵天乐(tyler zhao) <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the AstrBot project's path management system by introducing a centralized AstrbotPaths class to replace scattered path utility functions. The refactoring moves core startup logic from main.py to astrbot/__main__.py and deprecates old path functions while maintaining backward compatibility.
Key changes:
- Introduces a new 
astrbot.basepackage withAstrbotPathsclass for unified path management usingpathlib.Path - Replaces 
get_astrbot_data_path()and related functions throughout the codebase withAstrbotPaths.astrbot_root - Migrates VERSION string from hardcoded value to dynamic retrieval via 
importlib.metadata.version() 
Reviewed Changes
Copilot reviewed 36 out of 37 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description | 
|---|---|
| pyproject.toml | Added dotenv>=0.9.9 dependency for environment variable loading | 
| astrbot/base/paths.py | New centralized path management class with singleton pattern | 
| astrbot/main.py | Moved application entry point logic from main.py | 
| main.py | Simplified to delegate to astrbot.main.main() | 
| astrbot/core/utils/astrbot_path.py | Deprecated functions with warnings, delegating to AstrbotPaths | 
| Multiple plugin/core files | Updated to use AstrbotPaths instead of old path functions | 
| astrbot/core/config/default.py | VERSION now uses importlib.metadata.version() | 
| astrbot/core/message/components.py | Improved File component's .file property behavior | 
        
          
                astrbot/core/platform/sources/wechatpadpro/wechatpadpro_adapter.py
              
                Outdated
          
            Show resolved
            Hide resolved
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 36 out of 37 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
astrbot/base/paths.py:1
- The documentation message is confusing and contains an awkward example 
AstrbotPaths.getPaths('any!'). The proper usage should just beAstrbotPaths.astrbot_rootas shown in the implementation. Update the message to:\"不要再使用本函数!实际上就是获取data目录!等效于: AstrbotPaths.astrbot_root\" 
from __future__ import annotations
astrbot/base/paths.py:1
- The deprecation warning message is unclear and contains incorrect syntax. 
AstrbotPaths.getPaths('any!').root.envis not valid -.envis not a property of Path. The message should simply state:\"等效于: AstrbotPaths.astrbot_root (环境变量 ASTRBOT_ROOT 默认值: ./data)\" 
from __future__ import annotations
| 
           @copilot 你可以使用ruff format --check帮我格式化吗?创建一个pr帮我格式化一下。  | 
    
| 
           @LIghtJUNction I've opened a new pull request, #3283, to work on those changes. Once the pull request is ready, I'll request review from you.  | 
    
* Initial plan * chore: 使用 ruff format 格式化代码 Co-authored-by: LIghtJUNction <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: LIghtJUNction <[email protected]>
| 
           @copilot 创建一个pr,执行ruff check --fix,然后提交  | 
    
| 
           @LIghtJUNction I've opened a new pull request, #3285, to work on those changes. Once the pull request is ready, I'll request review from you.  | 
    
| 
           @copilot 运行ruff check --fix,并执行uv run main.py检查  | 
    
| 
           @LIghtJUNction I've opened a new pull request, #3286, to work on those changes. Once the pull request is ready, I'll request review from you.  | 
    
* Initial plan * fix: 移除未使用的临时目录变量 (ruff check --fix) Co-authored-by: LIghtJUNction <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: LIghtJUNction <[email protected]>
| 
           @copilot 对路径类进行覆盖率测试,测试各种情况。覆盖率需达到100%  | 
    
| 
           @LIghtJUNction I've opened a new pull request, #3287, to work on those changes. Once the pull request is ready, I'll request review from you.  | 
    
* Initial plan * feat: 为 AstrbotPaths 添加全面测试,覆盖率达到 100% Co-authored-by: LIghtJUNction <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: LIghtJUNction <[email protected]>
Co-authored-by: Copilot <[email protected]>
| 
           @copilot 使用ruff修复格式等错误,使用mypy检查类型错误  | 
    
| 
           @LIghtJUNction I've opened a new pull request, #3289, to work on those changes. Once the pull request is ready, I'll request review from you.  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sus.
这个代码有实际运行吗?
行为一致吗?
| DeprecationWarning, | ||
| stacklevel=2, | ||
| ) | ||
| return str(AstrbotPaths.astrbot_root) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个函数行为和原先一致吗?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.env: ASTRBOT_ROOT = ./data
完全一致的
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
原来的逻辑太乱,一下子从环境变量读,一下子硬编码成cwd
我真的搞不懂了
现在就是唯一的一个来源,环境变量
或者当前目录的.env文件加载环境变量
这样不就不用担心不一致了
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
如果没搞错的话……
原先的行为是返回根目录而不是data吧。
* Chore: Dockerfile (#3266) * fix: Dockerfile python main.py 改为uv run main.py * fix(dockerfile): 减少重复安装 * fix: 修复一些细节问题 * fix(.dockerignore): 需要git文件夹以获取astrbot版本(带git commit hash后缀) * fix(.dockerignore): uv run之前会uv sync * Replace insecure random with secrets module in cryptographic contexts (#3248) * Initial plan * Security fixes: Replace insecure random with secrets module and improve SSL context Co-authored-by: LIghtJUNction <[email protected]> * Address code review feedback: fix POST method and add named constants Co-authored-by: LIghtJUNction <[email protected]> * Improve documentation for random number generation constants Co-authored-by: LIghtJUNction <[email protected]> * Update astrbot/core/utils/io.py Co-authored-by: Copilot <[email protected]> * Update astrbot/core/platform/sources/wecom_ai_bot/WXBizJsonMsgCrypt.py Co-authored-by: Copilot <[email protected]> * Update tests/test_security_fixes.py Co-authored-by: Copilot <[email protected]> * Update astrbot/core/utils/io.py Co-authored-by: Copilot <[email protected]> * Update astrbot/core/utils/io.py Co-authored-by: Copilot <[email protected]> * Fix: Handle path parameter in SSL fallback for download_image_by_url Co-authored-by: LIghtJUNction <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: LIghtJUNction <[email protected]> Co-authored-by: LIghtJUNction <[email protected]> Co-authored-by: Copilot <[email protected]> * chore: nodejs in Dockerfile --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Copilot <[email protected]> Co-authored-by: Soulter <[email protected]>
          
 
 运行过啊,行为一致,并且更加安全 而且cli包的路径获取函数又不一致  | 
    
| 
           @Dt8333 你切到这个分支测试下就知道了  | 
    
           | 
    
          
 这个是正常的,第一次运行需要使用astrbot init初始化才行  | 
    
          
 是这样的,这就是我说的路径乱  | 
    
FunctionTool 的验证器中,试图访问 self.parameters 属性,但这个属性在对象初始化时还不存在
| 
           这是因为data/目录下没有.astrbot文件标记这是根目录  | 
    
        
          
                astrbot/core/platform/sources/wechatpadpro/wechatpadpro_adapter.py
              
                Outdated
          
            Show resolved
            Hide resolved
        
      There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
我个人保留对get_astrbot_data_path和get_astrbot_path的意见。
目前的修改使get_astrbot_path的语义混乱,并且行为和先前版本不一致。建议回滚。
有关AstrbotPaths.astrbot_root,个人建议用一个函数读取这个对象并返回它的深拷贝,而不是让所有人直接访问这个变量。尽量减少别人犯错误的可能性。
          
 目前的修改语意很明确了,只有根目录一个锚点。 而不是之前的的项目根目录锚点,data目录锚点,以及用的很少的root目录锚点。  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
测试运行通过



.env文件保存统一的路径
静态文件一起打包而不是从上上上级目录找...
fixes #XYZ
Motivation / 动机
Modifications / 改动点
Verification Steps / 验证步骤
Screenshots or Test Results / 运行截图或测试结果
Compatibility & Breaking Changes / 兼容性与破坏性变更
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Sourcery 总结
引入统一的
AstrbotPaths类进行路径管理,弃用旧的路径工具函数,采用importlib.resources加载静态资源,并通过.env文件集中化配置。新功能:
AstrbotPaths类,带有IAstrbotPaths抽象基类,用于集中化和结构化的路径处理改进:
get_astrbot_*函数和 CLI 路径检查,转而使用AstrbotPaths并附带弃用警告importlib.resources,以实现更安全的包资源访问AstrBotUpdator,在解压更新时使用新的AstrbotPaths根路径构建:
pyproject.toml中添加dotenv依赖,用于基于环境的配置文档:
.env和.env.example文件,用于管理ASTRBOT_ROOTOriginal summary in English
Summary by Sourcery
Introduce a unified AstrbotPaths class for path management and deprecate legacy path utility functions, adopt importlib.resources for static asset loading, and centralize configuration via a .env file.
New Features:
Enhancements:
Build:
Documentation: