Skip to content

fix(desktop): stabilize tray restart signaling and harden build/resource flow#24

Merged
zouyonghe merged 25 commits into
AstrBotDevs:mainfrom
zouyonghe:pr/desktop-runtime-ci-20260220
Feb 20, 2026
Merged

fix(desktop): stabilize tray restart signaling and harden build/resource flow#24
zouyonghe merged 25 commits into
AstrBotDevs:mainfrom
zouyonghe:pr/desktop-runtime-ci-20260220

Conversation

@zouyonghe
Copy link
Copy Markdown
Member

@zouyonghe zouyonghe commented Feb 20, 2026

Summary

This PR stabilizes tray-initiated backend restart UX in desktop builds, removes brittle legacy bridge patching, and tightens build/CI environment handling.

What Changed

  • Tray restart signaling (Tauri + WebUI bridge)

    • Emit a dedicated desktop event from Rust: astrbot://tray-restart-backend.
    • Use tokenized payloads to de-duplicate repeated tray restart signals.
    • Rework desktop bridge bootstrap to subscribe via:
      1. tauriEvent.listen when available
      2. plugin:event|listen fallback path when needed
    • Keep pending callback delivery semantics so UI handlers still receive restart signals after registration.
  • Desktop capability fix for loopback WebUI origin

    • Add src-tauri/capabilities/default.json to grant core:default for main window on:
      • http://127.0.0.1:*
      • http://localhost:*
    • Fixes ACL-denied event-listen behavior on loopback-loaded dashboard pages.
  • Resource preparation hardening

    • Replace legacy string-patching flow in prepare-resources.mjs with explicit desktop-bridge artifact verification.
    • Add strict-mode support via ASTRBOT_DESKTOP_STRICT_BRIDGE_EXPECTATIONS.
    • Improve warning/error hints and optional-file messaging for easier diagnostics.
  • Build and env workflow cleanup

    • make build now exports shared env consistently and only overrides ASTRBOT_SOURCE_DIR when build source is explicitly resolved.
    • make clean-env now generates/refreshes reset script based on env-key hash to avoid stale reset behavior.
    • Ignore generated reset helper script in git.
  • Docs

    • Simplify README build/version workflow and environment variable guidance.

Root Cause Fixed

The missing tray restart waiting prompt was primarily caused by event-listen ACL denial on loopback dashboard origin (plugin:event|listen not allowed by ACL).

This PR fixes that by combining:

  • capability grant for loopback origin, and
  • reliable tray restart event wiring in desktop bridge bootstrap.

Validation

  • CI:
    • actionlint
    • node + python syntax checks
    • rustfmt + clippy + check
  • Local:
    • make clean
    • make update
    • make build
    • Manual tray restart verification in macOS packaged app

@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @zouyonghe, 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 improves the desktop application's stability, user experience, and development workflow. It focuses on providing clearer feedback during backend restarts, modernizing the integration with the dashboard by removing legacy Electron-specific code, and streamlining the build process for better environment management.

Highlights

  • Desktop Tray Restart UX: Enhanced the desktop tray restart user experience by implementing a new Tauri event emission mechanism. This allows the WebUI to reliably display waiting and restart feedback when the backend is restarted from the tray.
  • Resource Patching and Legacy Bridge Removal: Refactored desktop resource patching logic to improve compatibility with the upstream dashboard and eliminate outdated Electron-specific runtime assumptions. This involved updating regex patterns and patching functions to use more generic 'isDesktop' flags.
  • Build and Environment Management: Improved the build and cleanup workflows by introducing a 'make clean-env' target to generate a script for unsetting build environment variables. The 'make build' target's documentation and logic were also updated to clarify and support explicit build source directory specifications.
Changelog
  • Makefile
    • Added new variables 'ASTRBOT_BUILD_SOURCE_DIR', 'ASTRBOT_RESET_ENV_SCRIPT', and 'ASTRBOT_ENV_KEYS' to manage build source and environment cleanup.
    • Introduced a 'clean-env' target to generate a shell script for unsetting build-related environment variables.
    • Updated the 'build' target to support specifying a build source directory via 'ASTRBOT_BUILD_SOURCE_DIR' or 'ASTRBOT_SOURCE_DIR' and improved version handling.
    • Expanded the 'help' output to include descriptions for the new 'clean-env' target and clarified 'build' instructions.
  • README.md
    • Updated the documentation for 'make build' to explain the new options for specifying local source directories.
    • Clarified the priority and usage of 'ASTRBOT_SOURCE_DIR' and 'ASTRBOT_BUILD_SOURCE_DIR' for local source builds.
  • scripts/prepare-resources.mjs
    • Modified regex patterns in 'LEGACY_DESKTOP_BRIDGE_PATTERNS' to include new patterns for tray restart handling and more specific desktop release flags.
    • Updated the 'transform' function for 'App.vue' to replace legacy tray restart guards and remove 'restartAstrBotImport'.
    • Modified the 'transform' function for 'electron-bridge.d.ts' to replace 'isElectron' types with 'isDesktop' types.
    • Adjusted the 'transform' function for 'VerticalHeader.vue' to replace Electron-specific flags and runtime checks with generic desktop equivalents.
  • src-tauri/src/main.rs
    • Added a call to 'emit_tray_restart_backend_event' within the 'TRAY_MENU_RESTART_BACKEND' handler to notify the frontend of a restart.
    • Implemented a new function 'emit_tray_restart_backend_event' to send a JavaScript event to the main webview window, facilitating frontend feedback during backend restarts.
Activity
  • The pull request introduces a new event emission mechanism from the Tauri backend to the WebUI for tray restart events.
  • Significant refactoring of desktop bridge patching logic was performed to remove Electron-specific assumptions and align with a more generic desktop environment.
  • Build system enhancements were made to improve local development and CI/CD processes, including new Makefile targets and updated documentation.
  • The changes span across build scripts, frontend patching logic, and backend Rust code, indicating a comprehensive update to the desktop application's core functionalities.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - 我发现了两个问题,并给出了一些高层面的反馈:

  • build 目标中,即使 build_source_dir 为空,ASTRBOT_SOURCE_DIR 也总是会被设置,这会把环境中已有的值覆盖成空字符串;建议只在 build_source_dir 非空时才导出 ASTRBOT_SOURCE_DIR,以避免让调用方产生意外行为。
  • patchLegacyDesktopBridgeArtifacts 里链式的正则替换已经相当复杂(尤其是托盘重启处理和 legacy bridge 标志相关的逻辑);可以考虑把这些转换封装起来,或者增加一些简单的结构性检查(例如验证导入和调用点是否同时被更新),以减少在稍有差异的上游源码上出现“只打了一半补丁”的风险。
给 AI Agent 的提示
请根据本次代码评审中的评论进行修改:

## 整体意见
-`build` 目标中,即使 `build_source_dir` 为空,`ASTRBOT_SOURCE_DIR` 也总是会被设置,这会把环境中已有的值覆盖成空字符串;建议只在 `build_source_dir` 非空时才导出 `ASTRBOT_SOURCE_DIR`,以避免让调用方产生意外行为。
- `patchLegacyDesktopBridgeArtifacts` 里链式的正则替换已经相当复杂(尤其是托盘重启处理和 legacy bridge 标志相关的逻辑);可以考虑把这些转换封装起来,或者增加一些简单的结构性检查(例如验证导入和调用点是否同时被更新),以减少在稍有差异的上游源码上出现“只打了一半补丁”的风险。

## 逐条评论

### 评论 1
<location> `scripts/prepare-resources.mjs:229-230` </location>
<code_context>

 const MODERN_DESKTOP_BRIDGE_PATTERNS = {
   trayRestartGuard: /if\s*\(\s*!desktopBridge\?\.onTrayRestartBackend\s*\)\s*\{/,
+  trayRestartPromptInvoke: /await\s+globalWaitingRef\.value\?\.check\?\.\(\s*\)/,
   desktopBridgeTypeIsDesktop: /^\s+isDesktop:\s*boolean;\r?\n/m,
   desktopBridgeTypeRuntime: /^\s+isDesktopRuntime:\s*\(\)\s*=>\s*Promise<boolean>;\r?\n/m,
</code_context>

<issue_to_address>
**suggestion (bug_risk):** trayRestartPromptInvoke 的正则目前非常严格,可能会因为细微的格式差异漏掉合法的现代调用点。

这个正则只匹配完全等于 `await globalWaitingRef.value?.check?.()` 的字符串(不带分号、`?.` 周围没有额外空格、`()` 前后无空格、没有换行或参数)。任何格式变动(例如多一些空格、加分号、换行、或者增加参数)都会导致 `hasModernTrayRestartHook` 检测失败,从而错误地回退到 legacy 补丁。建议让该模式在空白字符和末尾可选分号上更宽松一些,以便对常见的格式变化更稳健。

```suggestion
  trayRestartGuard: /if\s*\(\s*!desktopBridge\?\.onTrayRestartBackend\s*\)\s*\{/,
  // Match `await globalWaitingRef.value?.check?.(...)` with flexible whitespace, optional args, and optional trailing semicolon
  trayRestartPromptInvoke: /await\s+globalWaitingRef\s*\.\s*value\s*\?\.\s*check\s*\?\.\s*\(\s*[^)]*\s*\)\s*;?/,
```
</issue_to_address>

### 评论 2
<location> `README.md:143` </location>
<code_context>
 推荐日常使用 `make update`,避免本地切换分支导致版本漂移。

-补充:`make build` 会默认使用当前 `package.json` 中的版本作为 `ASTRBOT_DESKTOP_VERSION`,避免构建前资源准备阶段把版本回写到其他值。若需覆盖,可显式传入 `ASTRBOT_DESKTOP_VERSION=...`。
+补充:`make build` 会默认使用当前 `package.json` 中的版本作为 `ASTRBOT_DESKTOP_VERSION`。若当前环境设置了 `ASTRBOT_SOURCE_DIR`,构建会直接使用该本地源码;也可通过 `ASTRBOT_BUILD_SOURCE_DIR` 显式覆盖。若需覆盖版本,可传入 `ASTRBOT_DESKTOP_VERSION=...`。

 示例:
</code_context>

<issue_to_address>
**suggestion:** 建议在文档中说明同时设置 ASTRBOT_SOURCE_DIR 和 ASTRBOT_BUILD_SOURCE_DIR 时的优先级。

由于这里提供了两种方式将 `make build` 指向本地源码,最好简要说明当两者同时设置时的优先级(例如:“若同时设置,则以 ASTRBOT_BUILD_SOURCE_DIR 为准”)。

```suggestion
补充:`make build` 会默认使用当前 `package.json` 中的版本作为 `ASTRBOT_DESKTOP_VERSION`。若当前环境设置了 `ASTRBOT_SOURCE_DIR`,构建会直接使用该本地源码;也可通过 `ASTRBOT_BUILD_SOURCE_DIR` 显式覆盖,若同时设置,则以 `ASTRBOT_BUILD_SOURCE_DIR` 为准。若需覆盖版本,可传入 `ASTRBOT_DESKTOP_VERSION=...`。
```
</issue_to_address>

Sourcery 对开源项目免费——如果你喜欢我们的评审,欢迎分享 ✨
帮我变得更有用!请对每条评论点击 👍 或 👎,我会根据你的反馈改进评审质量。
Original comment in English

Hey - I've found 2 issues, and left some high level feedback:

  • In the build target, ASTRBOT_SOURCE_DIR is always set (even when build_source_dir is empty), which will override any existing environment-provided value with an empty string; consider only exporting ASTRBOT_SOURCE_DIR when build_source_dir is non-empty to avoid surprising callers.
  • The chained regex-based replacements in patchLegacyDesktopBridgeArtifacts are getting fairly intricate (especially around tray restart handling and legacy bridge flags); consider encapsulating these transformations or adding simple structural checks (e.g., verify both import and call sites were updated together) to reduce the chance of partially-applied patches on slightly divergent upstream sources.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In the `build` target, `ASTRBOT_SOURCE_DIR` is always set (even when `build_source_dir` is empty), which will override any existing environment-provided value with an empty string; consider only exporting `ASTRBOT_SOURCE_DIR` when `build_source_dir` is non-empty to avoid surprising callers.
- The chained regex-based replacements in `patchLegacyDesktopBridgeArtifacts` are getting fairly intricate (especially around tray restart handling and legacy bridge flags); consider encapsulating these transformations or adding simple structural checks (e.g., verify both import and call sites were updated together) to reduce the chance of partially-applied patches on slightly divergent upstream sources.

## Individual Comments

### Comment 1
<location> `scripts/prepare-resources.mjs:229-230` </location>
<code_context>

 const MODERN_DESKTOP_BRIDGE_PATTERNS = {
   trayRestartGuard: /if\s*\(\s*!desktopBridge\?\.onTrayRestartBackend\s*\)\s*\{/,
+  trayRestartPromptInvoke: /await\s+globalWaitingRef\.value\?\.check\?\.\(\s*\)/,
   desktopBridgeTypeIsDesktop: /^\s+isDesktop:\s*boolean;\r?\n/m,
   desktopBridgeTypeRuntime: /^\s+isDesktopRuntime:\s*\(\)\s*=>\s*Promise<boolean>;\r?\n/m,
</code_context>

<issue_to_address>
**suggestion (bug_risk):** The trayRestartPromptInvoke regex is very strict and may miss valid modern call sites with small formatting differences.

This regex only matches the exact string `await globalWaitingRef.value?.check?.()` (no trailing semicolon, no extra spaces around `?.` or before `()`, no line breaks or arguments). Any formatting change (e.g., extra spaces, a semicolon, line wrapping, or added arguments) would cause `hasModernTrayRestartHook` to fail detection and incorrectly fall back to the legacy patch. Consider making the pattern tolerant of whitespace and an optional trailing semicolon so it’s robust to typical formatting changes.

```suggestion
  trayRestartGuard: /if\s*\(\s*!desktopBridge\?\.onTrayRestartBackend\s*\)\s*\{/,
  // Match `await globalWaitingRef.value?.check?.(...)` with flexible whitespace, optional args, and optional trailing semicolon
  trayRestartPromptInvoke: /await\s+globalWaitingRef\s*\.\s*value\s*\?\.\s*check\s*\?\.\s*\(\s*[^)]*\s*\)\s*;?/,
```
</issue_to_address>

### Comment 2
<location> `README.md:143` </location>
<code_context>
 推荐日常使用 `make update`,避免本地切换分支导致版本漂移。

-补充:`make build` 会默认使用当前 `package.json` 中的版本作为 `ASTRBOT_DESKTOP_VERSION`,避免构建前资源准备阶段把版本回写到其他值。若需覆盖,可显式传入 `ASTRBOT_DESKTOP_VERSION=...`。
+补充:`make build` 会默认使用当前 `package.json` 中的版本作为 `ASTRBOT_DESKTOP_VERSION`。若当前环境设置了 `ASTRBOT_SOURCE_DIR`,构建会直接使用该本地源码;也可通过 `ASTRBOT_BUILD_SOURCE_DIR` 显式覆盖。若需覆盖版本,可传入 `ASTRBOT_DESKTOP_VERSION=...`。

 示例:
</code_context>

<issue_to_address>
**suggestion:** Consider clarifying the precedence between ASTRBOT_SOURCE_DIR and ASTRBOT_BUILD_SOURCE_DIR when both are set.

Since this introduces two ways to point `make build` at local source, a brief note on their precedence when both are set would be helpful (e.g., “若同时设置,则以 ASTRBOT_BUILD_SOURCE_DIR 为准”).

```suggestion
补充:`make build` 会默认使用当前 `package.json` 中的版本作为 `ASTRBOT_DESKTOP_VERSION`。若当前环境设置了 `ASTRBOT_SOURCE_DIR`,构建会直接使用该本地源码;也可通过 `ASTRBOT_BUILD_SOURCE_DIR` 显式覆盖,若同时设置,则以 `ASTRBOT_BUILD_SOURCE_DIR` 为准。若需覆盖版本,可传入 `ASTRBOT_DESKTOP_VERSION=...`。
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread scripts/prepare-resources.mjs Outdated
Comment thread README.md Outdated
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces several improvements to the desktop application's build process, resource management, and user experience. However, a medium-severity vulnerability was identified in the Makefile. The newly added build and environment cleanup targets use environment variables directly in shell commands without proper sanitization, which could lead to command injection and arbitrary file write risks, especially in CI/CD environments. Aside from this, the PR stabilizes the tray restart UX, modernizes desktop bridge resource patching, and refines local build and environment controls, including updates to the Makefile, README.md, scripts/prepare-resources.mjs, and src-tauri/src/main.rs. Other changes in the Rust code and resource preparation scripts were found to follow secure coding practices. Overall, these changes aim to enhance the robustness, clarity, and maintainability of the desktop application's build and runtime processes.

@zouyonghe
Copy link
Copy Markdown
Member Author

@sourcery-ai review

Copy link
Copy Markdown

@SourceryAI SourceryAI left a comment

Choose a reason for hiding this comment

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

Hey - 我发现了 1 个问题,并给出了一些整体性的反馈:

  • Makefile 中新增的 build 目标在 if [ -n "$$build_source_dir" ] 这个分支语句的两个分支里,都重复了大部分环境变量的设置;可以考虑先统一设置共享的环境变量,只在需要时有条件地覆盖 ASTRBOT_SOURCE_DIR,这样既能简化逻辑,又能减少未来配置漂移的风险。
  • emit_tray_restart_backend_event 中,传给 window.eval 的内联 JS 字符串比较长且不够直观;将其提取为一个具名常量(或辅助函数)会让意图更清晰,也能降低之后修改时出现引号/转义等细微问题的风险。
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Makefile 中新增的 `build` 目标在 `if [ -n "$$build_source_dir" ]` 这个分支语句的两个分支里,都重复了大部分环境变量的设置;可以考虑先统一设置共享的环境变量,只在需要时有条件地覆盖 `ASTRBOT_SOURCE_DIR`,这样既能简化逻辑,又能减少未来配置漂移的风险。
-`emit_tray_restart_backend_event` 中,传给 `window.eval` 的内联 JS 字符串比较长且不够直观;将其提取为一个具名常量(或辅助函数)会让意图更清晰,也能降低之后修改时出现引号/转义等细微问题的风险。

## Individual Comments

### Comment 1
<location> `src-tauri/src/main.rs:1418-1427` </location>
<code_context>
     }
 }

+fn emit_tray_restart_backend_event(app_handle: &AppHandle) {
+    let Some(window) = app_handle.get_webview_window("main") else {
+        append_desktop_log("tray restart event skipped: main window not found");
+        return;
+    };
+
+    if let Err(error) = window.eval(
+        "if (typeof window !== 'undefined' && typeof window.__astrbotDesktopEmitTrayRestart === 'function') { window.__astrbotDesktopEmitTrayRestart(); }",
+    ) {
+        append_desktop_log(&format!("failed to emit tray restart event: {error}"));
+    }
+}
</code_context>

<issue_to_address>
**suggestion:** 托盘重启通知使用了原始的 JS eval,相比 Tauri 的事件系统更脆弱。

`emit_tray_restart_backend_event` 使用 `window.eval` 并在其中对 `window.__astrbotDesktopEmitTrayRestart()` 进行硬编码调用,这使得 Rust 与一个特定的全局变量强耦合,同时绕开了 Tauri 的事件 API。建议改为发出一个具名的 Tauri 事件(`emit` / `emit_to`),并在前端订阅该事件,以避免“字符串驱动”的 JS,同时保持与其他事件的风格一致。

建议实现:

```rust
fn emit_tray_restart_backend_event(app_handle: &AppHandle) {
    let Some(window) = app_handle.get_webview_window("main") else {
        append_desktop_log("tray restart event skipped: main window not found");
        return;
    };

    // 使用 Tauri 的事件系统而不是原始 JS eval 来通知前端。
    // 前端应订阅 "tray_restart_backend" 事件。
    if let Err(error) = window.emit("tray_restart_backend", ()) {
        append_desktop_log(&format!(
            "failed to emit tray restart backend event: {error}"
        ));
    }
}

```

要完成从 `window.eval` 到 Tauri 事件的迁移,前端需要:

1. 在主窗口上订阅 `"tray_restart_backend"` 事件(使用合适的 Tauri JS API,例如 `listen("tray_restart_backend", ...)`)。
2. 在该事件监听器中,调用之前由 `window.__astrbotDesktopEmitTrayRestart()` 触发的现有重启处理函数,或者如果不再需要该全局变量,则可以把逻辑内联到监听器中。
3. 在所有调用方都改用事件驱动方案后,可以选择性地移除 `window.__astrbotDesktopEmitTrayRestart` 这个全局变量。

如果该代码库中已有事件命名规范(例如 `astrbot://tray-restart-backend` 等),你可能需要将这里的 `"tray_restart_backend"` 字符串按该约定进行调整,并在 Rust 端和前端订阅处保持一致。
</issue_to_address>

Hi @zouyonghe! 👋

感谢你通过评论 @sourcery-ai review 来试用 Sourcery!🚀

安装 sourcery-ai bot 即可在每个 Pull Request 上自动获得代码审查 ✨

帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的评审。
Original comment in English

Hey - I've found 1 issue, and left some high level feedback:

  • The new build target in the Makefile duplicates most of the environment setup in both branches of the if [ -n "$$build_source_dir" ] block; consider setting the shared env vars once and only conditionally overriding ASTRBOT_SOURCE_DIR to simplify and reduce the chance of future drift.
  • In emit_tray_restart_backend_event, the inline JS string passed to window.eval is fairly long and opaque; extracting it to a named constant (or helper) would make the intent clearer and reduce the risk of subtle quoting/escaping issues in future changes.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new `build` target in the Makefile duplicates most of the environment setup in both branches of the `if [ -n "$$build_source_dir" ]` block; consider setting the shared env vars once and only conditionally overriding `ASTRBOT_SOURCE_DIR` to simplify and reduce the chance of future drift.
- In `emit_tray_restart_backend_event`, the inline JS string passed to `window.eval` is fairly long and opaque; extracting it to a named constant (or helper) would make the intent clearer and reduce the risk of subtle quoting/escaping issues in future changes.

## Individual Comments

### Comment 1
<location> `src-tauri/src/main.rs:1418-1427` </location>
<code_context>
     }
 }

+fn emit_tray_restart_backend_event(app_handle: &AppHandle) {
+    let Some(window) = app_handle.get_webview_window("main") else {
+        append_desktop_log("tray restart event skipped: main window not found");
+        return;
+    };
+
+    if let Err(error) = window.eval(
+        "if (typeof window !== 'undefined' && typeof window.__astrbotDesktopEmitTrayRestart === 'function') { window.__astrbotDesktopEmitTrayRestart(); }",
+    ) {
+        append_desktop_log(&format!("failed to emit tray restart event: {error}"));
+    }
+}
</code_context>

<issue_to_address>
**suggestion:** Tray restart notification uses raw JS eval, which is more fragile than Tauri’s event system.

`emit_tray_restart_backend_event` uses `window.eval` with a hard‑coded call to `window.__astrbotDesktopEmitTrayRestart()`, tightly coupling Rust to a specific global and bypassing Tauri’s event API. Prefer emitting a named Tauri event (`emit`/`emit_to`) and subscribing on the frontend to avoid stringly‑typed JS and keep this consistent with other events.

Suggested implementation:

```rust
fn emit_tray_restart_backend_event(app_handle: &AppHandle) {
    let Some(window) = app_handle.get_webview_window("main") else {
        append_desktop_log("tray restart event skipped: main window not found");
        return;
    };

    // Use Tauri's event system instead of raw JS eval to notify the frontend.
    // Frontend should subscribe to the "tray_restart_backend" event.
    if let Err(error) = window.emit("tray_restart_backend", ()) {
        append_desktop_log(&format!(
            "failed to emit tray restart backend event: {error}"
        ));
    }
}

```

To finish the migration from `window.eval` to Tauri events, the frontend needs to:

1. Subscribe to the `"tray_restart_backend"` event on the main window (using the appropriate Tauri JS API, e.g. `listen("tray_restart_backend", ...)`).
2. In that listener, call the existing restart handler that `window.__astrbotDesktopEmitTrayRestart()` previously invoked, or inline the logic if that global is no longer needed.
3. Optionally, remove the `window.__astrbotDesktopEmitTrayRestart` global once all callers use the event-based approach.

If there is an existing naming convention for events in this codebase (e.g. `astrbot://tray-restart-backend` or similar), you may want to adjust the `"tray_restart_backend"` string to match that convention in both Rust and the frontend subscription.
</issue_to_address>

Hi @zouyonghe! 👋

Thanks for trying out Sourcery by commenting with @sourcery-ai review! 🚀

Install the sourcery-ai bot to get automatic code reviews on every pull request ✨

Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src-tauri/src/main.rs Outdated
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - 我发现了两个问题,并且留下了一些高层面的反馈:

  • prepare-resources.mjs 中新增的 DESKTOP_BRIDGE_EXPECTATIONS 正则检查对上游 dash 的改动非常脆弱;建议放宽匹配模式,或者把这些检查放到带版本/特性开关的分组下,这样上游仅有轻微的布局或格式变动时,不至于直接导致构建失败。
  • emit_tray_restart_backend_event 中,将 JS 片段作为一个长字符串字面量传给 window.eval 来执行,会让逻辑更难维护和理解;建议把这段逻辑移到一个专用的 helper 中(或者改用前端订阅的 Tauri 事件)来代替直接 eval JS 字符串。
  • make clean-env 这个目标在 ASTRBOT_RESET_ENV_SCRIPT 已经存在时会直接静默覆盖它;如果这不是有意为之,或许可以增加提示,或者要求显式确认后再覆盖,以避免误删用户自定义的脚本。
给 AI Agent 的提示
Please address the comments from this code review:

## Overall Comments
- `prepare-resources.mjs` 中新增的 `DESKTOP_BRIDGE_EXPECTATIONS` 正则检查对上游 dash 的改动非常脆弱;建议放宽匹配模式,或者把这些检查放到带版本/特性开关的分组下,这样上游仅有轻微的布局或格式变动时,不至于直接导致构建失败。
-`emit_tray_restart_backend_event` 中,将 JS 片段作为一个长字符串字面量传给 `window.eval` 来执行,会让逻辑更难维护和理解;建议把这段逻辑移到一个专用的 helper 中(或者改用前端订阅的 Tauri 事件)来代替直接 `eval` JS 字符串。
- `make clean-env` 这个目标在 `ASTRBOT_RESET_ENV_SCRIPT` 已经存在时会直接静默覆盖它;如果这不是有意为之,或许可以增加提示,或者要求显式确认后再覆盖,以避免误删用户自定义的脚本。

## Individual Comments

### Comment 1
<location> `src-tauri/src/main.rs:1393-1399` </location>
<code_context>
         TRAY_MENU_RESTART_BACKEND => {
             append_desktop_log("tray requested backend restart");
             show_main_window(app_handle);
</code_context>

<issue_to_address>
**suggestion:** 使用硬编码的 `eval` 字符串来触发前端 hook 较为脆弱,也更难追踪和重构。

当前的托盘重启,是通过执行一段 JS 代码片段来触发一个“魔法”全局函数(`window.__astrbotDesktopEmitTrayRestart()`),只要前端改动了这部分就很容易被破坏。

在可行的情况下,更推荐发出一个 Tauri 事件(`emit_all` / `emit_to`),并通过正常的事件系统处理,这样 Rust 端可以保持类型安全,也不需要依赖自定义的 `eval` 字符串。

如果目前确实需要保留 `eval`,至少也可以把这段 JS 片段或函数名抽取成一个共享的常量,这样需要更新时只改一个地方即可。
</issue_to_address>

### Comment 2
<location> `Makefile:150-159` </location>
<code_context>
 clean-node:
 	rm -rf $(NODE_MODULES_DIR) $(PNPM_STORE_DIR)

+clean-env:
+	@set -e; \
+	reset_script="$(ASTRBOT_RESET_ENV_SCRIPT)"; \
+	{ \
+		echo "#!/usr/bin/env sh"; \
+		echo "unset $(ASTRBOT_ENV_KEYS)"; \
+	} > "$$reset_script"; \
+	chmod +x "$$reset_script"; \
+	echo "Generated $$reset_script"; \
+	echo "Run: source $$reset_script"
+
 clean: clean-rust clean-resources clean-vendor clean-node
</code_context>

<issue_to_address>
**question (bug_risk):** 生成的重置脚本只会取消设置一组固定的环境变量;需要确认这是否覆盖了构建依赖的全部状态。

`clean-env` 只会调用 `unset $(ASTRBOT_ENV_KEYS)`,因此任何未来对构建至关重要的环境变量(例如:标志位、缓存目录、AstrBot 配置等)都必须手动加入,否则重置脚本会留下“脏”的环境。建议在 `ASTRBOT_ENV_KEYS` 附近对此进行文档说明(或者通过某种约定/单一事实来源来约束),以避免今后新增的环境变量被遗漏。
</issue_to_address>

Sourcery 对开源项目是免费的——如果你觉得我们的评审有帮助,欢迎分享 ✨
帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据反馈改进后续评审。
Original comment in English

Hey - I've found 2 issues, and left some high level feedback:

  • The new DESKTOP_BRIDGE_EXPECTATIONS regex checks in prepare-resources.mjs are quite brittle against upstream dash changes; consider loosening the patterns or grouping them under a versioned/feature flag so minor layout or formatting changes don’t hard-fail the build.
  • In emit_tray_restart_backend_event, embedding the JS snippet as a long string literal passed to window.eval makes it harder to maintain and reason about; consider moving this into a dedicated helper (or using a Tauri event that the front end subscribes to) rather than raw eval-ed JS.
  • The make clean-env target silently overwrites ASTRBOT_RESET_ENV_SCRIPT if it already exists; if that’s not intentional, you might want to warn or require an explicit override to avoid clobbering a customized script.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new `DESKTOP_BRIDGE_EXPECTATIONS` regex checks in `prepare-resources.mjs` are quite brittle against upstream dash changes; consider loosening the patterns or grouping them under a versioned/feature flag so minor layout or formatting changes don’t hard-fail the build.
- In `emit_tray_restart_backend_event`, embedding the JS snippet as a long string literal passed to `window.eval` makes it harder to maintain and reason about; consider moving this into a dedicated helper (or using a Tauri event that the front end subscribes to) rather than raw `eval`-ed JS.
- The `make clean-env` target silently overwrites `ASTRBOT_RESET_ENV_SCRIPT` if it already exists; if that’s not intentional, you might want to warn or require an explicit override to avoid clobbering a customized script.

## Individual Comments

### Comment 1
<location> `src-tauri/src/main.rs:1393-1399` </location>
<code_context>
         TRAY_MENU_RESTART_BACKEND => {
             append_desktop_log("tray requested backend restart");
             show_main_window(app_handle);
</code_context>

<issue_to_address>
**suggestion:** Using a hard-coded `eval` string to trigger the frontend hook is somewhat fragile and harder to track/refactor.

The tray restart is currently triggered by evaluating a JS snippet that calls a magic global (`window.__astrbotDesktopEmitTrayRestart()`), which is easy to break if the frontend changes it.

Where possible, prefer emitting a Tauri event (`emit_all` / `emit_to`) and handling it through the normal event system so the Rust side stays typed and doesn’t rely on custom `eval` strings.

If you need to keep `eval` for now, at least extract the JS snippet or function name into a shared const so it can be updated in one place.
</issue_to_address>

### Comment 2
<location> `Makefile:150-159` </location>
<code_context>
 clean-node:
 	rm -rf $(NODE_MODULES_DIR) $(PNPM_STORE_DIR)

+clean-env:
+	@set -e; \
+	reset_script="$(ASTRBOT_RESET_ENV_SCRIPT)"; \
+	{ \
+		echo "#!/usr/bin/env sh"; \
+		echo "unset $(ASTRBOT_ENV_KEYS)"; \
+	} > "$$reset_script"; \
+	chmod +x "$$reset_script"; \
+	echo "Generated $$reset_script"; \
+	echo "Run: source $$reset_script"
+
 clean: clean-rust clean-resources clean-vendor clean-node
</code_context>

<issue_to_address>
**question (bug_risk):** The generated reset script will only unset a fixed set of env vars; confirm this matches all state the build depends on.

`clean-env` unsets only `$(ASTRBOT_ENV_KEYS)`, so any future build-critical env vars (e.g., flags, cache dirs, AstrBot config) must be manually added or the reset script will leave a “dirty” environment. Consider documenting this near `ASTRBOT_ENV_KEYS` (or enforcing a convention/single source of truth) so new env vars aren’t missed.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src-tauri/src/main.rs
Comment thread Makefile Outdated
@zouyonghe
Copy link
Copy Markdown
Member Author

@sourcery-ai review

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - 我发现了 1 个问题,并给出了一些整体性的反馈:

  • 托盘重启后端事件名(astrbot://tray-restart-backend)在 Rust 和注入的 JS bridge 中都有重复定义;建议将其集中管理(例如通过共享常量或构建时替换),以避免未来改动时事件绑定被悄然搞乱。
  • 在桌面 bridge 的启动代码里,listenToTrayRestartBackendEventawait tauriEvent.listen,但既没有使用其返回的 unlisten 回调,也没有防止多次注册;如果脚本有可能被多次执行(例如在 HMR 期间),建议增加一个简单的幂等检查,或者存储/注销该监听器,以避免监听器不断累积。
给 AI Agents 的提示词
Please address the comments from this code review:

## Overall Comments
- The tray restart backend event name (`astrbot://tray-restart-backend`) is duplicated in Rust and the injected JS bridge; consider centralizing this (e.g., via a shared constant or build-time substitution) so a future change cannot silently desynchronize the event wiring.
- In the desktop bridge bootstrap, `listenToTrayRestartBackendEvent` awaits `tauriEvent.listen` but never uses the returned unlisten callback or guards against multiple registrations; if the script can be evaluated more than once (e.g., during HMR), consider adding a simple idempotency check or storing/unsubscribing the listener to avoid accumulating handlers.

## Individual Comments

### Comment 1
<location> `src-tauri/src/main.rs:1638-1641` </location>
<code_context>
   }

   const invoke = window.__TAURI_INTERNALS__?.invoke;
+  const tauriEvent = window.__TAURI_INTERNALS__?.event;
   if (typeof invoke !== 'function') return;

</code_context>

<issue_to_address>
**suggestion:** Consider handling the case where `tauriEvent.listen` rejects with a more informative log for easier debugging.

Right now `listenToTrayRestartBackendEvent` swallows any `listen` errors with `catch {}`. That makes integration issues or Tauri version mismatches difficult to diagnose. At least log a warning (optionally behind a debug flag), e.g. `console.warn('Failed to listen for tray restart backend event', error);` so failures leave a trace.

Suggested implementation:

```rust
  const invoke = window.__TAURI_INTERNALS__?.invoke;
  const tauriEvent = window.__TAURI_INTERNALS__?.event;

```

```rust
  } catch (error) {
    // Swallowing this makes Tauri integration issues very hard to debug.
    // At least leave a trace so we can see failures in the console.
    console.warn('Failed to listen for tray restart backend event', error);
  }

```

The second edit assumes that `catch {}` is the catch block for `listenToTrayRestartBackendEvent` (the function wiring `tauriEvent.listen` to `TRAY_RESTART_BACKEND_EVENT`). If there are multiple bare `catch {}` blocks in the embedded script, narrow the `SEARCH` pattern to the specific function, e.g.:

- Include the function header in the `SEARCH` block:
  `async function listenToTrayRestartBackendEvent() {` … `} catch {}`

This will ensure only the intended catch is updated to log
`'Failed to listen for tray restart backend event'`.
</issue_to_address>

Sourcery 对开源项目免费使用——如果你觉得我们的评审有帮助,请考虑分享它 ✨
帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的评审。
Original comment in English

Hey - I've found 1 issue, and left some high level feedback:

  • The tray restart backend event name (astrbot://tray-restart-backend) is duplicated in Rust and the injected JS bridge; consider centralizing this (e.g., via a shared constant or build-time substitution) so a future change cannot silently desynchronize the event wiring.
  • In the desktop bridge bootstrap, listenToTrayRestartBackendEvent awaits tauriEvent.listen but never uses the returned unlisten callback or guards against multiple registrations; if the script can be evaluated more than once (e.g., during HMR), consider adding a simple idempotency check or storing/unsubscribing the listener to avoid accumulating handlers.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The tray restart backend event name (`astrbot://tray-restart-backend`) is duplicated in Rust and the injected JS bridge; consider centralizing this (e.g., via a shared constant or build-time substitution) so a future change cannot silently desynchronize the event wiring.
- In the desktop bridge bootstrap, `listenToTrayRestartBackendEvent` awaits `tauriEvent.listen` but never uses the returned unlisten callback or guards against multiple registrations; if the script can be evaluated more than once (e.g., during HMR), consider adding a simple idempotency check or storing/unsubscribing the listener to avoid accumulating handlers.

## Individual Comments

### Comment 1
<location> `src-tauri/src/main.rs:1638-1641` </location>
<code_context>
   }

   const invoke = window.__TAURI_INTERNALS__?.invoke;
+  const tauriEvent = window.__TAURI_INTERNALS__?.event;
   if (typeof invoke !== 'function') return;

</code_context>

<issue_to_address>
**suggestion:** Consider handling the case where `tauriEvent.listen` rejects with a more informative log for easier debugging.

Right now `listenToTrayRestartBackendEvent` swallows any `listen` errors with `catch {}`. That makes integration issues or Tauri version mismatches difficult to diagnose. At least log a warning (optionally behind a debug flag), e.g. `console.warn('Failed to listen for tray restart backend event', error);` so failures leave a trace.

Suggested implementation:

```rust
  const invoke = window.__TAURI_INTERNALS__?.invoke;
  const tauriEvent = window.__TAURI_INTERNALS__?.event;

```

```rust
  } catch (error) {
    // Swallowing this makes Tauri integration issues very hard to debug.
    // At least leave a trace so we can see failures in the console.
    console.warn('Failed to listen for tray restart backend event', error);
  }

```

The second edit assumes that `catch {}` is the catch block for `listenToTrayRestartBackendEvent` (the function wiring `tauriEvent.listen` to `TRAY_RESTART_BACKEND_EVENT`). If there are multiple bare `catch {}` blocks in the embedded script, narrow the `SEARCH` pattern to the specific function, e.g.:

- Include the function header in the `SEARCH` block:
  `async function listenToTrayRestartBackendEvent() {` … `} catch {}`

This will ensure only the intended catch is updated to log
`'Failed to listen for tray restart backend event'`.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src-tauri/src/main.rs Outdated
@zouyonghe
Copy link
Copy Markdown
Member Author

@sourcery-ai review

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - 我发现了 1 个问题,并给出了一些整体性的反馈:

  • desktop_bridge_bootstrap_script() 中,建议在 replace 之前先断言 DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE 仍然包含 DESKTOP_BRIDGE_TRAY_EVENT_PLACEHOLDER,这样未来如果有人移除/重命名该占位符,就不会在事件绑定上静默出错。
  • verifyDesktopBridgeArtifacts 中,DESKTOP_BRIDGE_PATTERNS.desktopRuntimeUsage 同时被 restartAstrBot.tsVerticalHeader.vue 复用;如果这两个文件在调用该 helper 的方式上有所不同,可以考虑按文件拆分成更有针对性的匹配模式,以避免上游重构用法时出现脆弱的误报/漏报。
  • clean-env 目标只会在重置脚本不存在时才写入;如果 ASTRBOT_ENV_KEYS 发生变化,或者用户希望重新生成脚本,这会比较出乎意料——建议改为每次都重新生成脚本(或者新增一个 clean-env-force 变体),以确保它与 Makefile 保持同步。
面向 AI Agent 的提示词
请根据本次代码评审中的评论进行修改:

## 总体评论
-`desktop_bridge_bootstrap_script()` 中,建议在 `replace` 之前先断言 `DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE` 仍然包含 `DESKTOP_BRIDGE_TRAY_EVENT_PLACEHOLDER`,这样未来如果有人移除/重命名该占位符,就不会在事件绑定上静默出错。
-`verifyDesktopBridgeArtifacts` 中,`DESKTOP_BRIDGE_PATTERNS.desktopRuntimeUsage` 同时被 `restartAstrBot.ts``VerticalHeader.vue` 复用;如果这两个文件在调用该 helper 的方式上有所不同,可以考虑按文件拆分成更有针对性的匹配模式,以避免上游重构用法时出现脆弱的误报/漏报。
- `clean-env` 目标只会在重置脚本不存在时才写入;如果 `ASTRBOT_ENV_KEYS` 发生变化,或者用户希望重新生成脚本,这会比较出乎意料——建议改为每次都重新生成脚本(或者新增一个 `clean-env-force` 变体),以确保它与 Makefile 保持同步。

## 逐条评论

### 评论 1
<location> `Makefile:20-27` </location>
<code_context>
 TAURI_TARGET_DIR ?= src-tauri/target
+# Single source of env keys managed by `make clean-env`.
+# If build/resource scripts start consuming a new persistent env var, add it here.
+ASTRBOT_ENV_KEYS := ASTRBOT_SOURCE_DIR ASTRBOT_SOURCE_GIT_URL ASTRBOT_SOURCE_GIT_REF ASTRBOT_DESKTOP_VERSION ASTRBOT_BUILD_SOURCE_DIR

 .PHONY: help deps sync-version update prepare-webui prepare-backend prepare-resources dev build \
</code_context>

<issue_to_address>
**suggestion (bug_risk):** 如果 `ASTRBOT_ENV_KEYS` 发生变化,`clean-env` 不会更新已存在的重置脚本,这可能导致环境清理逻辑过期。

由于 `clean-env` 目标在 `$(ASTRBOT_RESET_ENV_SCRIPT)` 已存在时会跳过执行,对 `ASTRBOT_ENV_KEYS` 的更改不会反映到已有脚本中。这可能导致部分环境变量无法被清理,或者尝试 unset 已删除的变量。你可以选择每次都重新生成脚本(可以视情况增加交互式确认),或者在脚本中嵌入 `ASTRBOT_ENV_KEYS` 的版本/哈希值,在其变化时重新生成脚本。

建议实现方式:

```
TAURI_TARGET_DIR ?= src-tauri/target
# Single source of env keys managed by `make clean-env`.
# If build/resource scripts start consuming a new persistent env var, add it here.
ASTRBOT_ENV_KEYS := ASTRBOT_SOURCE_DIR ASTRBOT_SOURCE_GIT_URL ASTRBOT_SOURCE_GIT_REF ASTRBOT_DESKTOP_VERSION ASTRBOT_BUILD_SOURCE_DIR
# Hash of the env key set, used by `make clean-env` to detect stale reset scripts.
ASTRBOT_ENV_KEYS_HASH := $(shell printf '%s\n' "$(ASTRBOT_ENV_KEYS)" | shasum -a 256 | cut -d' ' -f1 2>/dev/null || printf '%s\n' "$(ASTRBOT_ENV_KEYS)" | sha256sum | cut -d' ' -f1)

```

你还需要在后面的 `Makefile` 中更新 `clean-env` 目标。将其命令体替换为类似如下内容(根据你环境的路径和参数适当调整):

```make
clean-env:
	@echo "Ensuring $(ASTRBOT_RESET_ENV_SCRIPT) matches current env keys..."
	@reset_script="$(ASTRBOT_RESET_ENV_SCRIPT)"; \
	current_hash="$(ASTRBOT_ENV_KEYS_HASH)"; \
	existing_hash=""; \
	if [ -f "$$reset_script" ]; then \
		# Look for a marker line of the form: "# ASTRBOT_ENV_KEYS_HASH=<hash>"
		existing_hash="$$(grep '^# ASTRBOT_ENV_KEYS_HASH=' "$$reset_script" | head -n1 | cut -d'=' -f2-)"; \
	fi; \
	if [ "$$existing_hash" != "$$current_hash" ]; then \
		echo "Regenerating $$reset_script (env keys changed)..."; \
		{ \
			echo '#!/usr/bin/env sh'; \
			echo "# Generated by 'make clean-env'"; \
			echo "# ASTRBOT_ENV_KEYS_HASH=$$current_hash"; \
			for key in $(ASTRBOT_ENV_KEYS); do \
				printf 'unset %s\n' "$$key"; \
			done; \
		} > "$$reset_script"; \
		chmod +x "$$reset_script"; \
	else \
		echo "$$reset_script is up to date."; \
	fi
```

该实现的关键点:

1. 重置脚本现在包含一行标记 `# ASTRBOT_ENV_KEYS_HASH=<hash>`,其内容对应当前的 `ASTRBOT_ENV_KEYS_HASH`。
2. 每次执行 `make clean-env` 时,该配方会:
   - 如果脚本已存在,则从脚本中读取已有的哈希;
   - 将其与当前的 `ASTRBOT_ENV_KEYS_HASH` 进行比较;
   - 当两者不一致(或脚本不存在)时重新生成脚本。
3. 这样就避免了脚本过期的问题:每当 `ASTRBOT_ENV_KEYS` 发生变化,哈希值也会变化,`clean-env` 就会自动刷新脚本。

确保删除之前在 `clean-env` 中仅以 `$(ASTRBOT_RESET_ENV_SCRIPT)` 是否存在为条件而直接跳过工作的逻辑,避免在未检查哈希的情况下短路退出。
</issue_to_address>

Sourcery 对开源项目是免费的——如果你喜欢我们的评审,请考虑帮我们分享 ✨
帮我变得更有用!请在每条评论上点 👍 或 👎,我会基于这些反馈改进后续评审。
Original comment in English

Hey - I've found 1 issue, and left some high level feedback:

  • In desktop_bridge_bootstrap_script(), consider asserting that DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE still contains DESKTOP_BRIDGE_TRAY_EVENT_PLACEHOLDER before replace, so future edits that remove/rename the placeholder don’t silently break event wiring.
  • In verifyDesktopBridgeArtifacts, DESKTOP_BRIDGE_PATTERNS.desktopRuntimeUsage is reused for both restartAstrBot.ts and VerticalHeader.vue; if those files differ in how they call the helper, consider splitting this into more targeted patterns per file to avoid brittle false negatives when upstream refactors usage.
  • The clean-env target writes the reset script only if it does not exist; if ASTRBOT_ENV_KEYS changes or a user wants to regenerate the script, this will be surprising—consider always regenerating the script (or adding a clean-env-force variant) to keep it in sync with the Makefile.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `desktop_bridge_bootstrap_script()`, consider asserting that `DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE` still contains `DESKTOP_BRIDGE_TRAY_EVENT_PLACEHOLDER` before `replace`, so future edits that remove/rename the placeholder don’t silently break event wiring.
- In `verifyDesktopBridgeArtifacts`, `DESKTOP_BRIDGE_PATTERNS.desktopRuntimeUsage` is reused for both `restartAstrBot.ts` and `VerticalHeader.vue`; if those files differ in how they call the helper, consider splitting this into more targeted patterns per file to avoid brittle false negatives when upstream refactors usage.
- The `clean-env` target writes the reset script only if it does not exist; if `ASTRBOT_ENV_KEYS` changes or a user wants to regenerate the script, this will be surprising—consider always regenerating the script (or adding a `clean-env-force` variant) to keep it in sync with the Makefile.

## Individual Comments

### Comment 1
<location> `Makefile:20-27` </location>
<code_context>
 TAURI_TARGET_DIR ?= src-tauri/target
+# Single source of env keys managed by `make clean-env`.
+# If build/resource scripts start consuming a new persistent env var, add it here.
+ASTRBOT_ENV_KEYS := ASTRBOT_SOURCE_DIR ASTRBOT_SOURCE_GIT_URL ASTRBOT_SOURCE_GIT_REF ASTRBOT_DESKTOP_VERSION ASTRBOT_BUILD_SOURCE_DIR

 .PHONY: help deps sync-version update prepare-webui prepare-backend prepare-resources dev build \
</code_context>

<issue_to_address>
**suggestion (bug_risk):** `clean-env` won’t update an existing reset script if `ASTRBOT_ENV_KEYS` changes, which can lead to stale env cleanup.

Because the `clean-env` target skips work when `$(ASTRBOT_RESET_ENV_SCRIPT)` already exists, changes to `ASTRBOT_ENV_KEYS` won’t be reflected in existing scripts. That can leave some vars uncleared or try to unset removed ones. You could either always regenerate the script (optionally with a prompt) or embed a version/hash of `ASTRBOT_ENV_KEYS` in the script and regenerate when it changes.

Suggested implementation:

```
TAURI_TARGET_DIR ?= src-tauri/target
# Single source of env keys managed by `make clean-env`.
# If build/resource scripts start consuming a new persistent env var, add it here.
ASTRBOT_ENV_KEYS := ASTRBOT_SOURCE_DIR ASTRBOT_SOURCE_GIT_URL ASTRBOT_SOURCE_GIT_REF ASTRBOT_DESKTOP_VERSION ASTRBOT_BUILD_SOURCE_DIR
# Hash of the env key set, used by `make clean-env` to detect stale reset scripts.
ASTRBOT_ENV_KEYS_HASH := $(shell printf '%s\n' "$(ASTRBOT_ENV_KEYS)" | shasum -a 256 | cut -d' ' -f1 2>/dev/null || printf '%s\n' "$(ASTRBOT_ENV_KEYS)" | sha256sum | cut -d' ' -f1)

```

You will also need to update the `clean-env` target later in `Makefile`. Replace its body with something like this (adjust the location and flags as needed for your environment):

```make
clean-env:
	@echo "Ensuring $(ASTRBOT_RESET_ENV_SCRIPT) matches current env keys..."
	@reset_script="$(ASTRBOT_RESET_ENV_SCRIPT)"; \
	current_hash="$(ASTRBOT_ENV_KEYS_HASH)"; \
	existing_hash=""; \
	if [ -f "$$reset_script" ]; then \
		# Look for a marker line of the form: "# ASTRBOT_ENV_KEYS_HASH=<hash>"
		existing_hash="$$(grep '^# ASTRBOT_ENV_KEYS_HASH=' "$$reset_script" | head -n1 | cut -d'=' -f2-)"; \
	fi; \
	if [ "$$existing_hash" != "$$current_hash" ]; then \
		echo "Regenerating $$reset_script (env keys changed)..."; \
		{ \
			echo '#!/usr/bin/env sh'; \
			echo "# Generated by 'make clean-env'"; \
			echo "# ASTRBOT_ENV_KEYS_HASH=$$current_hash"; \
			for key in $(ASTRBOT_ENV_KEYS); do \
				printf 'unset %s\n' "$$key"; \
			done; \
		} > "$$reset_script"; \
		chmod +x "$$reset_script"; \
	else \
		echo "$$reset_script is up to date."; \
	fi
```

Key points of this implementation:

1. The reset script now includes a marker line `# ASTRBOT_ENV_KEYS_HASH=<hash>` that reflects the current `ASTRBOT_ENV_KEYS_HASH`.
2. On each `make clean-env`, the recipe:
   - Reads the existing hash from the script, if it exists.
   - Compares it to the current `ASTRBOT_ENV_KEYS_HASH`.
   - Regenerates the script if they differ (or if the script is missing).
3. This removes the stale-script problem: whenever `ASTRBOT_ENV_KEYS` changes, the hash changes, and `clean-env` will refresh the script automatically.

Ensure you remove any previous logic in `clean-env` that skipped work purely based on `$(ASTRBOT_RESET_ENV_SCRIPT)` already existing or otherwise short-circuiting without checking this hash.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread Makefile
@zouyonghe
Copy link
Copy Markdown
Member Author

@sourcery-ai review

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - 我发现了 1 个问题

给 AI Agents 的提示词
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `src-tauri/src/main.rs:56` </location>
<code_context>
 const TRAY_MENU_RESTART_BACKEND: &str = "tray_restart_backend";
 const TRAY_MENU_QUIT: &str = "tray_quit";
+const TRAY_RESTART_BACKEND_EVENT: &str = "astrbot://tray-restart-backend";
+const DESKTOP_BRIDGE_TRAY_EVENT_PLACEHOLDER: &str = "__ASTRBOT_TRAY_RESTART_BACKEND_EVENT__";
 const DEFAULT_SHELL_LOCALE: &str = "zh-CN";
 const STARTUP_MODE_ENV: &str = "ASTRBOT_DESKTOP_STARTUP_MODE";
</code_context>

<issue_to_address>
**issue (complexity):** 建议通过使用单个由 `format!` 构造的脚本字符串来简化 desktop bridge 的引导初始化,而不是使用基于占位符的模板再加上 `assert!` 和替换逻辑。

你可以在初始化脚本时,通过 `format!` 内联这个常量,从而让事件名称依然保持单一来源,而不需要通过占位符 + `assert!` + 模板间接引用。

这种方式可以保留新的事件接线逻辑和 `OnceLock` 缓存,同时去掉额外的占位符常量、搜索以及断言逻辑:

```rust
const TRAY_RESTART_BACKEND_EVENT: &str = "astrbot://tray-restart-backend";

// 去掉 DESKTOP_BRIDGE_TRAY_EVENT_PLACEHOLDER 和 DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE
// 并改为只定义一个带 Rust 格式占位符的模板:
static DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: OnceLock<String> = OnceLock::new();

fn desktop_bridge_bootstrap_script() -> &'static str {
    DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT
        .get_or_init(|| {
            format!(
                r#"(() => {{
  const existingTrayRestartState = window.__astrbotDesktopTrayRestartState;
  if (
    window.astrbotDesktop &&
    window.astrbotDesktop.__tauriBridge === true &&
    typeof window.astrbotDesktop.onTrayRestartBackend === 'function' &&
    typeof existingTrayRestartState?.unlistenTrayRestartBackendEvent === 'function'
  ) {{
    return;
  }}

  const invoke = window.__TAURI_INTERNALS__?.invoke;
  const tauriEvent = window.__TAURI_INTERNALS__?.event ?? window.__TAURI__?.event;
  if (typeof invoke !== 'function') return;

  const BRIDGE_COMMANDS = Object.freeze({{
    IS_DESKTOP_RUNTIME: 'desktop_bridge_is_desktop_runtime',
    GET_BACKEND_STATE: 'desktop_bridge_get_backend_state',
    SET_AUTH_TOKEN: 'desktop_bridge_set_auth_token',
    RESTART_BACKEND: 'desktop_bridge_restart_backend',
    STOP_BACKEND: 'desktop_bridge_stop_backend',
  }});

  const TRAY_RESTART_BACKEND_EVENT = '{event}';

  // ... rest of the JS stays identical ...
}})();"#,
                event = TRAY_RESTART_BACKEND_EVENT,
            )
        })
        .as_str()
}
```

这样做的好处是:

- 事件名称仍然只在 Rust 中定义一次(`TRAY_RESTART_BACKEND_EVENT`),并同时用于 `window.emit(...)` 和注入的 JS 中。
- 不再有额外的“模板”常量、占位符常量或运行时的 `contains` 检查需要理解和维护。
- 调用端(`webview.eval(desktop_bridge_bootstrap_script())`)的行为保持不变,但其背后的初始化路径更为简单。
</issue_to_address>

Sourcery 对开源项目免费 —— 如果你觉得我们的评审有帮助,欢迎分享 ✨
帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续评审。
Original comment in English

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `src-tauri/src/main.rs:56` </location>
<code_context>
 const TRAY_MENU_RESTART_BACKEND: &str = "tray_restart_backend";
 const TRAY_MENU_QUIT: &str = "tray_quit";
+const TRAY_RESTART_BACKEND_EVENT: &str = "astrbot://tray-restart-backend";
+const DESKTOP_BRIDGE_TRAY_EVENT_PLACEHOLDER: &str = "__ASTRBOT_TRAY_RESTART_BACKEND_EVENT__";
 const DEFAULT_SHELL_LOCALE: &str = "zh-CN";
 const STARTUP_MODE_ENV: &str = "ASTRBOT_DESKTOP_STARTUP_MODE";
</code_context>

<issue_to_address>
**issue (complexity):** Consider simplifying the desktop bridge bootstrap initialization by using a single `format!`-constructed script string instead of a placeholder-based template plus `assert!` and replacement logic.

You can keep the event name single‑sourced without the placeholder + `assert!` + template indirection by inlining the constant via `format!` when initializing the script.

This preserves the new event wiring and the `OnceLock` caching, but removes the extra placeholder constant, search, and assert:

```rust
const TRAY_RESTART_BACKEND_EVENT: &str = "astrbot://tray-restart-backend";

// Drop DESKTOP_BRIDGE_TRAY_EVENT_PLACEHOLDER and DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE
// and define a single template with a Rust-format placeholder instead:
static DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: OnceLock<String> = OnceLock::new();

fn desktop_bridge_bootstrap_script() -> &'static str {
    DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT
        .get_or_init(|| {
            format!(
                r#"(() => {{
  const existingTrayRestartState = window.__astrbotDesktopTrayRestartState;
  if (
    window.astrbotDesktop &&
    window.astrbotDesktop.__tauriBridge === true &&
    typeof window.astrbotDesktop.onTrayRestartBackend === 'function' &&
    typeof existingTrayRestartState?.unlistenTrayRestartBackendEvent === 'function'
  ) {{
    return;
  }}

  const invoke = window.__TAURI_INTERNALS__?.invoke;
  const tauriEvent = window.__TAURI_INTERNALS__?.event ?? window.__TAURI__?.event;
  if (typeof invoke !== 'function') return;

  const BRIDGE_COMMANDS = Object.freeze({{
    IS_DESKTOP_RUNTIME: 'desktop_bridge_is_desktop_runtime',
    GET_BACKEND_STATE: 'desktop_bridge_get_backend_state',
    SET_AUTH_TOKEN: 'desktop_bridge_set_auth_token',
    RESTART_BACKEND: 'desktop_bridge_restart_backend',
    STOP_BACKEND: 'desktop_bridge_stop_backend',
  }});

  const TRAY_RESTART_BACKEND_EVENT = '{event}';

  // ... rest of the JS stays identical ...
}})();"#,
                event = TRAY_RESTART_BACKEND_EVENT,
            )
        })
        .as_str()
}
```

This way:

- The event name is still defined once in Rust (`TRAY_RESTART_BACKEND_EVENT`) and used both for `window.emit(...)` and in the injected JS.
- There is no extra “template” constant, placeholder constant, or runtime `contains` check to understand or maintain.
- The call site (`webview.eval(desktop_bridge_bootstrap_script())`) remains unchanged in behavior but is backed by a simpler initialization path.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src-tauri/src/main.rs Outdated
@zouyonghe
Copy link
Copy Markdown
Member Author

@sourcery-ai review

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - 我发现了 1 个问题,并留了一些整体性的反馈:

  • 新的 verifyDesktopBridgeArtifacts 对上游来源使用了比较脆弱的正则匹配期望;可以考虑把这些模式定义集中管理,并在错误文案里为每个文件加上一小段提示(例如预期片段或上游路径),这样在上游 dashboard 发生变更时更容易对照和排查。
  • 在 desktop bridge 的 bootstrap 脚本中,createEventListener 的回退路径通过 __TAURI_INTERNALS__ 手动调用 plugin:event|listen/unlisten;把这部分封装在更明确的特性检测(或一个统一的工具函数)之后可能会更安全,这样未来 Tauri 升级或插件移除时会以更可控的方式失败。
给 AI Agent 的提示
Please address the comments from this code review:

## Overall Comments
- The new `verifyDesktopBridgeArtifacts` uses quite brittle regex expectations against upstream sources; consider centralizing the pattern definitions and adding a short per-file hint (e.g., expected snippet or upstream path) in the error text to make it easier to reconcile when the upstream dashboard changes.
- In the desktop bridge bootstrap script, the `createEventListener` fallback path manually invokes `plugin:event|listen`/`unlisten` via `__TAURI_INTERNALS__`; it might be safer to gate this behind more explicit feature detection (or a single utility function) so that future Tauri upgrades or plugin removal fail in a more controlled way.

## Individual Comments

### Comment 1
<location> `scripts/prepare-resources.mjs:225-228` </location>
<code_context>
+    /await\s+globalWaitingRef\s*\.\s*value\s*\?\.\s*check\s*\?\.\s*\(\s*[^)]*\s*\)\s*;?/,
+  desktopRuntimeImport:
+    /import\s+\{\s*getDesktopRuntimeInfo\s*\}\s+from\s+['"]@\/utils\/desktopRuntime['"]\s*;?/,
+  desktopRuntimeUsageInRestart:
+    /hasDesktopRestartCapability[\s\S]{0,220}await\s+getDesktopRuntimeInfo\s*\(\s*\)/,
+  desktopRuntimeUsageInHeader:
+    /const\s+runtimeInfo\s*=\s*await\s+getDesktopRuntimeInfo\s*\(\s*\)\s*;?[\s\S]{0,220}isDesktopReleaseMode\.value\s*=\s*runtimeInfo\.isDesktopRuntime/,
</code_context>

<issue_to_address>
**suggestion (bug_risk):** 有界的 `{0,220}` 模式让这个兼容性检查对轻微的格式调整或重构变得非常脆弱。

这个正则假定 `hasDesktopRestartCapability``await getDesktopRuntimeInfo()` 会始终保持在彼此 220 个字符以内,因此哪怕只是做了一些小的插入(日志、注释、空格),也可能在逻辑没变的情况下让检查失败。可以考虑移除上界,或者基于更稳定的结构(例如函数名或多行模式)来匹配,使这个检查能够容忍无害的修改。

```suggestion
  desktopRuntimeUsageInRestart:
    /hasDesktopRestartCapability[\s\S]*?await\s+getDesktopRuntimeInfo\s*\(\s*\)/,
  desktopRuntimeUsageInHeader:
    /const\s+runtimeInfo\s*=\s*await\s+getDesktopRuntimeInfo\s*\(\s*\)\s*;?[\s\S]*?isDesktopReleaseMode\.value\s*=\s*runtimeInfo\.isDesktopRuntime/,
```
</issue_to_address>

Sourcery 对开源项目免费 —— 如果你觉得我们的 review 有帮助,欢迎分享 ✨
帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据反馈改进以后的 review。
Original comment in English

Hey - I've found 1 issue, and left some high level feedback:

  • The new verifyDesktopBridgeArtifacts uses quite brittle regex expectations against upstream sources; consider centralizing the pattern definitions and adding a short per-file hint (e.g., expected snippet or upstream path) in the error text to make it easier to reconcile when the upstream dashboard changes.
  • In the desktop bridge bootstrap script, the createEventListener fallback path manually invokes plugin:event|listen/unlisten via __TAURI_INTERNALS__; it might be safer to gate this behind more explicit feature detection (or a single utility function) so that future Tauri upgrades or plugin removal fail in a more controlled way.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new `verifyDesktopBridgeArtifacts` uses quite brittle regex expectations against upstream sources; consider centralizing the pattern definitions and adding a short per-file hint (e.g., expected snippet or upstream path) in the error text to make it easier to reconcile when the upstream dashboard changes.
- In the desktop bridge bootstrap script, the `createEventListener` fallback path manually invokes `plugin:event|listen`/`unlisten` via `__TAURI_INTERNALS__`; it might be safer to gate this behind more explicit feature detection (or a single utility function) so that future Tauri upgrades or plugin removal fail in a more controlled way.

## Individual Comments

### Comment 1
<location> `scripts/prepare-resources.mjs:225-228` </location>
<code_context>
+    /await\s+globalWaitingRef\s*\.\s*value\s*\?\.\s*check\s*\?\.\s*\(\s*[^)]*\s*\)\s*;?/,
+  desktopRuntimeImport:
+    /import\s+\{\s*getDesktopRuntimeInfo\s*\}\s+from\s+['"]@\/utils\/desktopRuntime['"]\s*;?/,
+  desktopRuntimeUsageInRestart:
+    /hasDesktopRestartCapability[\s\S]{0,220}await\s+getDesktopRuntimeInfo\s*\(\s*\)/,
+  desktopRuntimeUsageInHeader:
+    /const\s+runtimeInfo\s*=\s*await\s+getDesktopRuntimeInfo\s*\(\s*\)\s*;?[\s\S]{0,220}isDesktopReleaseMode\.value\s*=\s*runtimeInfo\.isDesktopRuntime/,
</code_context>

<issue_to_address>
**suggestion (bug_risk):** The bounded `{0,220}` pattern makes this compatibility check fragile to small formatting or refactors.

This regex assumes `hasDesktopRestartCapability` and `await getDesktopRuntimeInfo()` stay within 220 characters of each other, so minor inserts (logging, comments, spacing) can cause the check to fail even when the logic is unchanged. Consider removing the upper bound or keying off a more stable structure (e.g., function name or a multi-line pattern) so the check tolerates harmless edits.

```suggestion
  desktopRuntimeUsageInRestart:
    /hasDesktopRestartCapability[\s\S]*?await\s+getDesktopRuntimeInfo\s*\(\s*\)/,
  desktopRuntimeUsageInHeader:
    /const\s+runtimeInfo\s*=\s*await\s+getDesktopRuntimeInfo\s*\(\s*\)\s*;?[\s\S]*?isDesktopReleaseMode\.value\s*=\s*runtimeInfo\.isDesktopRuntime/,
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread scripts/prepare-resources.mjs Outdated
@zouyonghe
Copy link
Copy Markdown
Member Author

@sourcery-ai review

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - 我发现了 1 个问题

给 AI 代理的提示
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `src-tauri/src/main.rs:1630` </location>
<code_context>
 }

-const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: &str = r#"
+const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE: &str = r#"
 (() => {
+  const existingTrayRestartState = window.__astrbotDesktopTrayRestartState;
</code_context>

<issue_to_address>
**issue (complexity):** 考虑通过直接格式化脚本、在 JS 中将 token 处理与 pending 逻辑解耦,以及把旧版事件监听路径抽取成单独的 helper,来简化新的托盘重启(tray-restart)引导脚本接线逻辑。

你可以保留所有新的行为,只是通过局部化几个关注点来缩减一些复杂度。

### 1. 用单一的格式化引导脚本构建函数替换占位符 + `OnceLock`

当前的 `DESKTOP_BRIDGE_TRAY_EVENT_PLACEHOLDER` + 模板 + `replace` + `assert!` 增加了间接层,但收益不大。你可以通过用 `format!` 只构建一次脚本来保持事件名的单一事实来源(single source of truth),并完全去掉占位符相关的机制:

```rust
const DESKTOP_BRIDGE_BOOTSTRAP_TEMPLATE: &str = r#"(() => {
  // ...
  const TRAY_RESTART_BACKEND_EVENT = '{TRAY_RESTART_BACKEND_EVENT}';
  // ...
})();
"#;

static DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: OnceLock<String> = OnceLock::new();

fn desktop_bridge_bootstrap_script() -> &'static str {
    DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT
        .get_or_init(|| {
            DESKTOP_BRIDGE_BOOTSTRAP_TEMPLATE.replace(
                "{TRAY_RESTART_BACKEND_EVENT}",
                TRAY_RESTART_BACKEND_EVENT,
            )
        })
        .as_str()
}
```

或者更简单(完全不需要自定义占位符字符串):

```rust
static DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: OnceLock<String> = OnceLock::new();

fn desktop_bridge_bootstrap_script() -> &'static str {
    DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT
        .get_or_init(|| {
            format!(
                r#"(() => {{
  // ...
  const TRAY_RESTART_BACKEND_EVENT = '{event_name}';
  // ...
}})();
"#,
                event_name = TRAY_RESTART_BACKEND_EVENT,
            )
        })
        .as_str()
}
```

这样仍然保持 `TRAY_RESTART_BACKEND_EVENT` 作为唯一事实来源,但移除了额外的占位符常量和运行时的 `assert!` 分支。


### 2. 在 JS 托盘重启逻辑中将 token 语义与 “pending” 语义分离

现在的 `emitTrayRestart` 同时处理:

- 通过 `lastToken` 去重
- 对非数字 token 的回退逻辑
- 以及 pending 队列

把 token 处理抽离到一个非常小的 helper 中,可以让 `emitTrayRestart` 更易读、更容易推理,并让 `pending` 只关心“还没有 handler”的场景:

```js
const trayRestartState =
  window.__astrbotDesktopTrayRestartState ||
  (window.__astrbotDesktopTrayRestartState = {
    handlers: new Set(),
    pending: 0,
    lastToken: 0,
    unlistenTrayRestartBackendEvent: null,
  });

function shouldEmitForToken(token) {
  const numericToken = Number(token);
  if (Number.isFinite(numericToken) && numericToken > 0) {
    if (numericToken <= trayRestartState.lastToken) return false;
    trayRestartState.lastToken = numericToken;
    return true;
  }
  // 非数字 / 缺失 token:视为“新的”并自增
  trayRestartState.lastToken += 1;
  return true;
}

const emitTrayRestart = (token = null) => {
  if (!shouldEmitForToken(token)) return;

  if (trayRestartState.handlers.size === 0) {
    trayRestartState.pending = Number(trayRestartState.pending || 0) + 1;
    return;
  }

  for (const handler of trayRestartState.handlers) {
    try {
      handler();
    } catch {}
  }
};

const listenToTrayRestartBackendEvent = async () => {
  if (typeof trayRestartState.unlistenTrayRestartBackendEvent === 'function') return;
  try {
    const unlisten = await createEventListener(TRAY_RESTART_BACKEND_EVENT, (event) => {
      emitTrayRestart(event?.payload);
    });
    if (typeof unlisten === 'function') {
      trayRestartState.unlistenTrayRestartBackendEvent = unlisten;
    }
  } catch (error) {
    console.warn('Failed to listen for tray restart backend event', error);
  }
};
```

行为保持不变,但 token 与 pending 的职责被清晰地分离开了。


### 3. 从 `createEventListener` 中抽取旧版 `plugin:event|listen` 分支

目前 `createEventListener` 内联了旧版路径(包含 `transformCallback``eventId`、try/catch 等)。把这一块抽取成一个小 helper,可以让主引导脚本主体更短,也让函数本身更简单:

```js
const createLegacyEventListener = async (eventName, handler) => {
  if (typeof transformCallback !== 'function') {
    throw new Error('transformCallback is unavailable');
  }

  const eventId = await invoke('plugin:event|listen', {
    event: eventName,
    target: { kind: 'Any' },
    handler: transformCallback(handler),
  });

  return async () => {
    try {
      window.__TAURI_EVENT_PLUGIN_INTERNALS__?.unregisterListener?.(eventName, eventId);
    } catch {}
    await invoke('plugin:event|unlisten', { event: eventName, eventId });
  };
};

const createEventListener = async (eventName, handler) => {
  if (typeof tauriEvent?.listen === 'function') {
    return tauriEvent.listen(eventName, handler);
  }
  return createLegacyEventListener(eventName, handler);
};
```

这保留了所有兼容性行为,但减少了引导脚本内部的视觉和认知负担。
</issue_to_address>

Sourcery 对开源项目免费——如果你觉得这些评审有帮助,欢迎分享 ✨
帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据反馈改进后续评审。
Original comment in English

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `src-tauri/src/main.rs:1630` </location>
<code_context>
 }

-const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: &str = r#"
+const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE: &str = r#"
 (() => {
+  const existingTrayRestartState = window.__astrbotDesktopTrayRestartState;
</code_context>

<issue_to_address>
**issue (complexity):** Consider simplifying the new tray-restart bootstrap wiring by formatting the script directly, isolating token handling from pending logic in JS, and extracting the legacy event listener path into its own helper for clarity.

You can keep all the new behavior but trim some of the complexity by localizing a few concerns.

### 1. Replace placeholder + `OnceLock` with a single formatted bootstrap builder

The `DESKTOP_BRIDGE_TRAY_EVENT_PLACEHOLDER` + template + `replace` + `assert!` add indirection without much gain. You can keep a single source of truth for the event name by building the script once with `format!`, and drop the placeholder mechanics entirely:

```rust
const DESKTOP_BRIDGE_BOOTSTRAP_TEMPLATE: &str = r#"(() => {
  // ...
  const TRAY_RESTART_BACKEND_EVENT = '{TRAY_RESTART_BACKEND_EVENT}';
  // ...
})();
"#;

static DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: OnceLock<String> = OnceLock::new();

fn desktop_bridge_bootstrap_script() -> &'static str {
    DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT
        .get_or_init(|| {
            DESKTOP_BRIDGE_BOOTSTRAP_TEMPLATE.replace(
                "{TRAY_RESTART_BACKEND_EVENT}",
                TRAY_RESTART_BACKEND_EVENT,
            )
        })
        .as_str()
}
```

Or even simpler (no custom placeholder string at all):

```rust
static DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: OnceLock<String> = OnceLock::new();

fn desktop_bridge_bootstrap_script() -> &'static str {
    DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT
        .get_or_init(|| {
            format!(
                r#"(() => {{
  // ...
  const TRAY_RESTART_BACKEND_EVENT = '{event_name}';
  // ...
}})();
"#,
                event_name = TRAY_RESTART_BACKEND_EVENT,
            )
        })
        .as_str()
}
```

This keeps `TRAY_RESTART_BACKEND_EVENT` as the single source of truth, but removes the extra placeholder constant and runtime `assert!` branch.


### 2. Separate token semantics from “pending” semantics in the JS tray restart logic

Right now `emitTrayRestart` mixes:

- deduplication via `lastToken`
- fallback for non-numeric tokens
- and pending queueing

Pulling token handling into a very small helper makes `emitTrayRestart` easier to read and reason about, and keeps `pending` purely about “no handlers yet”:

```js
const trayRestartState =
  window.__astrbotDesktopTrayRestartState ||
  (window.__astrbotDesktopTrayRestartState = {
    handlers: new Set(),
    pending: 0,
    lastToken: 0,
    unlistenTrayRestartBackendEvent: null,
  });

function shouldEmitForToken(token) {
  const numericToken = Number(token);
  if (Number.isFinite(numericToken) && numericToken > 0) {
    if (numericToken <= trayRestartState.lastToken) return false;
    trayRestartState.lastToken = numericToken;
    return true;
  }
  // non‑numeric / missing token: treat as “new” and bump
  trayRestartState.lastToken += 1;
  return true;
}

const emitTrayRestart = (token = null) => {
  if (!shouldEmitForToken(token)) return;

  if (trayRestartState.handlers.size === 0) {
    trayRestartState.pending = Number(trayRestartState.pending || 0) + 1;
    return;
  }

  for (const handler of trayRestartState.handlers) {
    try {
      handler();
    } catch {}
  }
};

const listenToTrayRestartBackendEvent = async () => {
  if (typeof trayRestartState.unlistenTrayRestartBackendEvent === 'function') return;
  try {
    const unlisten = await createEventListener(TRAY_RESTART_BACKEND_EVENT, (event) => {
      emitTrayRestart(event?.payload);
    });
    if (typeof unlisten === 'function') {
      trayRestartState.unlistenTrayRestartBackendEvent = unlisten;
    }
  } catch (error) {
    console.warn('Failed to listen for tray restart backend event', error);
  }
};
```

The behavior stays the same, but the token and pending responsibilities are clearly separated.


### 3. Extract the legacy `plugin:event|listen` branch from `createEventListener`

`createEventListener` currently inlines the legacy path (with `transformCallback`, `eventId`, try/catch, etc.). Pulling that into a tiny helper keeps the main bootstrap body shorter and the function itself simpler:

```js
const createLegacyEventListener = async (eventName, handler) => {
  if (typeof transformCallback !== 'function') {
    throw new Error('transformCallback is unavailable');
  }

  const eventId = await invoke('plugin:event|listen', {
    event: eventName,
    target: { kind: 'Any' },
    handler: transformCallback(handler),
  });

  return async () => {
    try {
      window.__TAURI_EVENT_PLUGIN_INTERNALS__?.unregisterListener?.(eventName, eventId);
    } catch {}
    await invoke('plugin:event|unlisten', { event: eventName, eventId });
  };
};

const createEventListener = async (eventName, handler) => {
  if (typeof tauriEvent?.listen === 'function') {
    return tauriEvent.listen(eventName, handler);
  }
  return createLegacyEventListener(eventName, handler);
};
```

This preserves all compatibility behavior but reduces the visual and cognitive load inside the bootstrap script.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src-tauri/src/main.rs Outdated
@zouyonghe
Copy link
Copy Markdown
Member Author

@sourcery-ai review

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've found 2 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `scripts/prepare-resources.mjs:288-299` </location>
<code_context>
+    const mustPass = expectation.required || isDesktopBridgeExpectationStrict;
+    const file = path.join(dashboardDir, ...expectation.filePath);
+    if (!existsSync(file)) {
+      const message = `[prepare-resources] Missing required file for ${expectation.label}: ${path.relative(projectRoot, file)}`;
+      if (mustPass) {
+        issues.push(message);
+      } else {
+        console.warn(`${message} (compatibility check skipped)`);
+      }
+      continue;
</code_context>

<issue_to_address>
**suggestion:** 澄清对于非必需的桌面桥接(desktop bridge)期望在文件缺失时的日志信息。

对于非必需的期望,目前日志仍然显示 `Missing required file for ...`,这会暗示这是一个强制要求,即使实际上该文件是可选的并且检查已被跳过。请在 `!mustPass` 的情况下调整日志信息(例如说明这是可选/尽力而为的),以避免在缺失是预期行为时,日志仍然暗示存在错误配置。

```suggestion
  for (const expectation of DESKTOP_BRIDGE_EXPECTATIONS) {
    const mustPass = expectation.required || isDesktopBridgeExpectationStrict;
    const file = path.join(dashboardDir, ...expectation.filePath);
    if (!existsSync(file)) {
      const relativePath = path.relative(projectRoot, file);
      const message = mustPass
        ? `[prepare-resources] Missing required file for ${expectation.label}: ${relativePath}`
        : `[prepare-resources] Missing optional (best-effort) file for ${expectation.label}: ${relativePath}`;
      if (mustPass) {
        issues.push(message);
      } else {
        console.warn(`${message} (compatibility check skipped)`);
      }
      continue;
    }
```
</issue_to_address>

### Comment 2
<location> `src-tauri/src/main.rs:1630` </location>
<code_context>
 }

-const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: &str = r#"
+const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE: &str = r#"
 (() => {
+  const existingTrayRestartState = window.__astrbotDesktopTrayRestartState;
</code_context>

<issue_to_address>
**issue (complexity):** 在保持行为不变的前提下,考虑简化 Rust 引导脚本生成逻辑以及 JS 中托盘重启状态的处理逻辑,去除不必要的占位符、断言和状态归一化分支。

你可以在两个重点部分减少复杂度而不改变行为:Rust 端脚本模板化逻辑,以及 JS 端 `trayRestartState` 的归一化逻辑。

### 1. 简化 Rust 引导脚本模板

目前的占位符常量 + `assert!` + `replace` 可以用一个带命名占位符的 `format!` 直接替换。这样依然能在一个 Rust 常量和一个 JS 位置中维护事件名的“单一可信来源”,同时去掉额外的模板机制。

**Template:**

```rust
const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE: &str = r#"
(() => {
  // ...
  const TRAY_RESTART_BACKEND_EVENT = "{TRAY_RESTART_BACKEND_EVENT}";
  // ...
})();
"#;
```

**Builder:**

```rust
static DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: OnceLock<String> = OnceLock::new();

fn desktop_bridge_bootstrap_script() -> &'static str {
    DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT
        .get_or_init(|| {
            format!(
                DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE,
                TRAY_RESTART_BACKEND_EVENT = TRAY_RESTART_BACKEND_EVENT,
            )
        })
        .as_str()
}
```

这样可以移除 `DESKTOP_BRIDGE_TRAY_EVENT_PLACEHOLDER``assert!`,同时保留事件名的“单一可信来源”。

### 2. 简化 JS 中 trayRestartState 的归一化逻辑

你在创建对象时已经完整初始化了 `trayRestartState`,因此可以去掉这些防御性的“归一化”分支,而是依赖可预测的对象结构。

当前写法:

```js
const trayRestartState =
  window.__astrbotDesktopTrayRestartState ||
  (window.__astrbotDesktopTrayRestartState = {
    handlers: new Set(),
    pending: 0,
    lastToken: 0,
    unlistenTrayRestartBackendEvent: null
  });

if (
  typeof trayRestartState.lastToken !== 'number' ||
  !Number.isFinite(trayRestartState.lastToken)
) {
  trayRestartState.lastToken = 0;
}
if (typeof trayRestartState.unlistenTrayRestartBackendEvent === 'undefined') {
  trayRestartState.unlistenTrayRestartBackendEvent = null;
}
```

可以简化为:

```js
const trayRestartState =
  window.__astrbotDesktopTrayRestartState ||
  (window.__astrbotDesktopTrayRestartState = {
    handlers: new Set(),
    pending: 0,
    lastToken: 0,
    unlistenTrayRestartBackendEvent: null
  });
```

然后消费端逻辑只需要一个简单的 truthy 检查:

```js
const listenToTrayRestartBackendEvent = async () => {
  if (trayRestartState.unlistenTrayRestartBackendEvent) return;

  try {
    const unlisten = await createEventListener(
      TRAY_RESTART_BACKEND_EVENT,
      (event) => {
        emitTrayRestart(event?.payload);
      }
    );
    trayRestartState.unlistenTrayRestartBackendEvent = unlisten || null;
  } catch (error) {
    console.warn('Failed to listen for tray restart backend event', error);
  }
};
```

这样可以保持 token 语义、pending 行为以及 unlisten 逻辑不变,同时删减掉大多数读者不需要关心的分支和状态“归一化”逻辑。
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈改进后续的代码评审。
Original comment in English

Hey - I've found 2 issues

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `scripts/prepare-resources.mjs:288-299` </location>
<code_context>
+    const mustPass = expectation.required || isDesktopBridgeExpectationStrict;
+    const file = path.join(dashboardDir, ...expectation.filePath);
+    if (!existsSync(file)) {
+      const message = `[prepare-resources] Missing required file for ${expectation.label}: ${path.relative(projectRoot, file)}`;
+      if (mustPass) {
+        issues.push(message);
+      } else {
+        console.warn(`${message} (compatibility check skipped)`);
+      }
+      continue;
</code_context>

<issue_to_address>
**suggestion:** Clarify messaging for non-required desktop bridge expectations whose files are missing.

For non-required expectations, the log still says `Missing required file for ...`, which implies a hard requirement even though the file is optional and the check is skipped. Please adjust the message when `!mustPass` (e.g., call out that it’s optional/best-effort) so the logs don’t suggest a misconfiguration when the absence is expected.

```suggestion
  for (const expectation of DESKTOP_BRIDGE_EXPECTATIONS) {
    const mustPass = expectation.required || isDesktopBridgeExpectationStrict;
    const file = path.join(dashboardDir, ...expectation.filePath);
    if (!existsSync(file)) {
      const relativePath = path.relative(projectRoot, file);
      const message = mustPass
        ? `[prepare-resources] Missing required file for ${expectation.label}: ${relativePath}`
        : `[prepare-resources] Missing optional (best-effort) file for ${expectation.label}: ${relativePath}`;
      if (mustPass) {
        issues.push(message);
      } else {
        console.warn(`${message} (compatibility check skipped)`);
      }
      continue;
    }
```
</issue_to_address>

### Comment 2
<location> `src-tauri/src/main.rs:1630` </location>
<code_context>
 }

-const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: &str = r#"
+const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE: &str = r#"
 (() => {
+  const existingTrayRestartState = window.__astrbotDesktopTrayRestartState;
</code_context>

<issue_to_address>
**issue (complexity):** Consider simplifying both the Rust bootstrap script generation and the JS tray restart state handling to remove unnecessary placeholders, assertions, and normalization branches while preserving behavior.

You can reduce complexity in two focused spots without changing behavior: the Rust script templating and the JS trayRestartState normalization.

### 1. Simplify Rust bootstrap script templating

The placeholder constant + `assert!` + `replace` can be replaced by a straightforward `format!` with a named placeholder in the template. This keeps the event name in one Rust constant and one JS location, but drops the extra machinery.

**Template:**

```rust
const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE: &str = r#"
(() => {
  // ...
  const TRAY_RESTART_BACKEND_EVENT = "{TRAY_RESTART_BACKEND_EVENT}";
  // ...
})();
"#;
```

**Builder:**

```rust
static DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: OnceLock<String> = OnceLock::new();

fn desktop_bridge_bootstrap_script() -> &'static str {
    DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT
        .get_or_init(|| {
            format!(
                DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE,
                TRAY_RESTART_BACKEND_EVENT = TRAY_RESTART_BACKEND_EVENT,
            )
        })
        .as_str()
}
```

This removes `DESKTOP_BRIDGE_TRAY_EVENT_PLACEHOLDER` and the `assert!` while preserving the “single source of truth” for the event name.

### 2. Simplify JS trayRestartState normalization

You already fully initialize `trayRestartState` when the object is created, so you can drop the defensive “normalization” branches and rely on a predictable shape.

Current shape:

```js
const trayRestartState =
  window.__astrbotDesktopTrayRestartState ||
  (window.__astrbotDesktopTrayRestartState = {
    handlers: new Set(),
    pending: 0,
    lastToken: 0,
    unlistenTrayRestartBackendEvent: null
  });

if (
  typeof trayRestartState.lastToken !== 'number' ||
  !Number.isFinite(trayRestartState.lastToken)
) {
  trayRestartState.lastToken = 0;
}
if (typeof trayRestartState.unlistenTrayRestartBackendEvent === 'undefined') {
  trayRestartState.unlistenTrayRestartBackendEvent = null;
}
```

Can be simplified to:

```js
const trayRestartState =
  window.__astrbotDesktopTrayRestartState ||
  (window.__astrbotDesktopTrayRestartState = {
    handlers: new Set(),
    pending: 0,
    lastToken: 0,
    unlistenTrayRestartBackendEvent: null
  });
```

And the consumer logic becomes a simple truthy check:

```js
const listenToTrayRestartBackendEvent = async () => {
  if (trayRestartState.unlistenTrayRestartBackendEvent) return;

  try {
    const unlisten = await createEventListener(
      TRAY_RESTART_BACKEND_EVENT,
      (event) => {
        emitTrayRestart(event?.payload);
      }
    );
    trayRestartState.unlistenTrayRestartBackendEvent = unlisten || null;
  } catch (error) {
    console.warn('Failed to listen for tray restart backend event', error);
  }
};
```

This keeps the token semantics, pending behavior, and unlisten logic intact, but trims branches and state “normalization” that most readers don’t need to think about.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread scripts/prepare-resources.mjs
Comment thread src-tauri/src/main.rs Outdated
@zouyonghe
Copy link
Copy Markdown
Member Author

@sourcery-ai review

Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - 我发现了 1 个问题

给 AI 代理的提示
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `src-tauri/src/main.rs:1612` </location>
<code_context>
     set_menu_text_safe(&tray_state.quit_item, shell_texts.tray_quit, TRAY_MENU_QUIT);
 }

-const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: &str = r#"
+const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE: &str = r#"
 (() => {
</code_context>

<issue_to_address>
**issue (complexity):** 可以考虑通过在编译期内联事件名,并将 JS token 的记账逻辑提取到一些小的辅助函数中,来简化 desktop bridge 的改动,从而减少间接层和表面复杂度。

你可以在完全保持当前行为不变的前提下,裁剪掉大部分间接层。

### 1. 去掉 bootstrap 脚本的模板和 `OnceLock`

由于 `TRAY_RESTART_BACKEND_EVENT` 是一个 `const &str`,你可以在编译期通过 `concat!` 把它注入 JS,从而避免:

- `DESKTOP_BRIDGE_TRAY_EVENT_PLACEHOLDER`
- `DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE`
- `OnceLock<String>`
- `desktop_bridge_bootstrap_script()` 这一层间接调用

重构示例:

```rust
const TRAY_RESTART_BACKEND_EVENT: &str = "astrbot://tray-restart-backend";

const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: &str = concat!(
    r#"(() => {
  const existingTrayRestartState = window.__astrbotDesktopTrayRestartState;
  if (
    window.astrbotDesktop &&
    window.astrbotDesktop.__tauriBridge === true &&
    typeof window.astrbotDesktop.onTrayRestartBackend === 'function' &&
    typeof existingTrayRestartState?.unlistenTrayRestartBackendEvent === 'function'
  ) {
    return;
  }

  const invoke = window.__TAURI_INTERNALS__?.invoke;
  const transformCallback = window.__TAURI_INTERNALS__?.transformCallback;
  const tauriEvent = window.__TAURI_INTERNALS__?.event ?? window.__TAURI__?.event;
  if (typeof invoke !== 'function') return;

  const BRIDGE_COMMANDS = Object.freeze({
    IS_DESKTOP_RUNTIME: 'desktop_bridge_is_desktop_runtime',
    GET_BACKEND_STATE: 'desktop_bridge_get_backend_state',
    SET_AUTH_TOKEN: 'desktop_bridge_set_auth_token',
    RESTART_BACKEND: 'desktop_bridge_restart_backend',
    STOP_BACKEND: 'desktop_bridge_stop_backend',
  });
  const TRAY_RESTART_BACKEND_EVENT = '"#,
    TRAY_RESTART_BACKEND_EVENT,
    r#"';
  // ... rest of script unchanged ...
})();"#,
);
```

这样 `inject_desktop_bridge` 可以保持简单:

```rust
fn inject_desktop_bridge(webview: &tauri::Webview<tauri::Wry>) {
    if let Err(error) = webview.eval(DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT) {
        append_desktop_log(&format!("failed to inject desktop bridge script: {error}"));
    }
}
```

这既保留了事件名的单一真源(single source of truth),又去掉了运行时字符串替换、`OnceLock` 和占位符断言。

### 2. 封装 JS token 逻辑以降低表面复杂度

你可以保持基于 token 的去重以及 `unlisten` 处理的行为完全不变,但把记账逻辑移动到一些小的辅助函数里,这样主流程会更易读。

例如:

```js
const ensureTrayRestartState = () => {
  const state =
    window.__astrbotDesktopTrayRestartState ||
    (window.__astrbotDesktopTrayRestartState = {
      handlers: new Set(),
      pending: 0,
      lastToken: 0,
      unlistenTrayRestartBackendEvent: null
    });

  if (typeof state.lastToken !== 'number' || !Number.isFinite(state.lastToken)) {
    state.lastToken = 0;
  }
  if (typeof state.unlistenTrayRestartBackendEvent === 'undefined') {
    state.unlistenTrayRestartBackendEvent = null;
  }
  return state;
};

const shouldEmitForToken = (state, token) => {
  const numericToken = Number(token);
  if (Number.isFinite(numericToken) && numericToken > 0) {
    if (numericToken <= state.lastToken) return false;
    state.lastToken = numericToken;
  } else {
    state.lastToken += 1;
  }
  return true;
};

const trayRestartState = ensureTrayRestartState();

const emitTrayRestart = (token = null) => {
  if (!shouldEmitForToken(trayRestartState, token)) return;

  if (trayRestartState.handlers.size === 0) {
    trayRestartState.pending = Number(trayRestartState.pending || 0) + 1;
    return;
  }
  for (const handler of trayRestartState.handlers) {
    try {
      handler();
    } catch {}
  }
};
```

你剩余的逻辑(`listenToTrayRestartBackendEvent``onTrayRestartBackend`)可以在结构上保持不变,但现在阅读起来会更线性,因为 token/去重的细节被隐藏在 `ensureTrayRestartState``shouldEmitForToken` 之中。
</issue_to_address>

Sourcery 对开源项目是免费的——如果你觉得我们的评审有帮助,欢迎分享 ✨
帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续评审。
Original comment in English

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location> `src-tauri/src/main.rs:1612` </location>
<code_context>
     set_menu_text_safe(&tray_state.quit_item, shell_texts.tray_quit, TRAY_MENU_QUIT);
 }

-const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: &str = r#"
+const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE: &str = r#"
 (() => {
</code_context>

<issue_to_address>
**issue (complexity):** Consider simplifying the desktop bridge changes by inlining the event name at compile time and extracting the JS token bookkeeping into small helpers to reduce indirection and visible complexity.

You can keep the new behavior exactly as-is and still trim most of the indirection.

### 1. Drop the template + `OnceLock` for the bootstrap script

Because `TRAY_RESTART_BACKEND_EVENT` is a `const &str`, you can wire it into the JS at compile time with `concat!` and avoid:

- `DESKTOP_BRIDGE_TRAY_EVENT_PLACEHOLDER`
- `DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE`
- `OnceLock<String>`
- `desktop_bridge_bootstrap_script()` indirection

Example refactor:

```rust
const TRAY_RESTART_BACKEND_EVENT: &str = "astrbot://tray-restart-backend";

const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: &str = concat!(
    r#"(() => {
  const existingTrayRestartState = window.__astrbotDesktopTrayRestartState;
  if (
    window.astrbotDesktop &&
    window.astrbotDesktop.__tauriBridge === true &&
    typeof window.astrbotDesktop.onTrayRestartBackend === 'function' &&
    typeof existingTrayRestartState?.unlistenTrayRestartBackendEvent === 'function'
  ) {
    return;
  }

  const invoke = window.__TAURI_INTERNALS__?.invoke;
  const transformCallback = window.__TAURI_INTERNALS__?.transformCallback;
  const tauriEvent = window.__TAURI_INTERNALS__?.event ?? window.__TAURI__?.event;
  if (typeof invoke !== 'function') return;

  const BRIDGE_COMMANDS = Object.freeze({
    IS_DESKTOP_RUNTIME: 'desktop_bridge_is_desktop_runtime',
    GET_BACKEND_STATE: 'desktop_bridge_get_backend_state',
    SET_AUTH_TOKEN: 'desktop_bridge_set_auth_token',
    RESTART_BACKEND: 'desktop_bridge_restart_backend',
    STOP_BACKEND: 'desktop_bridge_stop_backend',
  });
  const TRAY_RESTART_BACKEND_EVENT = '"#,
    TRAY_RESTART_BACKEND_EVENT,
    r#"';
  // ... rest of script unchanged ...
})();"#,
);
```

Then `inject_desktop_bridge` stays simple:

```rust
fn inject_desktop_bridge(webview: &tauri::Webview<tauri::Wry>) {
    if let Err(error) = webview.eval(DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT) {
        append_desktop_log(&format!("failed to inject desktop bridge script: {error}"));
    }
}
```

This keeps the single source of truth for the event name, but removes the runtime string replacement, `OnceLock`, and placeholder assertion.

### 2. Encapsulate JS token logic to reduce visible complexity

You can keep token-based deduplication and the `unlisten` handling exactly the same, but move the bookkeeping into small helpers so the main flow is easier to read.

For example:

```js
const ensureTrayRestartState = () => {
  const state =
    window.__astrbotDesktopTrayRestartState ||
    (window.__astrbotDesktopTrayRestartState = {
      handlers: new Set(),
      pending: 0,
      lastToken: 0,
      unlistenTrayRestartBackendEvent: null
    });

  if (typeof state.lastToken !== 'number' || !Number.isFinite(state.lastToken)) {
    state.lastToken = 0;
  }
  if (typeof state.unlistenTrayRestartBackendEvent === 'undefined') {
    state.unlistenTrayRestartBackendEvent = null;
  }
  return state;
};

const shouldEmitForToken = (state, token) => {
  const numericToken = Number(token);
  if (Number.isFinite(numericToken) && numericToken > 0) {
    if (numericToken <= state.lastToken) return false;
    state.lastToken = numericToken;
  } else {
    state.lastToken += 1;
  }
  return true;
};

const trayRestartState = ensureTrayRestartState();

const emitTrayRestart = (token = null) => {
  if (!shouldEmitForToken(trayRestartState, token)) return;

  if (trayRestartState.handlers.size === 0) {
    trayRestartState.pending = Number(trayRestartState.pending || 0) + 1;
    return;
  }
  for (const handler of trayRestartState.handlers) {
    try {
      handler();
    } catch {}
  }
};
```

The rest of your logic (`listenToTrayRestartBackendEvent`, `onTrayRestartBackend`) can stay structurally the same but now reads more linearly, with the token/dedup details hidden behind `ensureTrayRestartState` and `shouldEmitForToken`.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src-tauri/src/main.rs
set_menu_text_safe(&tray_state.quit_item, shell_texts.tray_quit, TRAY_MENU_QUIT);
}

const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: &str = r#"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (complexity): 可以考虑通过在编译期内联事件名,并将 JS token 的记账逻辑提取到一些小的辅助函数中,来简化 desktop bridge 的改动,从而减少间接层和表面复杂度。

你可以在完全保持当前行为不变的前提下,裁剪掉大部分间接层。

1. 去掉 bootstrap 脚本的模板和 OnceLock

由于 TRAY_RESTART_BACKEND_EVENT 是一个 const &str,你可以在编译期通过 concat! 把它注入 JS,从而避免:

  • DESKTOP_BRIDGE_TRAY_EVENT_PLACEHOLDER
  • DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE
  • OnceLock<String>
  • desktop_bridge_bootstrap_script() 这一层间接调用

重构示例:

const TRAY_RESTART_BACKEND_EVENT: &str = "astrbot://tray-restart-backend";

const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: &str = concat!(
    r#"(() => {
  const existingTrayRestartState = window.__astrbotDesktopTrayRestartState;
  if (
    window.astrbotDesktop &&
    window.astrbotDesktop.__tauriBridge === true &&
    typeof window.astrbotDesktop.onTrayRestartBackend === 'function' &&
    typeof existingTrayRestartState?.unlistenTrayRestartBackendEvent === 'function'
  ) {
    return;
  }

  const invoke = window.__TAURI_INTERNALS__?.invoke;
  const transformCallback = window.__TAURI_INTERNALS__?.transformCallback;
  const tauriEvent = window.__TAURI_INTERNALS__?.event ?? window.__TAURI__?.event;
  if (typeof invoke !== 'function') return;

  const BRIDGE_COMMANDS = Object.freeze({
    IS_DESKTOP_RUNTIME: 'desktop_bridge_is_desktop_runtime',
    GET_BACKEND_STATE: 'desktop_bridge_get_backend_state',
    SET_AUTH_TOKEN: 'desktop_bridge_set_auth_token',
    RESTART_BACKEND: 'desktop_bridge_restart_backend',
    STOP_BACKEND: 'desktop_bridge_stop_backend',
  });
  const TRAY_RESTART_BACKEND_EVENT = '"#,
    TRAY_RESTART_BACKEND_EVENT,
    r#"';
  // ... rest of script unchanged ...
})();"#,
);

这样 inject_desktop_bridge 可以保持简单:

fn inject_desktop_bridge(webview: &tauri::Webview<tauri::Wry>) {
    if let Err(error) = webview.eval(DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT) {
        append_desktop_log(&format!("failed to inject desktop bridge script: {error}"));
    }
}

这既保留了事件名的单一真源(single source of truth),又去掉了运行时字符串替换、OnceLock 和占位符断言。

2. 封装 JS token 逻辑以降低表面复杂度

你可以保持基于 token 的去重以及 unlisten 处理的行为完全不变,但把记账逻辑移动到一些小的辅助函数里,这样主流程会更易读。

例如:

const ensureTrayRestartState = () => {
  const state =
    window.__astrbotDesktopTrayRestartState ||
    (window.__astrbotDesktopTrayRestartState = {
      handlers: new Set(),
      pending: 0,
      lastToken: 0,
      unlistenTrayRestartBackendEvent: null
    });

  if (typeof state.lastToken !== 'number' || !Number.isFinite(state.lastToken)) {
    state.lastToken = 0;
  }
  if (typeof state.unlistenTrayRestartBackendEvent === 'undefined') {
    state.unlistenTrayRestartBackendEvent = null;
  }
  return state;
};

const shouldEmitForToken = (state, token) => {
  const numericToken = Number(token);
  if (Number.isFinite(numericToken) && numericToken > 0) {
    if (numericToken <= state.lastToken) return false;
    state.lastToken = numericToken;
  } else {
    state.lastToken += 1;
  }
  return true;
};

const trayRestartState = ensureTrayRestartState();

const emitTrayRestart = (token = null) => {
  if (!shouldEmitForToken(trayRestartState, token)) return;

  if (trayRestartState.handlers.size === 0) {
    trayRestartState.pending = Number(trayRestartState.pending || 0) + 1;
    return;
  }
  for (const handler of trayRestartState.handlers) {
    try {
      handler();
    } catch {}
  }
};

你剩余的逻辑(listenToTrayRestartBackendEventonTrayRestartBackend)可以在结构上保持不变,但现在阅读起来会更线性,因为 token/去重的细节被隐藏在 ensureTrayRestartStateshouldEmitForToken 之中。

Original comment in English

issue (complexity): Consider simplifying the desktop bridge changes by inlining the event name at compile time and extracting the JS token bookkeeping into small helpers to reduce indirection and visible complexity.

You can keep the new behavior exactly as-is and still trim most of the indirection.

1. Drop the template + OnceLock for the bootstrap script

Because TRAY_RESTART_BACKEND_EVENT is a const &str, you can wire it into the JS at compile time with concat! and avoid:

  • DESKTOP_BRIDGE_TRAY_EVENT_PLACEHOLDER
  • DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT_TEMPLATE
  • OnceLock<String>
  • desktop_bridge_bootstrap_script() indirection

Example refactor:

const TRAY_RESTART_BACKEND_EVENT: &str = "astrbot://tray-restart-backend";

const DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT: &str = concat!(
    r#"(() => {
  const existingTrayRestartState = window.__astrbotDesktopTrayRestartState;
  if (
    window.astrbotDesktop &&
    window.astrbotDesktop.__tauriBridge === true &&
    typeof window.astrbotDesktop.onTrayRestartBackend === 'function' &&
    typeof existingTrayRestartState?.unlistenTrayRestartBackendEvent === 'function'
  ) {
    return;
  }

  const invoke = window.__TAURI_INTERNALS__?.invoke;
  const transformCallback = window.__TAURI_INTERNALS__?.transformCallback;
  const tauriEvent = window.__TAURI_INTERNALS__?.event ?? window.__TAURI__?.event;
  if (typeof invoke !== 'function') return;

  const BRIDGE_COMMANDS = Object.freeze({
    IS_DESKTOP_RUNTIME: 'desktop_bridge_is_desktop_runtime',
    GET_BACKEND_STATE: 'desktop_bridge_get_backend_state',
    SET_AUTH_TOKEN: 'desktop_bridge_set_auth_token',
    RESTART_BACKEND: 'desktop_bridge_restart_backend',
    STOP_BACKEND: 'desktop_bridge_stop_backend',
  });
  const TRAY_RESTART_BACKEND_EVENT = '"#,
    TRAY_RESTART_BACKEND_EVENT,
    r#"';
  // ... rest of script unchanged ...
})();"#,
);

Then inject_desktop_bridge stays simple:

fn inject_desktop_bridge(webview: &tauri::Webview<tauri::Wry>) {
    if let Err(error) = webview.eval(DESKTOP_BRIDGE_BOOTSTRAP_SCRIPT) {
        append_desktop_log(&format!("failed to inject desktop bridge script: {error}"));
    }
}

This keeps the single source of truth for the event name, but removes the runtime string replacement, OnceLock, and placeholder assertion.

2. Encapsulate JS token logic to reduce visible complexity

You can keep token-based deduplication and the unlisten handling exactly the same, but move the bookkeeping into small helpers so the main flow is easier to read.

For example:

const ensureTrayRestartState = () => {
  const state =
    window.__astrbotDesktopTrayRestartState ||
    (window.__astrbotDesktopTrayRestartState = {
      handlers: new Set(),
      pending: 0,
      lastToken: 0,
      unlistenTrayRestartBackendEvent: null
    });

  if (typeof state.lastToken !== 'number' || !Number.isFinite(state.lastToken)) {
    state.lastToken = 0;
  }
  if (typeof state.unlistenTrayRestartBackendEvent === 'undefined') {
    state.unlistenTrayRestartBackendEvent = null;
  }
  return state;
};

const shouldEmitForToken = (state, token) => {
  const numericToken = Number(token);
  if (Number.isFinite(numericToken) && numericToken > 0) {
    if (numericToken <= state.lastToken) return false;
    state.lastToken = numericToken;
  } else {
    state.lastToken += 1;
  }
  return true;
};

const trayRestartState = ensureTrayRestartState();

const emitTrayRestart = (token = null) => {
  if (!shouldEmitForToken(trayRestartState, token)) return;

  if (trayRestartState.handlers.size === 0) {
    trayRestartState.pending = Number(trayRestartState.pending || 0) + 1;
    return;
  }
  for (const handler of trayRestartState.handlers) {
    try {
      handler();
    } catch {}
  }
};

The rest of your logic (listenToTrayRestartBackendEvent, onTrayRestartBackend) can stay structurally the same but now reads more linearly, with the token/dedup details hidden behind ensureTrayRestartState and shouldEmitForToken.

@zouyonghe zouyonghe changed the title fix(desktop): stabilize tray restart UX and CI/build resource flow fix(desktop): stabilize tray restart signaling and harden build/resource flow Feb 20, 2026
@zouyonghe zouyonghe merged commit 4487d40 into AstrBotDevs:main Feb 20, 2026
4 checks passed
@zouyonghe zouyonghe deleted the pr/desktop-runtime-ci-20260220 branch February 21, 2026 07:52
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