refactor: 桌面端迁移 CPython 运行时并强化 Windows 打包稳定性#5170
Closed
zouyonghe wants to merge 50 commits intoAstrBotDevs:masterfrom
Closed
refactor: 桌面端迁移 CPython 运行时并强化 Windows 打包稳定性#5170zouyonghe wants to merge 50 commits intoAstrBotDevs:masterfrom
zouyonghe wants to merge 50 commits intoAstrBotDevs:masterfrom
Conversation
dosubot
bot
added
the
size:XL
|
Related Documentation Checked 1 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
嗨——我已经查看了你的更改，看起来非常棒！
帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
dosubot
bot
added
the
area:core
zouyonghe
requested review from
Dt8333,
RC-CHN,
Raven95676,
Soulter and
advent259141
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - 我发现了两个问题，并给出了一些高层次的反馈：
- 在
desktop/scripts/build-backend.mjs中，建议校验所选的 CPython 运行时是否与预期的 Python 主/次版本号匹配，并且至少能够执行一个简单命令（例如python -c 'import pip'），这样配置错误或不兼容的运行时可以在构建阶段快速失败，而不是等到应用运行时才暴露问题。 - Windows 上的后端进程识别现在依赖通过 PowerShell（
getWindowsProcessCommandLine）解析命令行；可以考虑约束调用的时机/频率，或者按 PID 缓存结果，以避免在存在大量 PID 的关机阶段重复调用 PowerShell。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- In `desktop/scripts/build-backend.mjs`, consider validating that the selected CPython runtime matches the expected Python major/minor version and can at least execute a trivial command (e.g., `python -c 'import pip'`) so misconfigured or incompatible runtimes fail fast at build time instead of only at app runtime.
- The backend process identification on Windows now relies on parsing the command line via PowerShell (`getWindowsProcessCommandLine`); it may be worth constraining when/how often this is called or caching results per PID to avoid repeated PowerShell invocations during shutdown when many PIDs are present.
## Individual Comments
### Comment 1
<location> `desktop/README.md:156` </location>
<code_context>
+
+- `Missing CPython runtime source`: set `ASTRBOT_DESKTOP_CPYTHON_HOME` (or `ASTRBOT_DESKTOP_BACKEND_RUNTIME`).
+- `Cannot find Python executable in runtime`: runtime directory is invalid or incomplete.
+- `Failed to detect purelib from runtime python`: runtime Python cannot run correctly.
+
If Electron download times out on restricted networks, configure mirrors before install:
</code_context>
<issue_to_address>
**nitpick (typo):** Capitalize "Python" for consistency and correctness.
Here, "runtime python" should be "runtime Python" to match the earlier "Python executable" and the later "runtime Python cannot run correctly."
```suggestion
- `Failed to detect purelib from runtime Python`: runtime Python cannot run correctly.
```
</issue_to_address>
### Comment 2
<location> `desktop/lib/backend-manager.js:175` </location>
<code_context>
+ return this.packagedBackendManifest;
+ }
+
+ getPackagedBackendAppDir() {
+ const backendDir = this.getPackagedBackendDir();
+ if (!backendDir) {
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the new manifest-based helpers and Windows PID matching logic into shared utilities to centralize behavior and reduce duplication.
You can reduce the added complexity in two focused places without changing behavior: manifest-driven path resolution and Windows PID matching.
---
### 1. Consolidate manifest-based path helpers
`getPackagedBackendAppDir`, `getPackagedBackendLaunchScriptPath`, and `getPackagedRuntimePythonPath` all:
- recompute `backendDir`
- call `getPackagedBackendManifest`
- compute a relative path with a default
- join + `existsSync`
This can be centralized so you only resolve the manifest once and reduce duplication.
**Example refactor (internal helper + cohesive config):**
```js
// Small internal helper
resolveManifestPath(fieldName, defaultValue) {
const backendDir = this.getPackagedBackendDir();
if (!backendDir) return null;
const manifest = this.getPackagedBackendManifest() || {};
const rel = typeof manifest[fieldName] === 'string' && manifest[fieldName]
? manifest[fieldName]
: defaultValue;
const candidate = path.join(backendDir, rel);
return fs.existsSync(candidate) ? candidate : null;
}
// Single cohesive config getter
getPackagedRuntimeConfig() {
if (!this.app.isPackaged) return null;
const backendDir = this.getPackagedBackendDir();
if (!backendDir) return null;
const pythonRelativeDefault =
process.platform === 'win32'
? path.join('python', 'Scripts', 'python.exe')
: path.join('python', 'bin', 'python3');
const appDir = this.resolveManifestPath('app', 'app');
const entry = this.resolveManifestPath('entrypoint', 'launch_backend.py');
const python = this.resolveManifestPath('python', pythonRelativeDefault);
if (!python || !entry) return null;
return { backendDir, appDir, entry, python };
}
```
Then the existing helpers become thin wrappers (or can be inlined where used):
```js
getPackagedBackendAppDir() {
const cfg = this.getPackagedRuntimeConfig();
return cfg ? cfg.appDir : null;
}
getPackagedBackendLaunchScriptPath() {
const cfg = this.getPackagedRuntimeConfig();
return cfg ? cfg.entry : null;
}
getPackagedRuntimePythonPath() {
const cfg = this.getPackagedRuntimeConfig();
return cfg ? cfg.python : null;
}
buildPackagedBackendLaunch(webuiDir) {
const cfg = this.getPackagedRuntimeConfig();
if (!cfg) return null;
const args = [cfg.entry];
if (webuiDir) args.push('--webui-dir', webuiDir);
return { cmd: cfg.python, args, shell: false };
}
```
This keeps all existing behavior intact but:
- removes repeated `backendDir` / manifest reads
- gives a single “runtime config” mental model
- makes future fields easier to add.
---
### 2. Encapsulate Windows PID matching logic
`stopUnmanagedBackendByPort` is handling:
- backendConfig extraction
- image-name matching
- deciding if strict command line check is required
- resolving/normalizing markers
- per-PID log decisions and `taskkill`
You already added `normalizeWindowsPathForMatch`, `isGenericWindowsPythonImage`, and `getWindowsProcessCommandLine`; you can push the branching into a single predicate like “does this PID look like our backend?” to linearize `stopUnmanagedBackendByPort`.
**Example refactor:**
```js
// Internal helper: decide whether a PID is our backend
isMatchingBackendPid(pid, backendConfig, expectedImageName, markers, requireStrict) {
const processInfo = this.getWindowsProcessInfo(pid);
if (!processInfo) {
this.log(`Skip unmanaged cleanup for pid=${pid}: unable to resolve process info.`);
return false;
}
const actualImageName = processInfo.imageName.toLowerCase();
if (actualImageName !== expectedImageName) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unexpected process image ${processInfo.imageName}.`,
);
return false;
}
if (!requireStrict) return true;
if (!markers.length) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: backend launch marker is unavailable.`,
);
return false;
}
const commandLine = this.getWindowsProcessCommandLine(pid);
if (!commandLine) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process command line.`,
);
return false;
}
const normalizedCommandLine = this.normalizeWindowsPathForMatch(commandLine);
const markerMatched = markers.some(
(marker) => marker && normalizedCommandLine.includes(marker),
);
if (!markerMatched) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: command line does not match AstrBot backend launch marker.`,
);
return false;
}
return true;
}
```
Then `stopUnmanagedBackendByPort` becomes mostly setup + a simple loop:
```js
async stopUnmanagedBackendByPort() {
if (!this.app.isPackaged || process.platform !== 'win32') return false;
const port = this.getBackendPort();
if (!port) return false;
const pids = this.findListeningPidsOnWindows(port);
if (!pids.length) return false;
this.log(
`Attempting unmanaged backend cleanup by port=${port} pids=${pids.join(',')}`,
);
const backendConfig = this.getBackendConfig();
const expectedImageName = path.basename(backendConfig.cmd || 'python.exe').toLowerCase();
const requireStrict = this.isGenericWindowsPythonImage(expectedImageName);
const markers = [];
if (Array.isArray(backendConfig.args) && backendConfig.args.length > 0) {
const primaryArg = backendConfig.args[0];
if (typeof primaryArg === 'string' && primaryArg) {
const resolvedPrimaryArg = path.isAbsolute(primaryArg)
? primaryArg
: path.resolve(backendConfig.cwd || process.cwd(), primaryArg);
markers.push(this.normalizeWindowsPathForMatch(resolvedPrimaryArg));
markers.push(this.normalizeWindowsPathForMatch(path.basename(primaryArg)));
}
}
for (const pid of pids) {
if (!this.isMatchingBackendPid(pid, backendConfig, expectedImageName, markers, requireStrict)) {
continue;
}
try {
spawnSync('taskkill', ['/pid', `${pid}`, '/t', '/f'], {
stdio: 'ignore',
windowsHide: true,
});
} catch {}
}
return true;
}
```
This preserves all the safety logic you’ve added, but:
- keeps the main method’s control flow linear
- keeps PID-matching responsibilities in one place
- makes it easier to reason about / test PID matching separately.
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- In
desktop/scripts/build-backend.mjs, consider validating that the selected CPython runtime matches the expected Python major/minor version and can at least execute a trivial command (e.g.,python -c 'import pip') so misconfigured or incompatible runtimes fail fast at build time instead of only at app runtime. - The backend process identification on Windows now relies on parsing the command line via PowerShell (
getWindowsProcessCommandLine); it may be worth constraining when/how often this is called or caching results per PID to avoid repeated PowerShell invocations during shutdown when many PIDs are present.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `desktop/scripts/build-backend.mjs`, consider validating that the selected CPython runtime matches the expected Python major/minor version and can at least execute a trivial command (e.g., `python -c 'import pip'`) so misconfigured or incompatible runtimes fail fast at build time instead of only at app runtime.
- The backend process identification on Windows now relies on parsing the command line via PowerShell (`getWindowsProcessCommandLine`); it may be worth constraining when/how often this is called or caching results per PID to avoid repeated PowerShell invocations during shutdown when many PIDs are present.
## Individual Comments
### Comment 1
<location> `desktop/README.md:156` </location>
<code_context>
+
+- `Missing CPython runtime source`: set `ASTRBOT_DESKTOP_CPYTHON_HOME` (or `ASTRBOT_DESKTOP_BACKEND_RUNTIME`).
+- `Cannot find Python executable in runtime`: runtime directory is invalid or incomplete.
+- `Failed to detect purelib from runtime python`: runtime Python cannot run correctly.
+
If Electron download times out on restricted networks, configure mirrors before install:
</code_context>
<issue_to_address>
**nitpick (typo):** Capitalize "Python" for consistency and correctness.
Here, "runtime python" should be "runtime Python" to match the earlier "Python executable" and the later "runtime Python cannot run correctly."
```suggestion
- `Failed to detect purelib from runtime Python`: runtime Python cannot run correctly.
```
</issue_to_address>
### Comment 2
<location> `desktop/lib/backend-manager.js:175` </location>
<code_context>
+ return this.packagedBackendManifest;
+ }
+
+ getPackagedBackendAppDir() {
+ const backendDir = this.getPackagedBackendDir();
+ if (!backendDir) {
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the new manifest-based helpers and Windows PID matching logic into shared utilities to centralize behavior and reduce duplication.
You can reduce the added complexity in two focused places without changing behavior: manifest-driven path resolution and Windows PID matching.
---
### 1. Consolidate manifest-based path helpers
`getPackagedBackendAppDir`, `getPackagedBackendLaunchScriptPath`, and `getPackagedRuntimePythonPath` all:
- recompute `backendDir`
- call `getPackagedBackendManifest`
- compute a relative path with a default
- join + `existsSync`
This can be centralized so you only resolve the manifest once and reduce duplication.
**Example refactor (internal helper + cohesive config):**
```js
// Small internal helper
resolveManifestPath(fieldName, defaultValue) {
const backendDir = this.getPackagedBackendDir();
if (!backendDir) return null;
const manifest = this.getPackagedBackendManifest() || {};
const rel = typeof manifest[fieldName] === 'string' && manifest[fieldName]
? manifest[fieldName]
: defaultValue;
const candidate = path.join(backendDir, rel);
return fs.existsSync(candidate) ? candidate : null;
}
// Single cohesive config getter
getPackagedRuntimeConfig() {
if (!this.app.isPackaged) return null;
const backendDir = this.getPackagedBackendDir();
if (!backendDir) return null;
const pythonRelativeDefault =
process.platform === 'win32'
? path.join('python', 'Scripts', 'python.exe')
: path.join('python', 'bin', 'python3');
const appDir = this.resolveManifestPath('app', 'app');
const entry = this.resolveManifestPath('entrypoint', 'launch_backend.py');
const python = this.resolveManifestPath('python', pythonRelativeDefault);
if (!python || !entry) return null;
return { backendDir, appDir, entry, python };
}
```
Then the existing helpers become thin wrappers (or can be inlined where used):
```js
getPackagedBackendAppDir() {
const cfg = this.getPackagedRuntimeConfig();
return cfg ? cfg.appDir : null;
}
getPackagedBackendLaunchScriptPath() {
const cfg = this.getPackagedRuntimeConfig();
return cfg ? cfg.entry : null;
}
getPackagedRuntimePythonPath() {
const cfg = this.getPackagedRuntimeConfig();
return cfg ? cfg.python : null;
}
buildPackagedBackendLaunch(webuiDir) {
const cfg = this.getPackagedRuntimeConfig();
if (!cfg) return null;
const args = [cfg.entry];
if (webuiDir) args.push('--webui-dir', webuiDir);
return { cmd: cfg.python, args, shell: false };
}
```
This keeps all existing behavior intact but:
- removes repeated `backendDir` / manifest reads
- gives a single “runtime config” mental model
- makes future fields easier to add.
---
### 2. Encapsulate Windows PID matching logic
`stopUnmanagedBackendByPort` is handling:
- backendConfig extraction
- image-name matching
- deciding if strict command line check is required
- resolving/normalizing markers
- per-PID log decisions and `taskkill`
You already added `normalizeWindowsPathForMatch`, `isGenericWindowsPythonImage`, and `getWindowsProcessCommandLine`; you can push the branching into a single predicate like “does this PID look like our backend?” to linearize `stopUnmanagedBackendByPort`.
**Example refactor:**
```js
// Internal helper: decide whether a PID is our backend
isMatchingBackendPid(pid, backendConfig, expectedImageName, markers, requireStrict) {
const processInfo = this.getWindowsProcessInfo(pid);
if (!processInfo) {
this.log(`Skip unmanaged cleanup for pid=${pid}: unable to resolve process info.`);
return false;
}
const actualImageName = processInfo.imageName.toLowerCase();
if (actualImageName !== expectedImageName) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unexpected process image ${processInfo.imageName}.`,
);
return false;
}
if (!requireStrict) return true;
if (!markers.length) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: backend launch marker is unavailable.`,
);
return false;
}
const commandLine = this.getWindowsProcessCommandLine(pid);
if (!commandLine) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process command line.`,
);
return false;
}
const normalizedCommandLine = this.normalizeWindowsPathForMatch(commandLine);
const markerMatched = markers.some(
(marker) => marker && normalizedCommandLine.includes(marker),
);
if (!markerMatched) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: command line does not match AstrBot backend launch marker.`,
);
return false;
}
return true;
}
```
Then `stopUnmanagedBackendByPort` becomes mostly setup + a simple loop:
```js
async stopUnmanagedBackendByPort() {
if (!this.app.isPackaged || process.platform !== 'win32') return false;
const port = this.getBackendPort();
if (!port) return false;
const pids = this.findListeningPidsOnWindows(port);
if (!pids.length) return false;
this.log(
`Attempting unmanaged backend cleanup by port=${port} pids=${pids.join(',')}`,
);
const backendConfig = this.getBackendConfig();
const expectedImageName = path.basename(backendConfig.cmd || 'python.exe').toLowerCase();
const requireStrict = this.isGenericWindowsPythonImage(expectedImageName);
const markers = [];
if (Array.isArray(backendConfig.args) && backendConfig.args.length > 0) {
const primaryArg = backendConfig.args[0];
if (typeof primaryArg === 'string' && primaryArg) {
const resolvedPrimaryArg = path.isAbsolute(primaryArg)
? primaryArg
: path.resolve(backendConfig.cwd || process.cwd(), primaryArg);
markers.push(this.normalizeWindowsPathForMatch(resolvedPrimaryArg));
markers.push(this.normalizeWindowsPathForMatch(path.basename(primaryArg)));
}
}
for (const pid of pids) {
if (!this.isMatchingBackendPid(pid, backendConfig, expectedImageName, markers, requireStrict)) {
continue;
}
try {
spawnSync('taskkill', ['/pid', `${pid}`, '/t', '/f'], {
stdio: 'ignore',
windowsHide: true,
});
} catch {}
}
return true;
}
```
This preserves all the safety logic you’ve added, but:
- keeps the main method’s control flow linear
- keeps PID-matching responsibilities in one place
- makes it easier to reason about / test PID matching separately.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey，我发现了 4 个问题，并给出了一些总体反馈：
- 在 Windows 的非托管 backend 清理逻辑中，
getWindowsProcessCommandLine会为每个 PID 调用一次同步的 PowerShell 命令，而且没有设置超时或回退机制；建议增加一个超时时间，并在 PowerShell 不可用或很慢的环境下提供一个优雅的回退方案，以避免在关闭应用时发生卡死。 - 在
desktop/scripts/build-backend.mjs中，即使源目录和目标目录有重叠，构建流程仍会递归地将 runtime 目录cpSync到desktop/resources/backend/python；可以增加一个保护条件，阻止ASTRBOT_DESKTOP_CPYTHON_HOME指向desktop/resources/backend内部，以避免构建产物被意外地复制到自身目录中。
AI 代理用提示词
Please address the comments from this code review:
## Overall Comments
- On Windows unmanaged-backend cleanup, `getWindowsProcessCommandLine` invokes a synchronous PowerShell command per PID without a timeout or fallback; consider adding a timeout and a graceful fallback for environments where PowerShell is unavailable or slow to avoid hangs during shutdown.
- In `desktop/scripts/build-backend.mjs`, the build will recursively `cpSync` the runtime directory into `desktop/resources/backend/python` even if the source and destination overlap; adding a guard to prevent `ASTRBOT_DESKTOP_CPYTHON_HOME` pointing inside `desktop/resources/backend` would avoid accidentally copying build artifacts into themselves.
## Individual Comments
### Comment 1
<location> `desktop/lib/backend-manager.js:728-753` </location>
<code_context>
+ );
+ }
+
+ getWindowsProcessCommandLine(pid) {
+ const numericPid = Number.parseInt(`${pid}`, 10);
+ if (!Number.isInteger(numericPid)) {
+ return null;
+ }
+
+ const query = `$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${numericPid}"; if ($null -ne $p) { $p.CommandLine }`;
+ const result = spawnSync(
+ 'powershell',
+ ['-NoProfile', '-NonInteractive', '-Command', query],
+ {
+ stdio: ['ignore', 'pipe', 'ignore'],
+ encoding: 'utf8',
+ windowsHide: true,
+ },
+ );
+ if (result.status !== 0 || !result.stdout) {
+ return null;
+ }
+
+ const line = result.stdout
+ .split(/\r?\n/)
+ .map((item) => item.trim())
+ .find((item) => item.length > 0);
+ return line || null;
+ }
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 考虑为 PowerShell 的 `spawnSync` 调用添加一个超时时间，以避免在关闭时可能出现的卡死。
由于这段代码跑在关闭/清理路径上，使用同步调用是合理的，但如果 PowerShell 进程变慢或卡住，仍然会阻塞应用关闭。通过为 `spawnSync` 添加一个合理的 `timeout`，并在超时时将结果视为 `null`/命令行不可用，可以在不改变清理行为的前提下提高健壮性。
```suggestion
getWindowsProcessCommandLine(pid) {
const numericPid = Number.parseInt(`${pid}`, 10);
if (!Number.isInteger(numericPid)) {
return null;
}
const query = `$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${numericPid}"; if ($null -ne $p) { $p.CommandLine }`;
const result = spawnSync(
'powershell',
['-NoProfile', '-NonInteractive', '-Command', query],
{
stdio: ['ignore', 'pipe', 'ignore'],
encoding: 'utf8',
windowsHide: true,
// Avoid hangs during shutdown: fail fast if PowerShell is slow or stuck.
timeout: 2000, // milliseconds
},
);
const timedOut = result.error && result.error.code === 'ETIMEDOUT';
if (timedOut || result.status !== 0 || !result.stdout) {
return null;
}
const line = result.stdout
.split(/\r?\n/)
.map((item) => item.trim())
.find((item) => item.length > 0);
return line || null;
}
```
</issue_to_address>
### Comment 2
<location> `astrbot/core/utils/pip_installer.py:441` </location>
<code_context>
+ wheel_only: bool = False,
) -> None:
args = ["install"]
+ pip_install_args = self.pip_install_arg.split() if self.pip_install_arg else []
requested_requirements: set[str] = set()
if package_name:
</code_context>
<issue_to_address>
**suggestion:** 直接对 `pip_install_arg` 使用普通的 `str.split()` 会错误处理带引号的参数，并影响对 `wheel_only` 的检测。
由于 `pip_install_args` 是用一个简单的 `.split()` 构造的，任何带引号的片段或带空格的值（例如 extra-index URL 或复合选项）都可能被错误拆分，因此用于检测 `--only-binary` / `--prefer-binary` 的逻辑可能看不到真实的参数 token。为了更健壮，建议使用 `shlex.split(self.pip_install_arg)`，它会正确尊重引号，同时依然返回一个参数列表。
建议实现：
```python
import shlex
class PipInstaller:
```
```python
pip_install_args = shlex.split(self.pip_install_arg) if self.pip_install_arg else []
```
如果这个文件在顶部已经有 import 区块，建议把 `import shlex` 移动到上面，与其他标准库 import 放在一起，以符合项目的 import 排序约定。
</issue_to_address>
### Comment 3
<location> `desktop/README.md:43` </location>
<code_context>
pnpm --dir desktop run dist:full
```
+If you are already developing in this repository, you can directly reuse the local virtual environment as runtime:
+
+```bash
</code_context>
<issue_to_address>
**nitpick (typo):** 建议将 "as runtime" 改为 "as the runtime"，语气会更自然一些。
例如：`reuse the local virtual environment as the runtime` 会更符合英文习惯表达。
</issue_to_address>
### Comment 4
<location> `desktop/lib/backend-manager.js:133` </location>
<code_context>
}
- getPackagedBackendPath() {
+ getPackagedBackendDir() {
if (!this.app.isPackaged) {
return null;
</code_context>
<issue_to_address>
**issue (complexity):** 建议将打包 backend 的 manifest/路径处理逻辑集中起来，并把 Windows 非托管 backend 清理的启发式逻辑抽取到独立的帮助方法中，以简化新增逻辑，让每个方法更易于理解。
你可以在不改变行为的情况下，通过以下方式降低新增复杂度：
---
### 1. 集中处理打包 backend 的 manifest/路径逻辑
目前每个 helper 都会：
- 重新解析 `backendDir`
- 重新获取 `manifest`
- 重复执行“相对路径 → join → existsSync → null”的模式
- 反复对 `manifest.<field>` 做类型保护
你可以保持对外 API 不变，但将这些逻辑重构为：
1. 一个带缓存的单一加载器，它：
- 解析 `backendDir`
- 加载并解析 manifest，一次完成
- 应用默认值
- 预先解析出绝对路径
2. 一个用于“相对路径 → join → existsSync”模式的小工具函数。
例如：
```js
// Small utility for path fields
resolveManifestPath(backendDir, manifest, key, defaultRelative) {
const rel = manifest && typeof manifest[key] === 'string' && manifest[key]
? manifest[key]
: defaultRelative;
const candidate = path.join(backendDir, rel);
return fs.existsSync(candidate) ? candidate : null;
}
// Centralized loader
loadPackagedBackendConfig() {
if (!this.app.isPackaged) return null;
if (this._packagedBackendConfig) return this._packagedBackendConfig;
const backendDir = path.join(process.resourcesPath, 'backend');
if (!fs.existsSync(backendDir)) return null;
let manifest = {};
const manifestPath = path.join(backendDir, 'runtime-manifest.json');
if (fs.existsSync(manifestPath)) {
try {
const raw = fs.readFileSync(manifestPath, 'utf8');
const parsed = JSON.parse(raw);
if (parsed && typeof parsed === 'object') manifest = parsed;
} catch (error) {
this.log(
`Failed to parse packaged backend manifest: ${
error instanceof Error ? error.message : String(error)
}`,
);
}
}
const appDir = this.resolveManifestPath(backendDir, manifest, 'app', 'app');
const launchScriptPath = this.resolveManifestPath(
backendDir,
manifest,
'entrypoint',
'launch_backend.py',
);
const defaultPythonRel =
process.platform === 'win32'
? path.join('python', 'Scripts', 'python.exe')
: path.join('python', 'bin', 'python3');
const pythonPath = this.resolveManifestPath(
backendDir,
manifest,
'python',
defaultPythonRel,
);
this._packagedBackendConfig = Object.freeze({
backendDir,
appDir,
launchScriptPath,
pythonPath,
});
return this._packagedBackendConfig;
}
```
然后，其他 helpers 就可以变成很薄的一层包装（或者在使用处直接内联）：
```js
getPackagedBackendAppDir() {
const cfg = this.loadPackagedBackendConfig();
return cfg ? cfg.appDir : null;
}
getPackagedBackendLaunchScriptPath() {
const cfg = this.loadPackagedBackendConfig();
return cfg ? cfg.launchScriptPath : null;
}
getPackagedRuntimePythonPath() {
const cfg = this.loadPackagedBackendConfig();
return cfg ? cfg.pythonPath : null;
}
buildPackagedBackendLaunch(webuiDir) {
const cfg = this.loadPackagedBackendConfig();
if (!cfg || !cfg.pythonPath || !cfg.launchScriptPath) return null;
const args = [cfg.launchScriptPath];
if (webuiDir) args.push('--webui-dir', webuiDir);
return { cmd: cfg.pythonPath, args, shell: false };
}
```
这样可以：
- 去掉重复的 null 检查以及 `backendDir`/manifest 管道逻辑。
- 将 manifest 解析、默认值和缓存集中在一个地方。
- 让依赖这些信息的方法变得非常简单、易于理解。
---
### 2. 抽取 Windows 非托管清理的启发式逻辑
`stopUnmanagedBackendByPort` 现在在循环中混合了很多不同的关注点。你可以把“这个 pid 是否应该被杀掉？”的逻辑提取为独立 helper，让端口查找 + 循环遍历保持更纯粹。
例如：
```js
shouldKillUnmanagedBackendProcess(pid, processInfo, backendConfig) {
const expectedImageName = path.basename(backendConfig.cmd || 'python.exe').toLowerCase();
const actualImageName = processInfo.imageName.toLowerCase();
if (actualImageName !== expectedImageName) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unexpected process image ${processInfo.imageName}.`,
);
return false;
}
const requireStrictCommandLineCheck =
this.isGenericWindowsPythonImage(expectedImageName);
if (!requireStrictCommandLineCheck) {
return true;
}
const expectedCommandLineMarkers = [];
if (Array.isArray(backendConfig.args) && backendConfig.args.length > 0) {
const primaryArg = backendConfig.args[0];
if (typeof primaryArg === 'string' && primaryArg) {
const resolvedPrimaryArg = path.isAbsolute(primaryArg)
? primaryArg
: path.resolve(backendConfig.cwd || process.cwd(), primaryArg);
expectedCommandLineMarkers.push(
this.normalizeWindowsPathForMatch(resolvedPrimaryArg),
);
expectedCommandLineMarkers.push(
this.normalizeWindowsPathForMatch(path.basename(primaryArg)),
);
}
}
if (!expectedCommandLineMarkers.length) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: backend launch marker is unavailable.`,
);
return false;
}
const commandLine = this.getWindowsProcessCommandLine(pid);
if (!commandLine) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process command line.`,
);
return false;
}
const normalizedCommandLine = this.normalizeWindowsPathForMatch(commandLine);
const markerMatched = expectedCommandLineMarkers.some(
(marker) => marker && normalizedCommandLine.includes(marker),
);
if (!markerMatched) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: command line does not match AstrBot backend launch marker.`,
);
return false;
}
return true;
}
```
并简化主方法：
```js
async stopUnmanagedBackendByPort() {
if (!this.app.isPackaged || process.platform !== 'win32') return false;
const port = this.getBackendPort();
if (!port) return false;
const pids = this.findListeningPidsOnWindows(port);
if (!pids.length) return false;
this.log(
`Attempting unmanaged backend cleanup by port=${port} pids=${pids.join(',')}`,
);
const backendConfig = this.getBackendConfig();
for (const pid of pids) {
const processInfo = this.getWindowsProcessInfo(pid);
if (!processInfo) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process info.`,
);
continue;
}
if (!this.shouldKillUnmanagedBackendProcess(pid, processInfo, backendConfig)) {
continue;
}
try {
spawnSync('taskkill', ['/pid', `${pid}`, '/t', '/f'], {
stdio: 'ignore',
windowsHide: true,
});
} catch {}
}
return true;
}
```
这样可以保留所有现有检查和日志记录，但：
- 让 `stopUnmanagedBackendByPort` 主要负责流程编排。
- 将“什么算是 AstrBot backend 进程”的判断逻辑集中在一个 helper 中。
- 减少循环中的分支和嵌套，在不损失新行为的情况下，解决复杂度问题。
</issue_to_address>请帮我变得更有用！欢迎在每条评论上点 👍 或 👎，我会根据反馈改进后续评审。
Original comment in English
Hey - I've found 4 issues, and left some high level feedback:
- On Windows unmanaged-backend cleanup,
getWindowsProcessCommandLineinvokes a synchronous PowerShell command per PID without a timeout or fallback; consider adding a timeout and a graceful fallback for environments where PowerShell is unavailable or slow to avoid hangs during shutdown. - In
desktop/scripts/build-backend.mjs, the build will recursivelycpSyncthe runtime directory intodesktop/resources/backend/pythoneven if the source and destination overlap; adding a guard to preventASTRBOT_DESKTOP_CPYTHON_HOMEpointing insidedesktop/resources/backendwould avoid accidentally copying build artifacts into themselves.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- On Windows unmanaged-backend cleanup, `getWindowsProcessCommandLine` invokes a synchronous PowerShell command per PID without a timeout or fallback; consider adding a timeout and a graceful fallback for environments where PowerShell is unavailable or slow to avoid hangs during shutdown.
- In `desktop/scripts/build-backend.mjs`, the build will recursively `cpSync` the runtime directory into `desktop/resources/backend/python` even if the source and destination overlap; adding a guard to prevent `ASTRBOT_DESKTOP_CPYTHON_HOME` pointing inside `desktop/resources/backend` would avoid accidentally copying build artifacts into themselves.
## Individual Comments
### Comment 1
<location> `desktop/lib/backend-manager.js:728-753` </location>
<code_context>
+ );
+ }
+
+ getWindowsProcessCommandLine(pid) {
+ const numericPid = Number.parseInt(`${pid}`, 10);
+ if (!Number.isInteger(numericPid)) {
+ return null;
+ }
+
+ const query = `$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${numericPid}"; if ($null -ne $p) { $p.CommandLine }`;
+ const result = spawnSync(
+ 'powershell',
+ ['-NoProfile', '-NonInteractive', '-Command', query],
+ {
+ stdio: ['ignore', 'pipe', 'ignore'],
+ encoding: 'utf8',
+ windowsHide: true,
+ },
+ );
+ if (result.status !== 0 || !result.stdout) {
+ return null;
+ }
+
+ const line = result.stdout
+ .split(/\r?\n/)
+ .map((item) => item.trim())
+ .find((item) => item.length > 0);
+ return line || null;
+ }
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider adding a timeout to the PowerShell `spawnSync` call to avoid potential hangs during shutdown.
Since this runs in shutdown/cleanup paths, a synchronous call is reasonable, but a slow or stuck PowerShell process would still block app shutdown. Adding a reasonable `timeout` to `spawnSync` and treating timeouts as `null`/unavailable command lines would improve robustness without changing the cleanup behavior.
```suggestion
getWindowsProcessCommandLine(pid) {
const numericPid = Number.parseInt(`${pid}`, 10);
if (!Number.isInteger(numericPid)) {
return null;
}
const query = `$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${numericPid}"; if ($null -ne $p) { $p.CommandLine }`;
const result = spawnSync(
'powershell',
['-NoProfile', '-NonInteractive', '-Command', query],
{
stdio: ['ignore', 'pipe', 'ignore'],
encoding: 'utf8',
windowsHide: true,
// Avoid hangs during shutdown: fail fast if PowerShell is slow or stuck.
timeout: 2000, // milliseconds
},
);
const timedOut = result.error && result.error.code === 'ETIMEDOUT';
if (timedOut || result.status !== 0 || !result.stdout) {
return null;
}
const line = result.stdout
.split(/\r?\n/)
.map((item) => item.trim())
.find((item) => item.length > 0);
return line || null;
}
```
</issue_to_address>
### Comment 2
<location> `astrbot/core/utils/pip_installer.py:441` </location>
<code_context>
+ wheel_only: bool = False,
) -> None:
args = ["install"]
+ pip_install_args = self.pip_install_arg.split() if self.pip_install_arg else []
requested_requirements: set[str] = set()
if package_name:
</code_context>
<issue_to_address>
**suggestion:** Using a plain `str.split()` for `pip_install_arg` can mis-handle quoted arguments and affect `wheel_only` detection.
Because `pip_install_args` is built with a naive `.split()`, any quoted segments or values with spaces (e.g., extra-index URLs or composite options) may be split incorrectly, so the `wheel_only` detection for `--only-binary` / `--prefer-binary` may not see the real tokens. To make this robust, consider using `shlex.split(self.pip_install_arg)` instead, which will respect quoting while still returning a list of args.
Suggested implementation:
```python
import shlex
class PipInstaller:
```
```python
pip_install_args = shlex.split(self.pip_install_arg) if self.pip_install_arg else []
```
If this file already has an import section at the top, you may want to move `import shlex` up to keep it grouped with the other standard library imports, following your project's import ordering conventions.
</issue_to_address>
### Comment 3
<location> `desktop/README.md:43` </location>
<code_context>
pnpm --dir desktop run dist:full
```
+If you are already developing in this repository, you can directly reuse the local virtual environment as runtime:
+
+```bash
</code_context>
<issue_to_address>
**nitpick (typo):** Consider "as the runtime" instead of "as runtime" for natural phrasing.
For example: `reuse the local virtual environment as the runtime` reads more naturally.
</issue_to_address>
### Comment 4
<location> `desktop/lib/backend-manager.js:133` </location>
<code_context>
}
- getPackagedBackendPath() {
+ getPackagedBackendDir() {
if (!this.app.isPackaged) {
return null;
</code_context>
<issue_to_address>
**issue (complexity):** Consider centralizing the packaged backend manifest/path handling and extracting the Windows unmanaged-backend cleanup heuristics into dedicated helpers to simplify the new logic and make each method easier to reason about.
You can reduce the added complexity without changing behavior by:
---
### 1. Centralizing packaged backend manifest/path logic
Right now each helper:
- Re-resolves `backendDir`
- Re-fetches `manifest`
- Repeats “relative path → join → existsSync → null” pattern
- Repeats type guards on `manifest.<field>`
You can keep the same externally-visible API but factor this into:
1. A single cached loader that:
- Resolves `backendDir`
- Loads + parses the manifest once
- Applies defaults
- Pre-resolves absolute paths
2. A tiny utility for the “relative → joined → existsSync” pattern.
For example:
```js
// Small utility for path fields
resolveManifestPath(backendDir, manifest, key, defaultRelative) {
const rel = manifest && typeof manifest[key] === 'string' && manifest[key]
? manifest[key]
: defaultRelative;
const candidate = path.join(backendDir, rel);
return fs.existsSync(candidate) ? candidate : null;
}
// Centralized loader
loadPackagedBackendConfig() {
if (!this.app.isPackaged) return null;
if (this._packagedBackendConfig) return this._packagedBackendConfig;
const backendDir = path.join(process.resourcesPath, 'backend');
if (!fs.existsSync(backendDir)) return null;
let manifest = {};
const manifestPath = path.join(backendDir, 'runtime-manifest.json');
if (fs.existsSync(manifestPath)) {
try {
const raw = fs.readFileSync(manifestPath, 'utf8');
const parsed = JSON.parse(raw);
if (parsed && typeof parsed === 'object') manifest = parsed;
} catch (error) {
this.log(
`Failed to parse packaged backend manifest: ${
error instanceof Error ? error.message : String(error)
}`,
);
}
}
const appDir = this.resolveManifestPath(backendDir, manifest, 'app', 'app');
const launchScriptPath = this.resolveManifestPath(
backendDir,
manifest,
'entrypoint',
'launch_backend.py',
);
const defaultPythonRel =
process.platform === 'win32'
? path.join('python', 'Scripts', 'python.exe')
: path.join('python', 'bin', 'python3');
const pythonPath = this.resolveManifestPath(
backendDir,
manifest,
'python',
defaultPythonRel,
);
this._packagedBackendConfig = Object.freeze({
backendDir,
appDir,
launchScriptPath,
pythonPath,
});
return this._packagedBackendConfig;
}
```
Then the other helpers become thin wrappers (or can be inlined where used):
```js
getPackagedBackendAppDir() {
const cfg = this.loadPackagedBackendConfig();
return cfg ? cfg.appDir : null;
}
getPackagedBackendLaunchScriptPath() {
const cfg = this.loadPackagedBackendConfig();
return cfg ? cfg.launchScriptPath : null;
}
getPackagedRuntimePythonPath() {
const cfg = this.loadPackagedBackendConfig();
return cfg ? cfg.pythonPath : null;
}
buildPackagedBackendLaunch(webuiDir) {
const cfg = this.loadPackagedBackendConfig();
if (!cfg || !cfg.pythonPath || !cfg.launchScriptPath) return null;
const args = [cfg.launchScriptPath];
if (webuiDir) args.push('--webui-dir', webuiDir);
return { cmd: cfg.pythonPath, args, shell: false };
}
```
This:
- Removes repeated null checks and `backendDir`/manifest plumbing.
- Keeps manifest parsing, defaults, and caching in one place.
- Makes the dependent methods trivial and easier to reason about.
---
### 2. Extracting unmanaged Windows cleanup heuristics
`stopUnmanagedBackendByPort` now mixes a lot of concerns inside the loop. You can pull out the “should this pid be killed?” logic into a dedicated helper and keep port lookup + iteration separate.
For example:
```js
shouldKillUnmanagedBackendProcess(pid, processInfo, backendConfig) {
const expectedImageName = path.basename(backendConfig.cmd || 'python.exe').toLowerCase();
const actualImageName = processInfo.imageName.toLowerCase();
if (actualImageName !== expectedImageName) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unexpected process image ${processInfo.imageName}.`,
);
return false;
}
const requireStrictCommandLineCheck =
this.isGenericWindowsPythonImage(expectedImageName);
if (!requireStrictCommandLineCheck) {
return true;
}
const expectedCommandLineMarkers = [];
if (Array.isArray(backendConfig.args) && backendConfig.args.length > 0) {
const primaryArg = backendConfig.args[0];
if (typeof primaryArg === 'string' && primaryArg) {
const resolvedPrimaryArg = path.isAbsolute(primaryArg)
? primaryArg
: path.resolve(backendConfig.cwd || process.cwd(), primaryArg);
expectedCommandLineMarkers.push(
this.normalizeWindowsPathForMatch(resolvedPrimaryArg),
);
expectedCommandLineMarkers.push(
this.normalizeWindowsPathForMatch(path.basename(primaryArg)),
);
}
}
if (!expectedCommandLineMarkers.length) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: backend launch marker is unavailable.`,
);
return false;
}
const commandLine = this.getWindowsProcessCommandLine(pid);
if (!commandLine) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process command line.`,
);
return false;
}
const normalizedCommandLine = this.normalizeWindowsPathForMatch(commandLine);
const markerMatched = expectedCommandLineMarkers.some(
(marker) => marker && normalizedCommandLine.includes(marker),
);
if (!markerMatched) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: command line does not match AstrBot backend launch marker.`,
);
return false;
}
return true;
}
```
And simplify the main method:
```js
async stopUnmanagedBackendByPort() {
if (!this.app.isPackaged || process.platform !== 'win32') return false;
const port = this.getBackendPort();
if (!port) return false;
const pids = this.findListeningPidsOnWindows(port);
if (!pids.length) return false;
this.log(
`Attempting unmanaged backend cleanup by port=${port} pids=${pids.join(',')}`,
);
const backendConfig = this.getBackendConfig();
for (const pid of pids) {
const processInfo = this.getWindowsProcessInfo(pid);
if (!processInfo) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process info.`,
);
continue;
}
if (!this.shouldKillUnmanagedBackendProcess(pid, processInfo, backendConfig)) {
continue;
}
try {
spawnSync('taskkill', ['/pid', `${pid}`, '/t', '/f'], {
stdio: 'ignore',
windowsHide: true,
});
} catch {}
}
return true;
}
```
This keeps all existing checks and logging, but:
- Makes `stopUnmanagedBackendByPort` mostly orchestration.
- Localizes “what qualifies as an AstrBot backend process” into one helper.
- Reduces in-loop branching and nesting, addressing the complexity concern without losing the new behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
1 similar comment
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - 我发现了 3 个问题，并给出了一些整体反馈：
build-backend.mjs中的readProjectRequiresPythonLowerBound/resolveExpectedRuntimeVersion逻辑依赖一个非常具体的requires-python = "..."行格式；建议考虑使用一个最小的 TOML 解析器，或者更灵活的正则，这样在格式发生变化时（单引号、额外空白、或放在工具专用的 table 下）不会悄悄导致运行时版本检测失效。- 在
BackendManager.stopUnmanagedBackendByPort中，getWindowsProcessCommandLine里的同步 PowerShell（powershell/pwsh）调用，每个 PID 最长可以阻塞事件循环达 2 秒；如果这段逻辑运行在 UI / 关键路径上，建议把命令行探测迁移到异步 / worker 进程中，或者缩短超时时间并收紧这个路径被调用的条件。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- `build-backend.mjs` 中的 `readProjectRequiresPythonLowerBound` / `resolveExpectedRuntimeVersion` 逻辑依赖一个非常具体的 `requires-python = "..."` 行格式；建议考虑使用一个最小的 TOML 解析器，或者更灵活的正则，这样在格式发生变化时（单引号、额外空白、或放在工具专用的 table 下）不会悄悄导致运行时版本检测失效。
- 在 `BackendManager.stopUnmanagedBackendByPort` 中，`getWindowsProcessCommandLine` 里的同步 PowerShell（`powershell` / `pwsh`）调用，每个 PID 最长可以阻塞事件循环达 2 秒；如果这段逻辑运行在 UI / 关键路径上，建议把命令行探测迁移到异步 / worker 进程中，或者缩短超时时间并收紧这个路径被调用的条件。
## Individual Comments
### Comment 1
<location> `desktop/lib/backend-manager.js:860-869` </location>
<code_context>
+ return null;
+ }
+
+ buildWindowsUnmanagedBackendMatcher(backendConfig) {
+ const expectedImageName = path.basename(backendConfig.cmd || 'python.exe').toLowerCase();
+ const requireStrictCommandLineCheck =
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against missing or incomplete backendConfig when building the unmanaged Windows backend matcher.
In `stopUnmanagedBackendByPort`, `this.getBackendConfig()` is passed straight into `buildWindowsUnmanagedBackendMatcher` without checking for `null`/`undefined` or expected shape. If `getBackendConfig()` ever returns a missing/partial config (e.g. no `cmd`/`cwd`/`args`), accessing `backendConfig.cmd`/`.cwd` will throw. Consider either early-returning from `stopUnmanagedBackendByPort` when the config is invalid, or having `buildWindowsUnmanagedBackendMatcher` handle missing/partial config (e.g. return `null`) and skipping cleanup in that case.
</issue_to_address>
### Comment 2
<location> `desktop/lib/backend-manager.js:136` </location>
<code_context>
- getPackagedBackendPath() {
- if (!this.app.isPackaged) {
+ getPackagedBackendDir() {
+ const packagedBackendConfig = this.loadPackagedBackendConfig();
+ return packagedBackendConfig ? packagedBackendConfig.backendDir : null;
</code_context>
<issue_to_address>
**issue (complexity):** 考虑简化新的 packaged-backend 配置和 Windows 进程匹配辅助函数，通过内联逻辑并整合这些小的包装器，让控制流更加线性、易于阅读。
- 新增的 packaged-backend 配置辅助函数都是非常薄的一层包装，并且扩展了类的公共接口。你可以把配置的使用集中在一起，这样既能把配置结构局部化，又能减少方法数量。
```js
// 保留一个统一入口
getPackagedBackendConfig() {
return this.loadPackagedBackendConfig();
}
// 在调用点直接使用，而不是多个很小的 getter
resolveBackendCwd() {
if (!this.app.isPackaged) {
return path.resolve(this.baseDir, '..');
}
const cfg = this.getPackagedBackendConfig();
return (cfg && cfg.appDir) || this.resolveBackendRoot();
}
buildPackagedBackendLaunch(webuiDir) {
const cfg = this.getPackagedBackendConfig();
if (!cfg || !cfg.runtimePythonPath || !cfg.launchScriptPath) {
return null;
}
const args = [cfg.launchScriptPath];
if (webuiDir) args.push('--webui-dir', webuiDir);
return { cmd: cfg.runtimePythonPath, args, shell: false };
}
```
这样你就可以删除 `getPackagedBackendManifest`、`getPackagedBackendAppDir`、`getPackagedBackendLaunchScriptPath`、`getPackagedRuntimePythonPath` 等方法，只在 `loadPackagedBackendConfig()` 加一个统一 getter 来持有配置结构的知识。
- `resolveManifestPath` 加上在 `loadPackagedBackendConfig()` 中的三个调用，其实可以改成更显式的写法而不是通用方法，这样更容易一眼看懂：
```js
loadPackagedBackendConfig() {
// ...
const manifest = this.parsePackagedBackendManifest(backendDir) || {};
const defaultPythonRelative =
process.platform === 'win32'
? path.join('python', 'Scripts', 'python.exe')
: path.join('python', 'bin', 'python3');
const resolve = (key, fallback) => {
const rel = (typeof manifest[key] === 'string' && manifest[key]) || fallback;
const candidate = path.join(backendDir, rel);
return fs.existsSync(candidate) ? candidate : null;
};
this.packagedBackendConfig = Object.freeze({
backendDir,
manifest: Object.keys(manifest).length ? manifest : null,
appDir: resolve('app', 'app'),
launchScriptPath: resolve('entrypoint', 'launch_backend.py'),
runtimePythonPath: resolve('python', defaultPythonRelative),
});
return this.packagedBackendConfig;
}
```
这样所有逻辑都集中在一个地方，而不需要单独的 `resolveManifestPath` 方法。
- PowerShell / Pwsh 查询辅助函数几乎完全相同；你可以把它们合并成一个可传参的函数，并把 `parseWindowsProcessCommandLine` 内联掉，以减少间接跳转：
```js
queryWindowsProcessCommandLine(pid, shellName) {
const query = `$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}"; if ($null -ne $p) { $p.CommandLine }`;
return spawnSync(
shellName,
['-NoProfile', '-NonInteractive', '-Command', query],
{
stdio: ['ignore', 'pipe', 'ignore'],
encoding: 'utf8',
windowsHide: true,
timeout: WINDOWS_PROCESS_QUERY_TIMEOUT_MS,
},
);
}
getWindowsProcessCommandLine(pid) {
const numericPid = Number.parseInt(`${pid}`, 10);
if (!Number.isInteger(numericPid)) return null;
const cached = this.windowsProcessCommandLineCache.get(numericPid);
if (cached && Date.now() - cached.timestampMs <= WINDOWS_PROCESS_COMMAND_LINE_CACHE_TTL_MS) {
return cached.commandLine;
}
this.windowsProcessCommandLineCache.delete(numericPid);
for (const shellName of ['powershell', 'pwsh']) {
let result;
try {
result = this.queryWindowsProcessCommandLine(numericPid, shellName);
} catch (error) {
// existing logging
continue;
}
// existing error/timeout handling...
if (result.status === 0 && result.stdout) {
const commandLine =
result.stdout
.split(/\r?\n/)
.map((s) => s.trim())
.find((s) => s.length > 0) || null;
this.windowsProcessCommandLineCache.set(numericPid, {
commandLine,
timestampMs: Date.now(),
});
return commandLine;
}
}
this.windowsProcessCommandLineCache.set(numericPid, { commandLine: null, timestampMs: Date.now() });
return null;
}
```
- 匹配器对象（`buildWindowsUnmanagedBackendMatcher` + `shouldKillUnmanagedBackendProcess`）只在一个调用点被使用。你可以用一个函数同时做 image 检查和命令行检查，让流程更线性：
```js
isUnmanagedBackendProcess(pid, processInfo, backendConfig) {
const expectedImageName = path.basename(backendConfig.cmd || 'python.exe').toLowerCase();
const actualImageName = processInfo.imageName.toLowerCase();
if (actualImageName !== expectedImageName) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unexpected process image ${processInfo.imageName}.`,
);
return false;
}
if (!this.isGenericWindowsPythonImage(expectedImageName)) {
return true;
}
const args = Array.isArray(backendConfig.args) ? backendConfig.args : [];
const primaryArg = args[0];
if (typeof primaryArg !== 'string' || !primaryArg) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: backend launch marker is unavailable.`,
);
return false;
}
const cwd = backendConfig.cwd || process.cwd();
const markers = [
this.normalizeWindowsPathForMatch(
path.isAbsolute(primaryArg) ? primaryArg : path.resolve(cwd, primaryArg),
),
this.normalizeWindowsPathForMatch(path.basename(primaryArg)),
];
const commandLine = this.getWindowsProcessCommandLine(pid);
if (!commandLine) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process command line.`,
);
return false;
}
const normalized = this.normalizeWindowsPathForMatch(commandLine);
const matched = markers.some((m) => m && normalized.includes(m));
if (!matched) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: command line does not match AstrBot backend launch marker.`,
);
return false;
}
return true;
}
async stopUnmanagedBackendByPort() {
// ...
const backendConfig = this.getBackendConfig();
for (const pid of pids) {
const processInfo = this.getWindowsProcessInfo(pid);
if (!processInfo) {
// existing log
continue;
}
if (!this.isUnmanagedBackendProcess(pid, processInfo, backendConfig)) {
continue;
}
// taskkill...
}
}
```
这样可以完全移除 `processMatcher` 对象 / 类型，同时保留所有检查逻辑。
- 考虑到 `stopUnmanagedBackendByPort` 是一个不常被调用的操作，你可能不需要带 TTL 和清理逻辑的长生命周期缓存。一个更简单的按调用缓存（没有时间戳，也没有清理）就能保持行为一致，同时移除全局状态：
```js
async stopUnmanagedBackendByPort() {
if (!this.app.isPackaged || process.platform !== 'win32') {
return false;
}
const commandLineCache = new Map(); // pid -> string|null
const getCommandLineOnce = (pid) => {
if (commandLineCache.has(pid)) return commandLineCache.get(pid);
const value = this.getWindowsProcessCommandLine(pid); // simplified version w/o TTL
commandLineCache.set(pid, value);
return value;
};
// 在 isUnmanagedBackendProcess 中使用 getCommandLineOnce，或者直接内联
}
```
如果你想保留现有的 TTL 语义，也可以把缓存范围收窄到 `stopUnmanagedBackendByPort`（例如把一个 `getCommandLine` 函数传给 `isUnmanagedBackendProcess`），从而移除 `this.windowsProcessCommandLineCache` 和 `pruneWindowsProcessCommandLineCache`。
</issue_to_address>
### Comment 3
<location> `desktop/scripts/build-backend.mjs:32` </location>
<code_context>
+ process.exit(1);
+}
+
+const normalizePathForCompare = (targetPath) => {
+ const resolved = path.resolve(targetPath).replace(/[\\/]+$/, '');
+ return process.platform === 'win32' ? resolved.toLowerCase() : resolved;
</code_context>
<issue_to_address>
**issue (complexity):** 考虑简化新的运行时校验辅助函数，通过合并重叠的函数，并使用更清晰的内联工具函数（用于路径重叠检查、版本解析、Python 校验），从而让控制流更直观、更不容易出错。
- 路径重叠检查的辅助函数只是为单个安全检查增加了一层抽象，而且当前还有一个逻辑 bug（`if (! isSameOrSubPath(...))`）。你可以直接用 `path.relative` 配合一个非常小的内联工具，同时实现简化与修复：
```js
const pathsOverlap = (a, b) => {
const relA = path.relative(a, b);
const relB = path.relative(b, a);
const isSub = (rel) => rel === '' || (!rel.startsWith('..') && !path.isAbsolute(rel));
return isSub(relA) || isSub(relB);
};
if (pathsOverlap(runtimeSourceReal, outputDir)) {
console.error(
`CPython runtime source overlaps with backend output directory. ` +
`runtime=${runtimeSourceReal}, output=${outputDir}. ` +
'Please set ASTRBOT_DESKTOP_CPYTHON_HOME to a separate runtime directory.',
);
process.exit(1);
}
```
这样就可以完全移除 `normalizePathForCompare` 和 `isSameOrSubPath`，并且让意图（“这些路径是否有重叠？”）一目了然。
- 版本解析流水线可以通过把 `parseExpectedRuntimeVersion` 和 `readProjectRequiresPythonLowerBound` 合并进 `resolveExpectedRuntimeVersion` 来简化，只保留一个辅助函数：
```js
const resolveExpectedRuntimeVersion = () => {
const parse = (raw, sourceName) => {
const match = /^(\d+)\.(\d+)$/.exec(String(raw).trim());
if (!match) {
console.error(
`Invalid expected Python version from ${sourceName}: ${raw}. ` +
'Expected format <major>.<minor>.',
);
process.exit(1);
}
return { major: Number(match[1]), minor: Number(match[2]) };
};
if (process.env.ASTRBOT_DESKTOP_EXPECTED_PYTHON) {
return parse(process.env.ASTRBOT_DESKTOP_EXPECTED_PYTHON, 'ASTRBOT_DESKTOP_EXPECTED_PYTHON');
}
const pyprojectPath = path.join(rootDir, 'pyproject.toml');
if (fs.existsSync(pyprojectPath)) {
const content = fs.readFileSync(pyprojectPath, 'utf8');
const requires = /^\s*requires-python\s*=\s*"([^"]+)"/m.exec(content)?.[1];
const lower = requires && />=\s*(\d+)\.(\d+)/.exec(requires);
if (lower) {
return parse(`${lower[1]}.${lower[2]}`, 'pyproject.toml requires-python');
}
}
console.error(
'Unable to determine expected runtime Python version. ' +
'Set ASTRBOT_DESKTOP_EXPECTED_PYTHON or declare project.requires-python in pyproject.toml.',
);
process.exit(1);
};
```
这样可以移除两个全局函数，并把所有错误处理放在一个地方。
- 通过把版本比较逻辑抽取成一个明确的条件，并复用一个通用的退出辅助函数，可以简化 `validateRuntimePython`，并修复当前比较逻辑中的 bug：
```js
const fatal = (msg) => {
console.error(msg);
process.exit(1);
};
const validateRuntimePython = (pythonExecutable) => {
const probe = spawnSync(
pythonExecutable,
['-c', 'import sys, pip; print(f"{sys.version_info.major}.{sys.version_info.minor}")'],
{
stdio: ['ignore', 'pipe', 'pipe'],
encoding: 'utf8',
windowsHide: true,
timeout: 5000,
},
);
if (probe.error) {
const reason =
probe.error.code === 'ETIMEDOUT'
? 'runtime Python probe timed out'
: probe.error.message || String(probe.error);
fatal(`Runtime Python probe failed: ${reason}`);
}
if (probe.status !== 0) {
const stderrText = (probe.stderr || '').trim();
fatal(
`Runtime Python probe failed with exit code ${probe.status}.` +
(stderrText ? ` stderr: ${stderrText}` : ''),
);
}
const versionMatch = /(\d+)\.(\d+)/.exec((probe.stdout || '').trim());
if (!versionMatch) {
fatal(
`Runtime Python probe did not report a valid version. Output: ${(probe.stdout || '').trim()}`,
);
}
const actual = { major: Number(versionMatch[1]), minor: Number(versionMatch[2]) };
if (actual.major !== expectedRuntimeVersion.major || actual.minor !== expectedRuntimeVersion.minor) {
fatal(
`Runtime Python version mismatch: expected ${expectedRuntimeVersion.major}.${expectedRuntimeVersion.minor}, ` +
`got ${actual.major}.${actual.minor}.`,
);
}
};
```
这就消除了 `! actualVersion.major !== ...` 这种易混淆的写法，并把 `console.error` / `process.exit(1)` 的模式集中到一起，从而减少分支噪音。
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈持续改进代码审查质量。
Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
- The
readProjectRequiresPythonLowerBound/resolveExpectedRuntimeVersionlogic inbuild-backend.mjsrelies on a very specificrequires-python = "..."line format; consider using a minimal TOML parser or more flexible regexes so that alternate formatting (single quotes, extra whitespace, or tool-specific tables) doesn’t silently break runtime version detection. - In
BackendManager.stopUnmanagedBackendByPortthe synchronous PowerShell (powershell/pwsh) calls ingetWindowsProcessCommandLinecan block the event loop for up to 2 seconds per PID; if this runs on the UI/critical path, consider moving command-line probing to an async/worker process or lowering the timeout and tightening when this path is invoked.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `readProjectRequiresPythonLowerBound`/`resolveExpectedRuntimeVersion` logic in `build-backend.mjs` relies on a very specific `requires-python = "..."` line format; consider using a minimal TOML parser or more flexible regexes so that alternate formatting (single quotes, extra whitespace, or tool-specific tables) doesn’t silently break runtime version detection.
- In `BackendManager.stopUnmanagedBackendByPort` the synchronous PowerShell (`powershell`/`pwsh`) calls in `getWindowsProcessCommandLine` can block the event loop for up to 2 seconds per PID; if this runs on the UI/critical path, consider moving command-line probing to an async/worker process or lowering the timeout and tightening when this path is invoked.
## Individual Comments
### Comment 1
<location> `desktop/lib/backend-manager.js:860-869` </location>
<code_context>
+ return null;
+ }
+
+ buildWindowsUnmanagedBackendMatcher(backendConfig) {
+ const expectedImageName = path.basename(backendConfig.cmd || 'python.exe').toLowerCase();
+ const requireStrictCommandLineCheck =
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against missing or incomplete backendConfig when building the unmanaged Windows backend matcher.
In `stopUnmanagedBackendByPort`, `this.getBackendConfig()` is passed straight into `buildWindowsUnmanagedBackendMatcher` without checking for `null`/`undefined` or expected shape. If `getBackendConfig()` ever returns a missing/partial config (e.g. no `cmd`/`cwd`/`args`), accessing `backendConfig.cmd`/`.cwd` will throw. Consider either early-returning from `stopUnmanagedBackendByPort` when the config is invalid, or having `buildWindowsUnmanagedBackendMatcher` handle missing/partial config (e.g. return `null`) and skipping cleanup in that case.
</issue_to_address>
### Comment 2
<location> `desktop/lib/backend-manager.js:136` </location>
<code_context>
- getPackagedBackendPath() {
- if (!this.app.isPackaged) {
+ getPackagedBackendDir() {
+ const packagedBackendConfig = this.loadPackagedBackendConfig();
+ return packagedBackendConfig ? packagedBackendConfig.backendDir : null;
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the new packaged-backend configuration and Windows process-matching helpers by inlining logic and consolidating small wrappers so the control flow is more linear and easier to scan.
- The new packaged-backend config helpers are very thin wrappers and widen the class surface. You can centralize config usage to keep the shape local and reduce method count.
```js
// Keep a single accessor
getPackagedBackendConfig() {
return this.loadPackagedBackendConfig();
}
// Use it at call sites instead of multiple tiny getters
resolveBackendCwd() {
if (!this.app.isPackaged) {
return path.resolve(this.baseDir, '..');
}
const cfg = this.getPackagedBackendConfig();
return (cfg && cfg.appDir) || this.resolveBackendRoot();
}
buildPackagedBackendLaunch(webuiDir) {
const cfg = this.getPackagedBackendConfig();
if (!cfg || !cfg.runtimePythonPath || !cfg.launchScriptPath) {
return null;
}
const args = [cfg.launchScriptPath];
if (webuiDir) args.push('--webui-dir', webuiDir);
return { cmd: cfg.runtimePythonPath, args, shell: false };
}
```
This lets you drop `getPackagedBackendManifest`, `getPackagedBackendAppDir`, `getPackagedBackendLaunchScriptPath`, `getPackagedRuntimePythonPath`, and keep config shape knowledge in `loadPackagedBackendConfig()` plus a single getter.
- `resolveManifestPath` + the three calls in `loadPackagedBackendConfig()` can be made more explicit instead of generic, which makes it easier to scan:
```js
loadPackagedBackendConfig() {
// ...
const manifest = this.parsePackagedBackendManifest(backendDir) || {};
const defaultPythonRelative =
process.platform === 'win32'
? path.join('python', 'Scripts', 'python.exe')
: path.join('python', 'bin', 'python3');
const resolve = (key, fallback) => {
const rel = (typeof manifest[key] === 'string' && manifest[key]) || fallback;
const candidate = path.join(backendDir, rel);
return fs.existsSync(candidate) ? candidate : null;
};
this.packagedBackendConfig = Object.freeze({
backendDir,
manifest: Object.keys(manifest).length ? manifest : null,
appDir: resolve('app', 'app'),
launchScriptPath: resolve('entrypoint', 'launch_backend.py'),
runtimePythonPath: resolve('python', defaultPythonRelative),
});
return this.packagedBackendConfig;
}
```
This keeps the logic in one place without a separate `resolveManifestPath` method.
- The PowerShell/Pwsh query helpers are almost identical; you can collapse them into one parameterized function and inline `parseWindowsProcessCommandLine` to reduce indirection:
```js
queryWindowsProcessCommandLine(pid, shellName) {
const query = `$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}"; if ($null -ne $p) { $p.CommandLine }`;
return spawnSync(
shellName,
['-NoProfile', '-NonInteractive', '-Command', query],
{
stdio: ['ignore', 'pipe', 'ignore'],
encoding: 'utf8',
windowsHide: true,
timeout: WINDOWS_PROCESS_QUERY_TIMEOUT_MS,
},
);
}
getWindowsProcessCommandLine(pid) {
const numericPid = Number.parseInt(`${pid}`, 10);
if (!Number.isInteger(numericPid)) return null;
const cached = this.windowsProcessCommandLineCache.get(numericPid);
if (cached && Date.now() - cached.timestampMs <= WINDOWS_PROCESS_COMMAND_LINE_CACHE_TTL_MS) {
return cached.commandLine;
}
this.windowsProcessCommandLineCache.delete(numericPid);
for (const shellName of ['powershell', 'pwsh']) {
let result;
try {
result = this.queryWindowsProcessCommandLine(numericPid, shellName);
} catch (error) {
// existing logging
continue;
}
// existing error/timeout handling...
if (result.status === 0 && result.stdout) {
const commandLine =
result.stdout
.split(/\r?\n/)
.map((s) => s.trim())
.find((s) => s.length > 0) || null;
this.windowsProcessCommandLineCache.set(numericPid, {
commandLine,
timestampMs: Date.now(),
});
return commandLine;
}
}
this.windowsProcessCommandLineCache.set(numericPid, { commandLine: null, timestampMs: Date.now() });
return null;
}
```
- The matcher object (`buildWindowsUnmanagedBackendMatcher` + `shouldKillUnmanagedBackendProcess`) is only used at one call site. You can make the flow more linear with a single function that does both the image and command-line checks:
```js
isUnmanagedBackendProcess(pid, processInfo, backendConfig) {
const expectedImageName = path.basename(backendConfig.cmd || 'python.exe').toLowerCase();
const actualImageName = processInfo.imageName.toLowerCase();
if (actualImageName !== expectedImageName) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unexpected process image ${processInfo.imageName}.`,
);
return false;
}
if (!this.isGenericWindowsPythonImage(expectedImageName)) {
return true;
}
const args = Array.isArray(backendConfig.args) ? backendConfig.args : [];
const primaryArg = args[0];
if (typeof primaryArg !== 'string' || !primaryArg) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: backend launch marker is unavailable.`,
);
return false;
}
const cwd = backendConfig.cwd || process.cwd();
const markers = [
this.normalizeWindowsPathForMatch(
path.isAbsolute(primaryArg) ? primaryArg : path.resolve(cwd, primaryArg),
),
this.normalizeWindowsPathForMatch(path.basename(primaryArg)),
];
const commandLine = this.getWindowsProcessCommandLine(pid);
if (!commandLine) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process command line.`,
);
return false;
}
const normalized = this.normalizeWindowsPathForMatch(commandLine);
const matched = markers.some((m) => m && normalized.includes(m));
if (!matched) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: command line does not match AstrBot backend launch marker.`,
);
return false;
}
return true;
}
async stopUnmanagedBackendByPort() {
// ...
const backendConfig = this.getBackendConfig();
for (const pid of pids) {
const processInfo = this.getWindowsProcessInfo(pid);
if (!processInfo) {
// existing log
continue;
}
if (!this.isUnmanagedBackendProcess(pid, processInfo, backendConfig)) {
continue;
}
// taskkill...
}
}
```
This removes the `processMatcher` object/type entirely while preserving all checks.
- Given `stopUnmanagedBackendByPort` is an infrequent operation, you may not need a long-lived cache with TTL and pruning. A simpler per-invocation cache (no timestamps, no pruning) keeps the behavior but removes global state:
```js
async stopUnmanagedBackendByPort() {
if (!this.app.isPackaged || process.platform !== 'win32') {
return false;
}
const commandLineCache = new Map(); // pid -> string|null
const getCommandLineOnce = (pid) => {
if (commandLineCache.has(pid)) return commandLineCache.get(pid);
const value = this.getWindowsProcessCommandLine(pid); // simplified version w/o TTL
commandLineCache.set(pid, value);
return value;
};
// use getCommandLineOnce inside isUnmanagedBackendProcess or inline it there
}
```
If you want to keep the existing TTL semantics, you can still narrow the cache to `stopUnmanagedBackendByPort` (pass a `getCommandLine` function into `isUnmanagedBackendProcess`) and remove `this.windowsProcessCommandLineCache` and `pruneWindowsProcessCommandLineCache`.
</issue_to_address>
### Comment 3
<location> `desktop/scripts/build-backend.mjs:32` </location>
<code_context>
+ process.exit(1);
+}
+
+const normalizePathForCompare = (targetPath) => {
+ const resolved = path.resolve(targetPath).replace(/[\\/]+$/, '');
+ return process.platform === 'win32' ? resolved.toLowerCase() : resolved;
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the new runtime validation helpers by collapsing overlapping functions and using clearer inline utilities for path overlap checks, version resolution, and Python validation to make the control flow more obvious and less error-prone.
- The path-overlap helpers add abstraction for a single safety check and currently have a logic bug (`if (! isSameOrSubPath(...))`). You can both simplify and fix this by using `path.relative` directly with a tiny inline helper:
```js
const pathsOverlap = (a, b) => {
const relA = path.relative(a, b);
const relB = path.relative(b, a);
const isSub = (rel) => rel === '' || (!rel.startsWith('..') && !path.isAbsolute(rel));
return isSub(relA) || isSub(relB);
};
if (pathsOverlap(runtimeSourceReal, outputDir)) {
console.error(
`CPython runtime source overlaps with backend output directory. ` +
`runtime=${runtimeSourceReal}, output=${outputDir}. ` +
'Please set ASTRBOT_DESKTOP_CPYTHON_HOME to a separate runtime directory.',
);
process.exit(1);
}
```
This removes `normalizePathForCompare` and `isSameOrSubPath` entirely and makes the intent (“do these paths overlap?”) immediately obvious.
- The version resolution pipeline can be made easier to follow by collapsing `parseExpectedRuntimeVersion` and `readProjectRequiresPythonLowerBound` into `resolveExpectedRuntimeVersion`, keeping only one helper in play:
```js
const resolveExpectedRuntimeVersion = () => {
const parse = (raw, sourceName) => {
const match = /^(\d+)\.(\d+)$/.exec(String(raw).trim());
if (!match) {
console.error(
`Invalid expected Python version from ${sourceName}: ${raw}. ` +
'Expected format <major>.<minor>.',
);
process.exit(1);
}
return { major: Number(match[1]), minor: Number(match[2]) };
};
if (process.env.ASTRBOT_DESKTOP_EXPECTED_PYTHON) {
return parse(process.env.ASTRBOT_DESKTOP_EXPECTED_PYTHON, 'ASTRBOT_DESKTOP_EXPECTED_PYTHON');
}
const pyprojectPath = path.join(rootDir, 'pyproject.toml');
if (fs.existsSync(pyprojectPath)) {
const content = fs.readFileSync(pyprojectPath, 'utf8');
const requires = /^\s*requires-python\s*=\s*"([^"]+)"/m.exec(content)?.[1];
const lower = requires && />=\s*(\d+)\.(\d+)/.exec(requires);
if (lower) {
return parse(`${lower[1]}.${lower[2]}`, 'pyproject.toml requires-python');
}
}
console.error(
'Unable to determine expected runtime Python version. ' +
'Set ASTRBOT_DESKTOP_EXPECTED_PYTHON or declare project.requires-python in pyproject.toml.',
);
process.exit(1);
};
```
That eliminates two globals and keeps all error handling in one place.
- `validateRuntimePython` can be simplified and have its comparison bug fixed by extracting the version comparison into a single obvious condition and reusing a common exit helper:
```js
const fatal = (msg) => {
console.error(msg);
process.exit(1);
};
const validateRuntimePython = (pythonExecutable) => {
const probe = spawnSync(
pythonExecutable,
['-c', 'import sys, pip; print(f"{sys.version_info.major}.{sys.version_info.minor}")'],
{
stdio: ['ignore', 'pipe', 'pipe'],
encoding: 'utf8',
windowsHide: true,
timeout: 5000,
},
);
if (probe.error) {
const reason =
probe.error.code === 'ETIMEDOUT'
? 'runtime Python probe timed out'
: probe.error.message || String(probe.error);
fatal(`Runtime Python probe failed: ${reason}`);
}
if (probe.status !== 0) {
const stderrText = (probe.stderr || '').trim();
fatal(
`Runtime Python probe failed with exit code ${probe.status}.` +
(stderrText ? ` stderr: ${stderrText}` : ''),
);
}
const versionMatch = /(\d+)\.(\d+)/.exec((probe.stdout || '').trim());
if (!versionMatch) {
fatal(
`Runtime Python probe did not report a valid version. Output: ${(probe.stdout || '').trim()}`,
);
}
const actual = { major: Number(versionMatch[1]), minor: Number(versionMatch[2]) };
if (actual.major !== expectedRuntimeVersion.major || actual.minor !== expectedRuntimeVersion.minor) {
fatal(
`Runtime Python version mismatch: expected ${expectedRuntimeVersion.major}.${expectedRuntimeVersion.minor}, ` +
`got ${actual.major}.${actual.minor}.`,
);
}
};
```
This removes the `! actualVersion.major !== ...` confusion and centralizes the `console.error`/`process.exit(1)` pattern, reducing branching noise.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
SourceryAI
reviewed
Feb 18, 2026
SourceryAI
left a comment
SourceryAI
left a comment
There was a problem hiding this comment.
Hey - 我发现了 3 个问题，并给出了一些整体层面的反馈：
- 在
BackendManager.stopUnmanagedBackendByPort中，buildWindowsUnmanagedBackendMatcher无条件地用this.getBackendConfig()调用，而如果当前会话从未启动过后端，这个值可能为null；建议在缺少配置时先做保护，并回退到一个更简单的基于镜像名的匹配方式，以避免在清理过程中出现运行时错误。 build-backend.mjs里的readProjectRequiresPythonLowerBound辅助函数通过正则解析pyproject.toml，并假设requires-python约束是简单形式；使用 TOML 解析器（或者至少处理更复杂的约束/分段）会让不同pyproject布局下的版本检测更健壮。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- In `BackendManager.stopUnmanagedBackendByPort`, `buildWindowsUnmanagedBackendMatcher` is called unconditionally with `this.getBackendConfig()`, which can be `null` if the backend was never started in the current session; consider guarding for a missing config and falling back to a simpler image-name-based match to avoid runtime errors during cleanup.
- The `readProjectRequiresPythonLowerBound` helper in `build-backend.mjs` parses `pyproject.toml` with regex and assumes a simple `requires-python` constraint; using a TOML parser (or at least handling more complex specifiers/sections) would make version detection more robust across different `pyproject` layouts.
## Individual Comments
### Comment 1
<location> `astrbot/core/utils/pip_installer.py:442-443` </location>
<code_context>
+ wheel_only: bool = False,
) -> None:
args = ["install"]
+ pip_install_args = (
+ shlex.split(self.pip_install_arg) if self.pip_install_arg else []
+ )
requested_requirements: set[str] = set()
</code_context>
<issue_to_address>
**question (bug_risk):** Consider whether changing from str.split to shlex.split for pip_install_arg might alter behavior for existing configurations.
shlex.split improves handling of quoted arguments but does change behavior versus .split(). Configs that depended on literal spaces or unusual quoting in pip_install_arg may now be parsed differently or even error. If backward compatibility is important, consider gating this behind a feature flag or catching shlex errors and falling back to .split(), with a warning.
</issue_to_address>
### Comment 2
<location> `desktop/lib/backend-manager.js:136` </location>
<code_context>
- getPackagedBackendPath() {
- if (!this.app.isPackaged) {
+ getPackagedBackendDir() {
+ const packagedBackendConfig = this.loadPackagedBackendConfig();
+ return packagedBackendConfig ? packagedBackendConfig.backendDir : null;
</code_context>
<issue_to_address>
**issue (complexity):** Consider inlining the small helper methods and collapsing duplicated logic to reduce indirection and make the backend config and Windows process-handling code easier to follow.
You can trim a fair bit of indirection here without changing behavior.
### 1. Packaged backend config: collapse trivial getters / helpers
You already have `loadPackagedBackendConfig` as a single source of truth; the tiny getters are just pass‑throughs and increase call depth.
Instead of:
```js
getPackagedBackendAppDir() {
const packagedBackendConfig = this.loadPackagedBackendConfig();
return packagedBackendConfig ? packagedBackendConfig.appDir : null;
}
resolveBackendCwd() {
if (!this.app.isPackaged) {
return path.resolve(this.baseDir, '..');
}
return this.getPackagedBackendAppDir() || this.resolveBackendRoot();
}
```
use the config object directly:
```js
resolveBackendCwd() {
if (!this.app.isPackaged) {
return path.resolve(this.baseDir, '..');
}
const cfg = this.loadPackagedBackendConfig();
return (cfg && cfg.appDir) || this.resolveBackendRoot();
}
```
and similarly in `buildPackagedBackendLaunch`:
```js
buildPackagedBackendLaunch(webuiDir) {
const cfg = this.loadPackagedBackendConfig();
if (!cfg || !cfg.runtimePythonPath || !cfg.launchScriptPath) {
return null;
}
const args = [cfg.launchScriptPath];
if (webuiDir) {
args.push('--webui-dir', webuiDir);
}
return {
cmd: cfg.runtimePythonPath,
args,
shell: false,
};
}
```
Then you can remove:
- `getPackagedBackendDir`
- `getPackagedBackendManifest`
- `getPackagedBackendAppDir`
- `getPackagedBackendLaunchScriptPath`
- `getPackagedRuntimePythonPath`
If `parsePackagedBackendManifest` / `resolveManifestPath` are only used in `loadPackagedBackendConfig`, consider inlining them as inner functions to keep the public surface smaller:
```js
loadPackagedBackendConfig() {
if (!this.app.isPackaged) return null;
if (this.packagedBackendConfig) return this.packagedBackendConfig;
const backendDir = path.join(process.resourcesPath, 'backend');
if (!fs.existsSync(backendDir)) return null;
const parseManifest = (dir) => { /* current body */ };
const resolveManifestPath = (dir, manifest, key, def) => { /* current body */ };
const manifest = parseManifest(backendDir);
const manifestForPathResolve = manifest || {};
// ... build frozen config as you already do ...
}
```
### 2. Windows process command line: unify PowerShell helpers and cache pruning
The two query functions are identical except for the executable name; you can replace both with one parametrized helper:
```js
queryWindowsProcessCommandLine(shellName, pid) {
const query =
`$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}";` +
' if ($null -ne $p) { $p.CommandLine }';
return spawnSync(
shellName,
['-NoProfile', '-NonInteractive', '-Command', query],
{
stdio: ['ignore', 'pipe', 'ignore'],
encoding: 'utf8',
windowsHide: true,
timeout: WINDOWS_PROCESS_QUERY_TIMEOUT_MS,
},
);
}
```
and simplify `getWindowsProcessCommandLine`:
```js
getWindowsProcessCommandLine(pid) {
const numericPid = Number.parseInt(`${pid}`, 10);
if (!Number.isInteger(numericPid)) return null;
const now = Date.now();
// prune old entries as you go
for (const [cachedPid, cached] of this.windowsProcessCommandLineCache.entries()) {
if (!cached || now - cached.timestampMs > WINDOWS_PROCESS_COMMAND_LINE_CACHE_TTL_MS) {
this.windowsProcessCommandLineCache.delete(cachedPid);
}
}
const cached = this.windowsProcessCommandLineCache.get(numericPid);
if (cached) return cached.commandLine;
const shells = ['powershell', 'pwsh'];
for (const shellName of shells) {
let result;
try {
result = this.queryWindowsProcessCommandLine(shellName, numericPid);
} catch (error) {
// existing logging
continue;
}
// existing error/timeout handling...
if (result.status === 0) {
const commandLine = this.parseWindowsProcessCommandLine(result);
this.windowsProcessCommandLineCache.set(numericPid, {
commandLine,
timestampMs: now,
});
return commandLine;
}
}
this.windowsProcessCommandLineCache.set(numericPid, {
commandLine: null,
timestampMs: now,
});
return null;
}
```
With this, you can drop:
- `queryWindowsProcessCommandLineByPowerShell`
- `queryWindowsProcessCommandLineByPwsh`
- `pruneWindowsProcessCommandLineCache`
- the explicit `this.pruneWindowsProcessCommandLineCache()` call in `stopUnmanagedBackendByPort`.
`parseWindowsProcessCommandLine` can also be made a small inner function in `getWindowsProcessCommandLine` if it’s not reused elsewhere.
### 3. Windows unmanaged backend matcher: inline the matcher object
`buildWindowsUnmanagedBackendMatcher` + `shouldKillUnmanagedBackendProcess` introduces an extra struct and flag that mostly serves a single call site. You can fold them into one clearly named helper, keeping behavior the same:
```js
isLikelyBackendProcess(pid, processInfo, backendConfig) {
const expectedImageName = path.basename(backendConfig.cmd || 'python.exe').toLowerCase();
const actualImageName = processInfo.imageName.toLowerCase();
if (actualImageName !== expectedImageName) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unexpected process image ${processInfo.imageName}.`,
);
return false;
}
if (!this.isGenericWindowsPythonImage(expectedImageName)) {
return true;
}
const args = Array.isArray(backendConfig.args) ? backendConfig.args : [];
const primaryArg = args[0];
if (typeof primaryArg !== 'string' || !primaryArg) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: backend launch marker is unavailable.`,
);
return false;
}
const resolvedPrimaryArg = path.isAbsolute(primaryArg)
? primaryArg
: path.resolve(backendConfig.cwd || process.cwd(), primaryArg);
const markers = [
this.normalizeWindowsPathForMatch(resolvedPrimaryArg),
this.normalizeWindowsPathForMatch(path.basename(primaryArg)),
];
const commandLine = this.getWindowsProcessCommandLine(pid);
if (!commandLine) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process command line.`,
);
return false;
}
const normalizedCommandLine = this.normalizeWindowsPathForMatch(commandLine);
const matched = markers.some(
(marker) => marker && normalizedCommandLine.includes(marker),
);
if (!matched) {
this.log(
'Skip unmanaged cleanup for pid=' +
pid +
': command line does not match AstrBot backend launch marker.',
);
return false;
}
return true;
}
```
Then `stopUnmanagedBackendByPort` becomes:
```js
async stopUnmanagedBackendByPort() {
if (!this.app.isPackaged || process.platform !== 'win32') {
return false;
}
const port = this.getBackendPort();
if (!port) return false;
const pids = this.findListeningPidsOnWindows(port);
if (!pids.length) return false;
this.log(
`Attempting unmanaged backend cleanup by port=${port} pids=${pids.join(',')}`,
);
const backendConfig = this.getBackendConfig();
for (const pid of pids) {
const processInfo = this.getWindowsProcessInfo(pid);
if (!processInfo) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process info.`,
);
continue;
}
if (!this.isLikelyBackendProcess(pid, processInfo, backendConfig)) {
continue;
}
// existing taskkill call...
}
}
```
This keeps the safety semantics (image check + generic-python command‑line markers) but removes the extra `processMatcher` layer and boolean flag, making the flow easier to follow from `stopUnmanagedBackendByPort` down.
</issue_to_address>
### Comment 3
<location> `desktop/scripts/build-backend.mjs:14` </location>
<code_context>
+const manifestPath = path.join(outputDir, 'runtime-manifest.json');
+const launcherPath = path.join(outputDir, 'launch_backend.py');
+
+const runtimeSource =
+ process.env.ASTRBOT_DESKTOP_CPYTHON_HOME ||
+ process.env.ASTRBOT_DESKTOP_BACKEND_RUNTIME;
</code_context>
<issue_to_address>
**issue (complexity):** Consider centralizing error handling and organizing the script around a small `main()`-style orchestration with cohesive helpers to make the control flow clearer and less repetitive.
You can reduce complexity without changing behavior by (1) consolidating error handling and (2) grouping the high‑level flow into a `main()` plus a couple of cohesive helpers.
### 1. Centralize error handling
Right now many helpers call `console.error(...); process.exit(1);`, which scatters control flow. Instead, throw `Error` from helpers and handle `process.exit` in one place:
```js
const fail = (message) => new Error(message);
const parseExpectedRuntimeVersion = (rawVersion, sourceName) => {
const match = /^(\d+)\.(\d+)$/.exec(String(rawVersion).trim());
if (!match) {
throw fail(
`Invalid expected Python version from ${sourceName}: ${rawVersion}. ` +
'Expected format <major>.<minor>.'
);
}
return {
major: Number.parseInt(match[1], 10),
minor: Number.parseInt(match[2], 10),
};
};
// example: replace process.exit in one of the callers
const readProjectRequiresPythonLowerBound = () => {
const pyprojectPath = path.join(rootDir, 'pyproject.toml');
if (!fs.existsSync(pyprojectPath)) {
return null;
}
const content = fs.readFileSync(pyprojectPath, 'utf8');
const requiresPythonMatch = /^\s*requires-python\s*=\s*"([^"]+)"/m.exec(content);
if (!requiresPythonMatch) {
return null;
}
const lowerBoundMatch = />=\s*(\d+)\.(\d+)/.exec(requiresPythonMatch[1]);
if (!lowerBoundMatch) {
return null;
}
return `${lowerBoundMatch[1]}.${lowerBoundMatch[2]}`;
};
```
Top-level:
```js
const main = () => {
const runtimeSourceReal = resolveAndValidateRuntimeSource();
const expectedRuntimeVersion = resolveExpectedRuntimeVersion();
prepareBackendAppTree();
const runtimePython = prepareRuntimeTree(runtimeSourceReal, expectedRuntimeVersion);
writeLauncherAndManifest(runtimePython);
console.log(`Prepared CPython backend runtime in ${outputDir}`);
};
main();
```
with a single exit point:
```js
try {
main();
} catch (err) {
console.error(err instanceof Error ? err.message : String(err));
process.exit(1);
}
```
This keeps error behavior the same (message + exit 1) but removes repeated `process.exit` and makes control flow clearer.
### 2. Group high‑level responsibilities
You already have good low‑level helpers (`copyTree`, `resolveRuntimePython`, etc.), but the script still reads as a long, linear sequence. You can wrap existing logic into a few cohesive helpers without changing what they do.
For example:
```js
const resolveAndValidateRuntimeSource = () => {
const runtimeSource =
process.env.ASTRBOT_DESKTOP_CPYTHON_HOME ||
process.env.ASTRBOT_DESKTOP_BACKEND_RUNTIME;
if (!runtimeSource) {
throw fail(
'Missing CPython runtime source. Set ASTRBOT_DESKTOP_CPYTHON_HOME ' +
'(recommended) or ASTRBOT_DESKTOP_BACKEND_RUNTIME.'
);
}
const runtimeSourceReal = path.resolve(rootDir, runtimeSource);
if (!fs.existsSync(runtimeSourceReal)) {
throw fail(`CPython runtime source does not exist: ${runtimeSourceReal}`);
}
assertNoOverlap(runtimeSourceReal, outputDir);
return runtimeSourceReal;
};
const prepareBackendAppTree = () => {
fs.rmSync(outputDir, { recursive: true, force: true });
fs.mkdirSync(outputDir, { recursive: true });
fs.mkdirSync(appDir, { recursive: true });
for (const [srcRelative, destRelative] of sourceEntries) {
const sourcePath = path.join(rootDir, srcRelative);
const targetPath = path.join(appDir, destRelative);
if (!fs.existsSync(sourcePath)) {
throw fail(`Backend source path does not exist: ${sourcePath}`);
}
copyTree(sourcePath, targetPath);
}
};
```
And encapsulate the runtime copy + validation:
```js
const prepareRuntimeTree = (runtimeSourceReal, expectedRuntimeVersion) => {
const sourceRuntimePython = resolveRuntimePython(runtimeSourceReal);
if (!sourceRuntimePython) {
throw fail(
`Cannot find Python executable in runtime source: ${runtimeSourceReal}. ` +
'Expected python under bin/ or Scripts/.'
);
}
validateRuntimePython(sourceRuntimePython.absolute, expectedRuntimeVersion);
copyTree(runtimeSourceReal, runtimeDir, { dereference: true });
const runtimePython = resolveRuntimePython(runtimeDir);
if (!runtimePython) {
throw fail(
`Cannot find Python executable in runtime: ${runtimeDir}. ` +
'Expected python under bin/ or Scripts/.'
);
}
return runtimePython;
};
```
### 3. Simplify the overlap check
`normalizePathForCompare` and `isSameOrSubPath` are only used once; wrapping the whole concern in a single helper makes the intent clearer:
```js
const assertNoOverlap = (runtimeSourceReal, outputDir) => {
const normalize = (p) => {
const resolved = path.resolve(p).replace(/[\\/]+$/, '');
return process.platform === 'win32' ? resolved.toLowerCase() : resolved;
};
const sourceNorm = normalize(runtimeSourceReal);
const outNorm = normalize(outputDir);
const isSub = (a, b) => a === b || a.startsWith(`${b}${path.sep}`);
if (isSub(sourceNorm, outNorm) || isSub(outNorm, sourceNorm)) {
throw fail(
`CPython runtime source overlaps with backend output directory. ` +
`runtime=${runtimeSourceReal}, output=${outputDir}. ` +
'Please set ASTRBOT_DESKTOP_CPYTHON_HOME to a separate runtime directory.'
);
}
};
```
This keeps all functionality but reduces the number of top‑level “utility” concepts and makes the top-level flow (`main`) much easier to read.
</issue_to_address>Hi @zouyonghe! 👋
感谢你通过评论 @sourcery-ai review 来体验 Sourcery！🚀
安装 sourcery-ai bot 来在每个 Pull Request 上自动获取代码审查 ✨
帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据这些反馈来改进后续的审查。Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
- In
BackendManager.stopUnmanagedBackendByPort,buildWindowsUnmanagedBackendMatcheris called unconditionally withthis.getBackendConfig(), which can benullif the backend was never started in the current session; consider guarding for a missing config and falling back to a simpler image-name-based match to avoid runtime errors during cleanup. - The
readProjectRequiresPythonLowerBoundhelper inbuild-backend.mjsparsespyproject.tomlwith regex and assumes a simplerequires-pythonconstraint; using a TOML parser (or at least handling more complex specifiers/sections) would make version detection more robust across differentpyprojectlayouts.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `BackendManager.stopUnmanagedBackendByPort`, `buildWindowsUnmanagedBackendMatcher` is called unconditionally with `this.getBackendConfig()`, which can be `null` if the backend was never started in the current session; consider guarding for a missing config and falling back to a simpler image-name-based match to avoid runtime errors during cleanup.
- The `readProjectRequiresPythonLowerBound` helper in `build-backend.mjs` parses `pyproject.toml` with regex and assumes a simple `requires-python` constraint; using a TOML parser (or at least handling more complex specifiers/sections) would make version detection more robust across different `pyproject` layouts.
## Individual Comments
### Comment 1
<location> `astrbot/core/utils/pip_installer.py:442-443` </location>
<code_context>
+ wheel_only: bool = False,
) -> None:
args = ["install"]
+ pip_install_args = (
+ shlex.split(self.pip_install_arg) if self.pip_install_arg else []
+ )
requested_requirements: set[str] = set()
</code_context>
<issue_to_address>
**question (bug_risk):** Consider whether changing from str.split to shlex.split for pip_install_arg might alter behavior for existing configurations.
shlex.split improves handling of quoted arguments but does change behavior versus .split(). Configs that depended on literal spaces or unusual quoting in pip_install_arg may now be parsed differently or even error. If backward compatibility is important, consider gating this behind a feature flag or catching shlex errors and falling back to .split(), with a warning.
</issue_to_address>
### Comment 2
<location> `desktop/lib/backend-manager.js:136` </location>
<code_context>
- getPackagedBackendPath() {
- if (!this.app.isPackaged) {
+ getPackagedBackendDir() {
+ const packagedBackendConfig = this.loadPackagedBackendConfig();
+ return packagedBackendConfig ? packagedBackendConfig.backendDir : null;
</code_context>
<issue_to_address>
**issue (complexity):** Consider inlining the small helper methods and collapsing duplicated logic to reduce indirection and make the backend config and Windows process-handling code easier to follow.
You can trim a fair bit of indirection here without changing behavior.
### 1. Packaged backend config: collapse trivial getters / helpers
You already have `loadPackagedBackendConfig` as a single source of truth; the tiny getters are just pass‑throughs and increase call depth.
Instead of:
```js
getPackagedBackendAppDir() {
const packagedBackendConfig = this.loadPackagedBackendConfig();
return packagedBackendConfig ? packagedBackendConfig.appDir : null;
}
resolveBackendCwd() {
if (!this.app.isPackaged) {
return path.resolve(this.baseDir, '..');
}
return this.getPackagedBackendAppDir() || this.resolveBackendRoot();
}
```
use the config object directly:
```js
resolveBackendCwd() {
if (!this.app.isPackaged) {
return path.resolve(this.baseDir, '..');
}
const cfg = this.loadPackagedBackendConfig();
return (cfg && cfg.appDir) || this.resolveBackendRoot();
}
```
and similarly in `buildPackagedBackendLaunch`:
```js
buildPackagedBackendLaunch(webuiDir) {
const cfg = this.loadPackagedBackendConfig();
if (!cfg || !cfg.runtimePythonPath || !cfg.launchScriptPath) {
return null;
}
const args = [cfg.launchScriptPath];
if (webuiDir) {
args.push('--webui-dir', webuiDir);
}
return {
cmd: cfg.runtimePythonPath,
args,
shell: false,
};
}
```
Then you can remove:
- `getPackagedBackendDir`
- `getPackagedBackendManifest`
- `getPackagedBackendAppDir`
- `getPackagedBackendLaunchScriptPath`
- `getPackagedRuntimePythonPath`
If `parsePackagedBackendManifest` / `resolveManifestPath` are only used in `loadPackagedBackendConfig`, consider inlining them as inner functions to keep the public surface smaller:
```js
loadPackagedBackendConfig() {
if (!this.app.isPackaged) return null;
if (this.packagedBackendConfig) return this.packagedBackendConfig;
const backendDir = path.join(process.resourcesPath, 'backend');
if (!fs.existsSync(backendDir)) return null;
const parseManifest = (dir) => { /* current body */ };
const resolveManifestPath = (dir, manifest, key, def) => { /* current body */ };
const manifest = parseManifest(backendDir);
const manifestForPathResolve = manifest || {};
// ... build frozen config as you already do ...
}
```
### 2. Windows process command line: unify PowerShell helpers and cache pruning
The two query functions are identical except for the executable name; you can replace both with one parametrized helper:
```js
queryWindowsProcessCommandLine(shellName, pid) {
const query =
`$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}";` +
' if ($null -ne $p) { $p.CommandLine }';
return spawnSync(
shellName,
['-NoProfile', '-NonInteractive', '-Command', query],
{
stdio: ['ignore', 'pipe', 'ignore'],
encoding: 'utf8',
windowsHide: true,
timeout: WINDOWS_PROCESS_QUERY_TIMEOUT_MS,
},
);
}
```
and simplify `getWindowsProcessCommandLine`:
```js
getWindowsProcessCommandLine(pid) {
const numericPid = Number.parseInt(`${pid}`, 10);
if (!Number.isInteger(numericPid)) return null;
const now = Date.now();
// prune old entries as you go
for (const [cachedPid, cached] of this.windowsProcessCommandLineCache.entries()) {
if (!cached || now - cached.timestampMs > WINDOWS_PROCESS_COMMAND_LINE_CACHE_TTL_MS) {
this.windowsProcessCommandLineCache.delete(cachedPid);
}
}
const cached = this.windowsProcessCommandLineCache.get(numericPid);
if (cached) return cached.commandLine;
const shells = ['powershell', 'pwsh'];
for (const shellName of shells) {
let result;
try {
result = this.queryWindowsProcessCommandLine(shellName, numericPid);
} catch (error) {
// existing logging
continue;
}
// existing error/timeout handling...
if (result.status === 0) {
const commandLine = this.parseWindowsProcessCommandLine(result);
this.windowsProcessCommandLineCache.set(numericPid, {
commandLine,
timestampMs: now,
});
return commandLine;
}
}
this.windowsProcessCommandLineCache.set(numericPid, {
commandLine: null,
timestampMs: now,
});
return null;
}
```
With this, you can drop:
- `queryWindowsProcessCommandLineByPowerShell`
- `queryWindowsProcessCommandLineByPwsh`
- `pruneWindowsProcessCommandLineCache`
- the explicit `this.pruneWindowsProcessCommandLineCache()` call in `stopUnmanagedBackendByPort`.
`parseWindowsProcessCommandLine` can also be made a small inner function in `getWindowsProcessCommandLine` if it’s not reused elsewhere.
### 3. Windows unmanaged backend matcher: inline the matcher object
`buildWindowsUnmanagedBackendMatcher` + `shouldKillUnmanagedBackendProcess` introduces an extra struct and flag that mostly serves a single call site. You can fold them into one clearly named helper, keeping behavior the same:
```js
isLikelyBackendProcess(pid, processInfo, backendConfig) {
const expectedImageName = path.basename(backendConfig.cmd || 'python.exe').toLowerCase();
const actualImageName = processInfo.imageName.toLowerCase();
if (actualImageName !== expectedImageName) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unexpected process image ${processInfo.imageName}.`,
);
return false;
}
if (!this.isGenericWindowsPythonImage(expectedImageName)) {
return true;
}
const args = Array.isArray(backendConfig.args) ? backendConfig.args : [];
const primaryArg = args[0];
if (typeof primaryArg !== 'string' || !primaryArg) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: backend launch marker is unavailable.`,
);
return false;
}
const resolvedPrimaryArg = path.isAbsolute(primaryArg)
? primaryArg
: path.resolve(backendConfig.cwd || process.cwd(), primaryArg);
const markers = [
this.normalizeWindowsPathForMatch(resolvedPrimaryArg),
this.normalizeWindowsPathForMatch(path.basename(primaryArg)),
];
const commandLine = this.getWindowsProcessCommandLine(pid);
if (!commandLine) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process command line.`,
);
return false;
}
const normalizedCommandLine = this.normalizeWindowsPathForMatch(commandLine);
const matched = markers.some(
(marker) => marker && normalizedCommandLine.includes(marker),
);
if (!matched) {
this.log(
'Skip unmanaged cleanup for pid=' +
pid +
': command line does not match AstrBot backend launch marker.',
);
return false;
}
return true;
}
```
Then `stopUnmanagedBackendByPort` becomes:
```js
async stopUnmanagedBackendByPort() {
if (!this.app.isPackaged || process.platform !== 'win32') {
return false;
}
const port = this.getBackendPort();
if (!port) return false;
const pids = this.findListeningPidsOnWindows(port);
if (!pids.length) return false;
this.log(
`Attempting unmanaged backend cleanup by port=${port} pids=${pids.join(',')}`,
);
const backendConfig = this.getBackendConfig();
for (const pid of pids) {
const processInfo = this.getWindowsProcessInfo(pid);
if (!processInfo) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process info.`,
);
continue;
}
if (!this.isLikelyBackendProcess(pid, processInfo, backendConfig)) {
continue;
}
// existing taskkill call...
}
}
```
This keeps the safety semantics (image check + generic-python command‑line markers) but removes the extra `processMatcher` layer and boolean flag, making the flow easier to follow from `stopUnmanagedBackendByPort` down.
</issue_to_address>
### Comment 3
<location> `desktop/scripts/build-backend.mjs:14` </location>
<code_context>
+const manifestPath = path.join(outputDir, 'runtime-manifest.json');
+const launcherPath = path.join(outputDir, 'launch_backend.py');
+
+const runtimeSource =
+ process.env.ASTRBOT_DESKTOP_CPYTHON_HOME ||
+ process.env.ASTRBOT_DESKTOP_BACKEND_RUNTIME;
</code_context>
<issue_to_address>
**issue (complexity):** Consider centralizing error handling and organizing the script around a small `main()`-style orchestration with cohesive helpers to make the control flow clearer and less repetitive.
You can reduce complexity without changing behavior by (1) consolidating error handling and (2) grouping the high‑level flow into a `main()` plus a couple of cohesive helpers.
### 1. Centralize error handling
Right now many helpers call `console.error(...); process.exit(1);`, which scatters control flow. Instead, throw `Error` from helpers and handle `process.exit` in one place:
```js
const fail = (message) => new Error(message);
const parseExpectedRuntimeVersion = (rawVersion, sourceName) => {
const match = /^(\d+)\.(\d+)$/.exec(String(rawVersion).trim());
if (!match) {
throw fail(
`Invalid expected Python version from ${sourceName}: ${rawVersion}. ` +
'Expected format <major>.<minor>.'
);
}
return {
major: Number.parseInt(match[1], 10),
minor: Number.parseInt(match[2], 10),
};
};
// example: replace process.exit in one of the callers
const readProjectRequiresPythonLowerBound = () => {
const pyprojectPath = path.join(rootDir, 'pyproject.toml');
if (!fs.existsSync(pyprojectPath)) {
return null;
}
const content = fs.readFileSync(pyprojectPath, 'utf8');
const requiresPythonMatch = /^\s*requires-python\s*=\s*"([^"]+)"/m.exec(content);
if (!requiresPythonMatch) {
return null;
}
const lowerBoundMatch = />=\s*(\d+)\.(\d+)/.exec(requiresPythonMatch[1]);
if (!lowerBoundMatch) {
return null;
}
return `${lowerBoundMatch[1]}.${lowerBoundMatch[2]}`;
};
```
Top-level:
```js
const main = () => {
const runtimeSourceReal = resolveAndValidateRuntimeSource();
const expectedRuntimeVersion = resolveExpectedRuntimeVersion();
prepareBackendAppTree();
const runtimePython = prepareRuntimeTree(runtimeSourceReal, expectedRuntimeVersion);
writeLauncherAndManifest(runtimePython);
console.log(`Prepared CPython backend runtime in ${outputDir}`);
};
main();
```
with a single exit point:
```js
try {
main();
} catch (err) {
console.error(err instanceof Error ? err.message : String(err));
process.exit(1);
}
```
This keeps error behavior the same (message + exit 1) but removes repeated `process.exit` and makes control flow clearer.
### 2. Group high‑level responsibilities
You already have good low‑level helpers (`copyTree`, `resolveRuntimePython`, etc.), but the script still reads as a long, linear sequence. You can wrap existing logic into a few cohesive helpers without changing what they do.
For example:
```js
const resolveAndValidateRuntimeSource = () => {
const runtimeSource =
process.env.ASTRBOT_DESKTOP_CPYTHON_HOME ||
process.env.ASTRBOT_DESKTOP_BACKEND_RUNTIME;
if (!runtimeSource) {
throw fail(
'Missing CPython runtime source. Set ASTRBOT_DESKTOP_CPYTHON_HOME ' +
'(recommended) or ASTRBOT_DESKTOP_BACKEND_RUNTIME.'
);
}
const runtimeSourceReal = path.resolve(rootDir, runtimeSource);
if (!fs.existsSync(runtimeSourceReal)) {
throw fail(`CPython runtime source does not exist: ${runtimeSourceReal}`);
}
assertNoOverlap(runtimeSourceReal, outputDir);
return runtimeSourceReal;
};
const prepareBackendAppTree = () => {
fs.rmSync(outputDir, { recursive: true, force: true });
fs.mkdirSync(outputDir, { recursive: true });
fs.mkdirSync(appDir, { recursive: true });
for (const [srcRelative, destRelative] of sourceEntries) {
const sourcePath = path.join(rootDir, srcRelative);
const targetPath = path.join(appDir, destRelative);
if (!fs.existsSync(sourcePath)) {
throw fail(`Backend source path does not exist: ${sourcePath}`);
}
copyTree(sourcePath, targetPath);
}
};
```
And encapsulate the runtime copy + validation:
```js
const prepareRuntimeTree = (runtimeSourceReal, expectedRuntimeVersion) => {
const sourceRuntimePython = resolveRuntimePython(runtimeSourceReal);
if (!sourceRuntimePython) {
throw fail(
`Cannot find Python executable in runtime source: ${runtimeSourceReal}. ` +
'Expected python under bin/ or Scripts/.'
);
}
validateRuntimePython(sourceRuntimePython.absolute, expectedRuntimeVersion);
copyTree(runtimeSourceReal, runtimeDir, { dereference: true });
const runtimePython = resolveRuntimePython(runtimeDir);
if (!runtimePython) {
throw fail(
`Cannot find Python executable in runtime: ${runtimeDir}. ` +
'Expected python under bin/ or Scripts/.'
);
}
return runtimePython;
};
```
### 3. Simplify the overlap check
`normalizePathForCompare` and `isSameOrSubPath` are only used once; wrapping the whole concern in a single helper makes the intent clearer:
```js
const assertNoOverlap = (runtimeSourceReal, outputDir) => {
const normalize = (p) => {
const resolved = path.resolve(p).replace(/[\\/]+$/, '');
return process.platform === 'win32' ? resolved.toLowerCase() : resolved;
};
const sourceNorm = normalize(runtimeSourceReal);
const outNorm = normalize(outputDir);
const isSub = (a, b) => a === b || a.startsWith(`${b}${path.sep}`);
if (isSub(sourceNorm, outNorm) || isSub(outNorm, sourceNorm)) {
throw fail(
`CPython runtime source overlaps with backend output directory. ` +
`runtime=${runtimeSourceReal}, output=${outputDir}. ` +
'Please set ASTRBOT_DESKTOP_CPYTHON_HOME to a separate runtime directory.'
);
}
};
```
This keeps all functionality but reduces the number of top‑level “utility” concepts and makes the top-level flow (`main`) much easier to read.
</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.
Contributor
There was a problem hiding this comment.
你好——我这边发现了 3 个问题，并留下了一些高层面的反馈：
- 在
desktop/scripts/build-backend.mjs中，即使是从requires-python >=x.y推导出来的预期 Python 运行时版本，当前也被强制要求精确匹配<major>.<minor>。建议将其放宽为接受>=下限的运行时版本（同时仍然拒绝不支持的主版本），这样就不会因为较新的、实际上可用的次版本导致构建不必要地失败。 - 目前在
build-backend.mjs中对pyproject.toml的解析是通过对原始文本使用正则实现的；如果改用 TOML 解析器（例如通过一个短小的辅助脚本调用tomllib，或使用 Node 的 TOML 库），会让requires-python的检测对格式变化和新增元数据更具鲁棒性。 - Windows 上的非托管后端清理现在会针对每个 PID 同步运行 PowerShell/Pwsh 命令以查询命令行；如果存在大量 PID，可能会让 Electron 主进程在可感知的时间内卡住，因此建议在候选过多时短路跳出，或者将这部分逻辑移动到单独的进程/worker 中。
面向 AI 代理的提示词
Please address the comments from this code review:
## Overall Comments
- In `desktop/scripts/build-backend.mjs`, the expected runtime Python version is enforced as an exact `<major>.<minor>` match even when derived from `requires-python >=x.y`; consider relaxing this to accept runtimes `>=` the lower bound (while still rejecting unsupported majors) so that a valid newer minor version doesn't cause the build to fail unnecessarily.
- The current `pyproject.toml` parsing in `build-backend.mjs` uses regex on raw text; using a TOML parser (e.g., `tomllib` via a short helper script or a Node TOML library) would make `requires-python` detection more robust to formatting changes and additional metadata.
- The Windows unmanaged backend cleanup now runs synchronous PowerShell/Pwsh commands per PID to query command lines; if many PIDs are present this could freeze the Electron main process for noticeable time, so consider short‑circuiting when there are many candidates or moving this logic to a separate process/worker.
## Individual Comments
### Comment 1
<location> `desktop/README.md:84` </location>
<code_context>
+ app/ # AstrBot backend source snapshot used in packaged mode
+ python/ # Bundled CPython runtime directory
+ launch_backend.py # Launcher executed by Electron
+ runtime-manifest.json # Runtime metadata (python path, entrypoint, app path)
+```
+
</code_context>
<issue_to_address>
**nitpick (typo):** Consider capitalizing "Python" in the inline comment for consistency.
To match the rest of the documentation (e.g., CPython, runtime Python), please change "python path" to "Python path" in this comment.
```suggestion
runtime-manifest.json # Runtime metadata (Python path, entrypoint, app path)
```
</issue_to_address>
### Comment 2
<location> `desktop/README.md:87` </location>
<code_context>
+ runtime-manifest.json # Runtime metadata (python path, entrypoint, app path)
+```
+
+Electron reads `runtime-manifest.json` and starts backend with:
+- `python` from `python/`
+- `launch_backend.py` as entrypoint
</code_context>
<issue_to_address>
**nitpick (typo):** Minor grammar tweak: add "the" before "backend".
Suggested wording: "Electron reads `runtime-manifest.json` and starts the backend with:"
```suggestion
Electron reads `runtime-manifest.json` and starts the backend with:
```
</issue_to_address>
### Comment 3
<location> `desktop/lib/backend-manager.js:742` </location>
<code_context>
+ );
+ }
+
+ queryWindowsProcessCommandLineByPowerShell(pid) {
+ const query = `$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}"; if ($null -ne $p) { $p.CommandLine }`;
+ return spawnSync(
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the new Windows process-handling helpers (PowerShell querying, command-line cache, and matcher) to reduce duplication and indirection while preserving behavior.
You can trim a fair bit of structural complexity without changing behavior by:
---
### 1. Unifying PowerShell / pwsh command‑line querying
`queryWindowsProcessCommandLineByPowerShell` and `queryWindowsProcessCommandLineByPwsh` are identical except for the binary name. You can collapse them into a single helper and simplify `getWindowsProcessCommandLine`’s `queryAttempts`:
```js
queryWindowsProcessCommandLine(shellName, pid) {
const query =
`$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}"; ` +
'if ($null -ne $p) { $p.CommandLine }';
return spawnSync(
shellName,
['-NoProfile', '-NonInteractive', '-Command', query],
{
stdio: ['ignore', 'pipe', 'ignore'],
encoding: 'utf8',
windowsHide: true,
timeout: WINDOWS_PROCESS_QUERY_TIMEOUT_MS,
},
);
}
```
Then:
```js
const queryAttempts = [
{ shellName: 'powershell' },
{ shellName: 'pwsh' },
];
for (const { shellName } of queryAttempts) {
let result;
try {
result = this.queryWindowsProcessCommandLine(shellName, numericPid);
} catch (error) {
if (error instanceof Error && error.message) {
this.log(
`Failed to query process command line by ${shellName} for pid=${numericPid}: ${error.message}`,
);
}
continue;
}
// ... existing ENOENT / ETIMEDOUT / status handling ...
}
```
This keeps all current behavior while reducing duplication and the surface area you need to touch for future changes (e.g., timeouts or flags).
---
### 2. Simplifying the command‑line cache
Right now you have:
- TTL on each entry,
- explicit pruning on every call,
- and negative caching (`commandLine: null`).
Given this is only used during unmanaged cleanup, you can simplify the cache without changing externally observable behavior:
- Drop `pruneWindowsProcessCommandLineCache` and the per‑call prune.
- Cache only successful lookups.
- Treat failures as “no cache entry” so they can be retried if needed.
Example:
```js
getWindowsProcessCommandLine(pid) {
const numericPid = Number.parseInt(`${pid}`, 10);
if (!Number.isInteger(numericPid)) {
return null;
}
const cached = this.windowsProcessCommandLineCache.get(numericPid);
if (cached) {
return cached; // just the command line string
}
const queryAttempts = [
{ shellName: 'powershell' },
{ shellName: 'pwsh' },
];
for (const { shellName } of queryAttempts) {
let result;
try {
result = this.queryWindowsProcessCommandLine(shellName, numericPid);
} catch (error) {
if (error instanceof Error && error.message) {
this.log(
`Failed to query process command line by ${shellName} for pid=${numericPid}: ${error.message}`,
);
}
continue;
}
if (result.error && result.error.code === 'ENOENT') {
continue;
}
if (result.error && result.error.code === 'ETIMEDOUT') {
this.log(
`Timed out (${WINDOWS_PROCESS_QUERY_TIMEOUT_MS}ms) querying process command line by ${shellName} for pid=${numericPid}.`,
);
continue;
}
if (result.status === 0) {
const commandLine = this.parseWindowsProcessCommandLine(result);
if (commandLine) {
this.windowsProcessCommandLineCache.set(numericPid, commandLine);
}
return commandLine;
}
}
return null;
}
```
This removes timestamp/TTL bookkeeping and negative entries while preserving semantics: callers still get `null` for “cannot resolve command line,” but the internal state is much easier to reason about.
---
### 3. Inlining the matcher to avoid an extra abstraction layer (optional)
If you want to further reduce indirection, you can compute the matcher fields directly inside `shouldKillUnmanagedBackendProcess` and pass `backendConfig` instead of a separate matcher object.
For example, instead of:
```js
const backendConfig = this.getBackendConfig();
const processMatcher = this.buildWindowsUnmanagedBackendMatcher(backendConfig);
if (!this.shouldKillUnmanagedBackendProcess(pid, processInfo, processMatcher)) {
continue;
}
```
You can make `shouldKillUnmanagedBackendProcess` accept `backendConfig` and build what it needs inside:
```js
shouldKillUnmanagedBackendProcess(pid, processInfo, backendConfig) {
const expectedImageName = path.basename(backendConfig.cmd || 'python.exe').toLowerCase();
const actualImageName = processInfo.imageName.toLowerCase();
if (actualImageName !== expectedImageName) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unexpected process image ${processInfo.imageName}.`,
);
return false;
}
const requireStrictCommandLineCheck =
this.isGenericWindowsPythonImage(expectedImageName);
if (!requireStrictCommandLineCheck) {
return true;
}
// ...build markers from backendConfig.args here and run the existing checks...
}
```
This keeps behavior intact but removes the extra “matcher” type and its construction step, making the control flow easier to follow from `stopUnmanagedBackendByPort` downwards.
</issue_to_address>帮我变得更加有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进之后的 Review。
Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
- In
desktop/scripts/build-backend.mjs, the expected runtime Python version is enforced as an exact<major>.<minor>match even when derived fromrequires-python >=x.y; consider relaxing this to accept runtimes>=the lower bound (while still rejecting unsupported majors) so that a valid newer minor version doesn't cause the build to fail unnecessarily. - The current
pyproject.tomlparsing inbuild-backend.mjsuses regex on raw text; using a TOML parser (e.g.,tomllibvia a short helper script or a Node TOML library) would makerequires-pythondetection more robust to formatting changes and additional metadata. - The Windows unmanaged backend cleanup now runs synchronous PowerShell/Pwsh commands per PID to query command lines; if many PIDs are present this could freeze the Electron main process for noticeable time, so consider short‑circuiting when there are many candidates or moving this logic to a separate process/worker.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `desktop/scripts/build-backend.mjs`, the expected runtime Python version is enforced as an exact `<major>.<minor>` match even when derived from `requires-python >=x.y`; consider relaxing this to accept runtimes `>=` the lower bound (while still rejecting unsupported majors) so that a valid newer minor version doesn't cause the build to fail unnecessarily.
- The current `pyproject.toml` parsing in `build-backend.mjs` uses regex on raw text; using a TOML parser (e.g., `tomllib` via a short helper script or a Node TOML library) would make `requires-python` detection more robust to formatting changes and additional metadata.
- The Windows unmanaged backend cleanup now runs synchronous PowerShell/Pwsh commands per PID to query command lines; if many PIDs are present this could freeze the Electron main process for noticeable time, so consider short‑circuiting when there are many candidates or moving this logic to a separate process/worker.
## Individual Comments
### Comment 1
<location> `desktop/README.md:84` </location>
<code_context>
+ app/ # AstrBot backend source snapshot used in packaged mode
+ python/ # Bundled CPython runtime directory
+ launch_backend.py # Launcher executed by Electron
+ runtime-manifest.json # Runtime metadata (python path, entrypoint, app path)
+```
+
</code_context>
<issue_to_address>
**nitpick (typo):** Consider capitalizing "Python" in the inline comment for consistency.
To match the rest of the documentation (e.g., CPython, runtime Python), please change "python path" to "Python path" in this comment.
```suggestion
runtime-manifest.json # Runtime metadata (Python path, entrypoint, app path)
```
</issue_to_address>
### Comment 2
<location> `desktop/README.md:87` </location>
<code_context>
+ runtime-manifest.json # Runtime metadata (python path, entrypoint, app path)
+```
+
+Electron reads `runtime-manifest.json` and starts backend with:
+- `python` from `python/`
+- `launch_backend.py` as entrypoint
</code_context>
<issue_to_address>
**nitpick (typo):** Minor grammar tweak: add "the" before "backend".
Suggested wording: "Electron reads `runtime-manifest.json` and starts the backend with:"
```suggestion
Electron reads `runtime-manifest.json` and starts the backend with:
```
</issue_to_address>
### Comment 3
<location> `desktop/lib/backend-manager.js:742` </location>
<code_context>
+ );
+ }
+
+ queryWindowsProcessCommandLineByPowerShell(pid) {
+ const query = `$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}"; if ($null -ne $p) { $p.CommandLine }`;
+ return spawnSync(
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the new Windows process-handling helpers (PowerShell querying, command-line cache, and matcher) to reduce duplication and indirection while preserving behavior.
You can trim a fair bit of structural complexity without changing behavior by:
---
### 1. Unifying PowerShell / pwsh command‑line querying
`queryWindowsProcessCommandLineByPowerShell` and `queryWindowsProcessCommandLineByPwsh` are identical except for the binary name. You can collapse them into a single helper and simplify `getWindowsProcessCommandLine`’s `queryAttempts`:
```js
queryWindowsProcessCommandLine(shellName, pid) {
const query =
`$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}"; ` +
'if ($null -ne $p) { $p.CommandLine }';
return spawnSync(
shellName,
['-NoProfile', '-NonInteractive', '-Command', query],
{
stdio: ['ignore', 'pipe', 'ignore'],
encoding: 'utf8',
windowsHide: true,
timeout: WINDOWS_PROCESS_QUERY_TIMEOUT_MS,
},
);
}
```
Then:
```js
const queryAttempts = [
{ shellName: 'powershell' },
{ shellName: 'pwsh' },
];
for (const { shellName } of queryAttempts) {
let result;
try {
result = this.queryWindowsProcessCommandLine(shellName, numericPid);
} catch (error) {
if (error instanceof Error && error.message) {
this.log(
`Failed to query process command line by ${shellName} for pid=${numericPid}: ${error.message}`,
);
}
continue;
}
// ... existing ENOENT / ETIMEDOUT / status handling ...
}
```
This keeps all current behavior while reducing duplication and the surface area you need to touch for future changes (e.g., timeouts or flags).
---
### 2. Simplifying the command‑line cache
Right now you have:
- TTL on each entry,
- explicit pruning on every call,
- and negative caching (`commandLine: null`).
Given this is only used during unmanaged cleanup, you can simplify the cache without changing externally observable behavior:
- Drop `pruneWindowsProcessCommandLineCache` and the per‑call prune.
- Cache only successful lookups.
- Treat failures as “no cache entry” so they can be retried if needed.
Example:
```js
getWindowsProcessCommandLine(pid) {
const numericPid = Number.parseInt(`${pid}`, 10);
if (!Number.isInteger(numericPid)) {
return null;
}
const cached = this.windowsProcessCommandLineCache.get(numericPid);
if (cached) {
return cached; // just the command line string
}
const queryAttempts = [
{ shellName: 'powershell' },
{ shellName: 'pwsh' },
];
for (const { shellName } of queryAttempts) {
let result;
try {
result = this.queryWindowsProcessCommandLine(shellName, numericPid);
} catch (error) {
if (error instanceof Error && error.message) {
this.log(
`Failed to query process command line by ${shellName} for pid=${numericPid}: ${error.message}`,
);
}
continue;
}
if (result.error && result.error.code === 'ENOENT') {
continue;
}
if (result.error && result.error.code === 'ETIMEDOUT') {
this.log(
`Timed out (${WINDOWS_PROCESS_QUERY_TIMEOUT_MS}ms) querying process command line by ${shellName} for pid=${numericPid}.`,
);
continue;
}
if (result.status === 0) {
const commandLine = this.parseWindowsProcessCommandLine(result);
if (commandLine) {
this.windowsProcessCommandLineCache.set(numericPid, commandLine);
}
return commandLine;
}
}
return null;
}
```
This removes timestamp/TTL bookkeeping and negative entries while preserving semantics: callers still get `null` for “cannot resolve command line,” but the internal state is much easier to reason about.
---
### 3. Inlining the matcher to avoid an extra abstraction layer (optional)
If you want to further reduce indirection, you can compute the matcher fields directly inside `shouldKillUnmanagedBackendProcess` and pass `backendConfig` instead of a separate matcher object.
For example, instead of:
```js
const backendConfig = this.getBackendConfig();
const processMatcher = this.buildWindowsUnmanagedBackendMatcher(backendConfig);
if (!this.shouldKillUnmanagedBackendProcess(pid, processInfo, processMatcher)) {
continue;
}
```
You can make `shouldKillUnmanagedBackendProcess` accept `backendConfig` and build what it needs inside:
```js
shouldKillUnmanagedBackendProcess(pid, processInfo, backendConfig) {
const expectedImageName = path.basename(backendConfig.cmd || 'python.exe').toLowerCase();
const actualImageName = processInfo.imageName.toLowerCase();
if (actualImageName !== expectedImageName) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unexpected process image ${processInfo.imageName}.`,
);
return false;
}
const requireStrictCommandLineCheck =
this.isGenericWindowsPythonImage(expectedImageName);
if (!requireStrictCommandLineCheck) {
return true;
}
// ...build markers from backendConfig.args here and run the existing checks...
}
```
This keeps behavior intact but removes the extra “matcher” type and its construction step, making the control flow easier to follow from `stopUnmanagedBackendByPort` downwards.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Hey - 我发现了 3 个问题，并留下了一些整体性的反馈：
- 在
desktop/scripts/build-backend.mjs中，当前的运行时探测要求import pip成功；如果有人提供的是不带 pip 的精简 CPython 运行时，这里会直接硬失败，即使后端实际上仍然可以工作——建议只探测sys.version_info，或者把 pip 检查做成可选，并在失败时给出更清晰的错误路径。 - 在
PipInstaller.install中，当wheel_only=True时，你无条件添加了--only-binary :all:和--prefer-binary，但没有防护可能已经在pip_install_arg中出现的--no-binary之类的冲突参数；在构造最终参数列表前，对不兼容选项做归一化处理或直接拒绝会更安全。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- 在 `desktop/scripts/build-backend.mjs` 中，当前的运行时探测要求 `import pip` 成功；如果有人提供的是不带 pip 的精简 CPython 运行时，这里会直接硬失败，即使后端实际上仍然可以工作——建议只探测 `sys.version_info`，或者把 pip 检查做成可选，并在失败时给出更清晰的错误路径。
- 在 `PipInstaller.install` 中，当 `wheel_only=True` 时，你无条件添加了 `--only-binary :all:` 和 `--prefer-binary`，但没有防护可能已经在 `pip_install_arg` 中出现的 `--no-binary` 之类的冲突参数；在构造最终参数列表前，对不兼容选项做归一化处理或直接拒绝会更安全。
## Individual Comments
### Comment 1
<location> `desktop/scripts/build-backend.mjs:107-43` </location>
<code_context>
+const expectedRuntimeVersion = resolveExpectedRuntimeVersion();
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 当前的运行时 Python 版本检查比实际需要更严格，可能会拒绝兼容的更高版本。
这里 `resolveExpectedRuntimeVersion` 实际上被当成一个严格的 `major.minor` 固定版本来处理。如果它是从 `requires-python >= X.Y` 这样的约束推导出来，那就会错误地拒绝兼容的更高版本运行时（例如要求 `>=3.10` 时拒绝 3.11），从而导致构建失败。
为了与“最低版本”约束对齐，检查逻辑应该在 `major.minor` 上使用 `>=` 语义（只在运行时版本低于最低版本时失败）。你仍然可以在明确设置了 `ASTRBOT_DESKTOP_EXPECTED_PYTHON` 时保持严格相等检查，而对从 `pyproject.toml` 推导出的值只使用“下界”逻辑。
建议的实现方式：
```javascript
const { expectedRuntimeVersion, isLowerBoundRuntimeVersion } = resolveExpectedRuntimeVersion();
```
为了完成你描述的行为变更，你还需要同时调整 `resolveExpectedRuntimeVersion` 的实现以及运行时检查逻辑：
1. 更新 `resolveExpectedRuntimeVersion`，让它返回一个对象，例如：
```js
// 当来源于环境变量时：
return { expectedRuntimeVersion: parseExpectedRuntimeVersion(envValue, 'ASTRBOT_DESKTOP_EXPECTED_PYTHON'), isLowerBoundRuntimeVersion: false };
// 当来源于 pyproject requires-python >=X.Y 时：
return { expectedRuntimeVersion: parseExpectedRuntimeVersion(projectLowerBound, 'pyproject.toml requires-python'), isLowerBoundRuntimeVersion: true };
```
2. 找到当前校验 Python 运行时版本的地方（很可能类似 `if (runtimeVersion !== expectedRuntimeVersion) { ... }`），并将其修改为：
```js
if (isLowerBoundRuntimeVersion) {
// 仅当运行时版本 *低于* expectedRuntimeVersion 的 major.minor 时失败
if (compareMajorMinor(runtimeVersion, expectedRuntimeVersion) < 0) {
// fail
}
} else {
// 当通过环境变量显式设置时使用严格相等
if (runtimeVersion !== expectedRuntimeVersion) {
// fail
}
}
```
你需要一个类似 `compareMajorMinor(a, b)` 的辅助函数，用来把 `major.minor` 解析成整数并返回 -1/0/1。
3. 确保所有 `resolveExpectedRuntimeVersion` 的调用点都已经更新为使用新的 `{ expectedRuntimeVersion, isLowerBoundRuntimeVersion }` 结构。
</issue_to_address>
### Comment 2
<location> `desktop/lib/backend-manager.js:742` </location>
<code_context>
+ );
+ }
+
+ queryWindowsProcessCommandLineByPowerShell(pid) {
+ const query = `$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}"; if ($null -ne $p) { $p.CommandLine }`;
+ return spawnSync(
</code_context>
<issue_to_address>
**issue (complexity):** 建议重构新的 Windows 进程检查和清理逻辑，以消除重复、简化缓存，并将“是否终止”的决策收敛到单一辅助函数中，便于推理和维护。
你可以保留所有新的安全特性，同时在几个关键位置降低复杂度：
---
### 1. 合并两个 PowerShell 辅助函数
`queryWindowsProcessCommandLineByPowerShell` 和 `queryWindowsProcessCommandLineByPwsh` 除了可执行文件名不同外完全相同。你可以用一个单一的辅助函数替换这两个函数，并简化 `getWindowsProcessCommandLine`：
```js
// 用下面的函数替换 queryWindowsProcessCommandLineByPowerShell/Pwsh：
queryWindowsProcessCommandLine(shellName, pid) {
const query =
`$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}"; ` +
'if ($null -ne $p) { $p.CommandLine }';
return spawnSync(
shellName,
['-NoProfile', '-NonInteractive', '-Command', query],
{
stdio: ['ignore', 'pipe', 'ignore'],
encoding: 'utf8',
windowsHide: true,
timeout: WINDOWS_PROCESS_QUERY_TIMEOUT_MS,
},
);
}
```
然后 `getWindowsProcessCommandLine` 可以重写为：
```js
const queryAttempts = ['powershell', 'pwsh'];
for (const shellName of queryAttempts) {
let result;
try {
result = this.queryWindowsProcessCommandLine(shellName, numericPid);
} catch (error) {
if (error instanceof Error && error.message) {
this.log(
`Failed to query process command line by ${shellName} for pid=${numericPid}: ${error.message}`,
);
}
continue;
}
if (result.error && result.error.code === 'ENOENT') continue;
if (result.error && result.error.code === 'ETIMEDOUT') {
this.log(
`Timed out (${WINDOWS_PROCESS_QUERY_TIMEOUT_MS}ms) querying process command line by ${shellName} for pid=${numericPid}.`,
);
continue;
}
if (result.status === 0) {
const commandLine = this.parseWindowsProcessCommandLine(result);
this.windowsProcessCommandLineCache.set(numericPid, {
commandLine,
timestampMs: Date.now(),
});
return commandLine;
}
}
```
---
### 2. 去掉缓存中的显式清理和时间戳
由于该缓存只在“非托管清理”路径中使用，你可以将其简化为一个普通的 `Map<pid, commandLine>`，并彻底移除 TTL/清理逻辑。这可以删除 `WINDOWS_PROCESS_COMMAND_LINE_CACHE_TTL_MS`、`pruneWindowsProcessCommandLineCache` 以及所有时间戳处理，而不会改变清理流程的可观察行为。
```js
// 构造函数中：
this.windowsProcessCommandLineCache = new Map(); // pid -> commandLine
// 完全删除 pruneWindowsProcessCommandLineCache 以及在 stopUnmanagedBackendByPort 中的调用
getWindowsProcessCommandLine(pid) {
const numericPid = Number.parseInt(`${pid}`, 10);
if (!Number.isInteger(numericPid)) {
return null;
}
if (this.windowsProcessCommandLineCache.has(numericPid)) {
return this.windowsProcessCommandLineCache.get(numericPid);
}
// ... 保持原有的查询逻辑 ...
if (result.status === 0) {
const commandLine = this.parseWindowsProcessCommandLine(result);
this.windowsProcessCommandLineCache.set(numericPid, commandLine);
return commandLine;
}
this.windowsProcessCommandLineCache.set(numericPid, null);
return null;
}
```
这样可以保留缓存语义（同一次清理运行中不会重复查询），但去掉生命周期管理和额外分支。
---
### 3. 将 matcher 对象内联为单一决策辅助函数
当前的 `buildWindowsUnmanagedBackendMatcher` 加上 `shouldKillUnmanagedBackendProcess` 可以合并为单一、便于审计的辅助函数，直接基于 `backendConfig` 进行操作。这样就可以移除中间的“matcher 对象”抽象以及额外的间接层。
```js
shouldKillUnmanagedBackendProcess(pid, processInfo, backendConfig) {
const expectedImageName = path
.basename(backendConfig.cmd || 'python.exe')
.toLowerCase();
const actualImageName = processInfo.imageName.toLowerCase();
if (actualImageName !== expectedImageName) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unexpected process image ${processInfo.imageName}.`,
);
return false;
}
if (!this.isGenericWindowsPythonImage(expectedImageName)) {
return true;
}
const expectedCommandLineMarkers = [];
if (Array.isArray(backendConfig.args) && backendConfig.args.length > 0) {
const primaryArg = backendConfig.args[0];
if (typeof primaryArg === 'string' && primaryArg) {
const resolvedPrimaryArg = path.isAbsolute(primaryArg)
? primaryArg
: path.resolve(backendConfig.cwd || process.cwd(), primaryArg);
expectedCommandLineMarkers.push(
this.normalizeWindowsPathForMatch(resolvedPrimaryArg),
);
expectedCommandLineMarkers.push(
this.normalizeWindowsPathForMatch(path.basename(primaryArg)),
);
}
}
if (!expectedCommandLineMarkers.length) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: backend launch marker is unavailable.`,
);
return false;
}
const commandLine = this.getWindowsProcessCommandLine(pid);
if (!commandLine) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process command line.`,
);
return false;
}
const normalizedCommandLine = this.normalizeWindowsPathForMatch(commandLine);
const markerMatched = expectedCommandLineMarkers.some(
(marker) => marker && normalizedCommandLine.includes(marker),
);
if (!markerMatched) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: command line does not match AstrBot backend launch marker.`,
);
return false;
}
return true;
}
```
这样 `stopUnmanagedBackendByPort` 也会更简单：
```js
async stopUnmanagedBackendByPort() {
if (!this.app.isPackaged || process.platform !== 'win32') {
return false;
}
const port = this.getBackendPort();
if (!port) {
return false;
}
const pids = this.findListeningPidsOnWindows(port);
if (!pids.length) {
return false;
}
this.log(
`Attempting unmanaged backend cleanup by port=${port} pids=${pids.join(',')}`,
);
const backendConfig = this.getBackendConfig();
for (const pid of pids) {
const processInfo = this.getWindowsProcessInfo(pid);
if (!processInfo) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process info.`,
);
continue;
}
if (!this.shouldKillUnmanagedBackendProcess(pid, processInfo, backendConfig)) {
continue;
}
// taskkill 逻辑保持不变...
}
return true;
}
```
这样可以保留所有现有检查（镜像名匹配 + 可选的严格命令行标记），但把“是否终止”的决策集中在一个容易追踪的地方。
</issue_to_address>
### Comment 3
<location> `desktop/scripts/build-backend.mjs:14` </location>
<code_context>
+const manifestPath = path.join(outputDir, 'runtime-manifest.json');
+const launcherPath = path.join(outputDir, 'launch_backend.py');
+
+const runtimeSource =
+ process.env.ASTRBOT_DESKTOP_CPYTHON_HOME ||
+ process.env.ASTRBOT_DESKTOP_BACKEND_RUNTIME;
</code_context>
<issue_to_address>
**issue (complexity):** 建议重构这个脚本，将失败处理集中起来，简化路径/运行时相关的辅助函数，并稍微收紧过滤逻辑，使主构建流程更短、更易阅读，同时保持行为不变。
你可以在不删除任何现有行为的前提下明显降低复杂度：
---
### 1. 集中错误处理
目前你在很多辅助函数中重复使用 `console.error` + `process.exit(1)`。可以增加一个简单的 `fail` 辅助函数，保持行为一致的同时简化所有调用点：
```ts
// 在文件顶部附近
const fail = (message: string): never => {
console.error(message);
process.exit(1);
};
```
然后把类似下面的模式：
```ts
if (!runtimeSource) {
console.error(
'Missing CPython runtime source. Set ASTRBOT_DESKTOP_CPYTHON_HOME ' +
'(recommended) or ASTRBOT_DESKTOP_BACKEND_RUNTIME.',
);
process.exit(1);
}
```
替换为：
```ts
if (!runtimeSource) {
fail(
'Missing CPython runtime source. Set ASTRBOT_DESKTOP_CPYTHON_HOME ' +
'(recommended) or ASTRBOT_DESKTOP_BACKEND_RUNTIME.',
);
}
```
在 `parseExpectedRuntimeVersion`、`resolveExpectedRuntimeVersion`、`validateRuntimePython` 等函数中同样进行替换。光是这一步就能去掉大量分支噪音。
---
### 2. 简化路径重叠逻辑
你可以保留路径重叠保护，但用更直接的方式实现，不需要在多处散落单独的“normalize”辅助函数：
```ts
const normalize = (p: string) => {
const resolved = path.resolve(p).replace(/[\\/]+$/, '');
return process.platform === 'win32' ? resolved.toLowerCase() : resolved;
};
const pathsOverlap = (a: string, b: string) => {
const na = normalize(a);
const nb = normalize(b);
return na === nb || na.startsWith(nb + path.sep) || nb.startsWith(na + path.sep);
};
const runtimeSourceReal = path.resolve(rootDir, runtimeSource);
if (!fs.existsSync(runtimeSourceReal)) {
fail(`CPython runtime source does not exist: ${runtimeSourceReal}`);
}
if (pathsOverlap(runtimeSourceReal, outputDir)) {
fail(
`CPython runtime source overlaps with backend output directory. ` +
`runtime=${runtimeSourceReal}, output=${outputDir}. ` +
'Please set ASTRBOT_DESKTOP_CPYTHON_HOME to a separate runtime directory.',
);
}
```
这样可以保持跨平台行为和重叠检查，但主流程会更容易阅读。
---
### 3. 将运行时解析 + 校验合并为单一辅助函数
你现在是先调用 `resolveRuntimePython`，然后立刻对结果进行校验。把这两个步骤合并到一个函数中，可以更清晰地表达“查找 + 断言”的意图，并减少调用图深度：
```ts
const findAndValidateRuntimePython = (runtimeRoot: string) => {
const candidates =
process.platform === 'win32'
? ['python.exe', path.join('Scripts', 'python.exe')]
: [path.join('bin', 'python3'), path.join('bin', 'python')];
for (const rel of candidates) {
const abs = path.join(runtimeRoot, rel);
if (fs.existsSync(abs)) {
validateRuntimePython(abs);
return {
absolute: abs,
relative: path.relative(outputDir, abs),
};
}
}
fail(
`Cannot find Python executable in runtime: ${runtimeRoot}. ` +
'Expected python under bin/ or Scripts/.',
);
};
```
然后主流程可以写成：
```ts
const sourceRuntimePython = findAndValidateRuntimePython(runtimeSourceReal);
// ...
const runtimePython = findAndValidateRuntimePython(runtimeDir);
```
---
### 4. 简化 `validateRuntimePython` 的版本解析
你可以保留 `pip` 的导入（用来确认它存在），但去掉格式化字符串 + 正则匹配的往返，只打印两个数字再 `split` 即可：
```ts
const validateRuntimePython = (pythonExecutable: string) => {
const probe = spawnSync(
pythonExecutable,
[
'-c',
'import sys, pip; print(sys.version_info[0], sys.version_info[1])',
],
{
stdio: ['ignore', 'pipe', 'pipe'],
encoding: 'utf8',
windowsHide: true,
timeout: 5000,
},
);
if (probe.error) {
const reason =
probe.error.code === 'ETIMEDOUT'
? 'runtime Python probe timed out'
: probe.error.message || String(probe.error);
fail(`Runtime Python probe failed: ${reason}`);
}
if (probe.status !== 0) {
const stderrText = (probe.stderr || '').trim();
fail(
`Runtime Python probe failed with exit code ${probe.status}.` +
(stderrText ? ` stderr: ${stderrText}` : ''),
);
}
const parts = (probe.stdout || '').trim().split(/\s+/);
if (parts.length < 2) {
fail(
`Runtime Python probe did not report a valid version. Output: ${(probe.stdout || '').trim()}`,
);
}
const actual = {
major: Number.parseInt(parts[0], 10),
minor: Number.parseInt(parts[1], 10),
};
if (
actual.major !== expectedRuntimeVersion.major ||
actual.minor !== expectedRuntimeVersion.minor
) {
fail(
`Runtime Python version mismatch: expected ${expectedRuntimeVersion.major}.${expectedRuntimeVersion.minor}, ` +
`got ${actual.major}.${actual.minor}.`,
);
}
};
```
这样可以保留所有功能（包括验证 `pip` 可被导入），同时简化探测逻辑并避免使用正则。
---
### 5. 在不改变行为的前提下略微收紧拷贝过滤
当前的 `shouldCopy` 没问题，但可以稍微合并一下条件，减轻视觉负担：
```ts
const IGNORED_NAMES = new Set([
'__pycache__',
'.pytest_cache',
'.ruff_cache',
'.git',
'.mypy_cache',
'.DS_Store',
]);
const shouldCopy = (srcPath: string) => {
const base = path.basename(srcPath);
if (IGNORED_NAMES.has(base)) return false;
return !base.endsWith('.pyc') && !base.endsWith('.pyo');
};
```
`copyTree` 的调用点可以完全保持不变，但意图会更清晰、代码也更紧凑。
---
应用这些针对性的改动，可以保留所有新增行为（环境变量/pyproject 配置、路径重叠检查、运行时校验、过滤器、manifest/launcher 等），同时让脚本的主流程和辅助函数更易读、更易维护。
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据反馈改进后续评审。
Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
- In
desktop/scripts/build-backend.mjs, the runtime probe requiresimport pipto succeed; if someone provides a minimal CPython runtime without pip this will hard-fail even though the backend might still work—consider probing onlysys.version_infoor making the pip check optional with a clearer error path. - In
PipInstaller.install, whenwheel_only=Trueyou unconditionally add--only-binary :all:and--prefer-binarybut do not guard against conflicting flags like--no-binarythat may be present inpip_install_arg; it would be safer to normalize or reject incompatible options before constructing the final argument list.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `desktop/scripts/build-backend.mjs`, the runtime probe requires `import pip` to succeed; if someone provides a minimal CPython runtime without pip this will hard-fail even though the backend might still work—consider probing only `sys.version_info` or making the pip check optional with a clearer error path.
- In `PipInstaller.install`, when `wheel_only=True` you unconditionally add `--only-binary :all:` and `--prefer-binary` but do not guard against conflicting flags like `--no-binary` that may be present in `pip_install_arg`; it would be safer to normalize or reject incompatible options before constructing the final argument list.
## Individual Comments
### Comment 1
<location> `desktop/scripts/build-backend.mjs:107-43` </location>
<code_context>
+const expectedRuntimeVersion = resolveExpectedRuntimeVersion();
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The runtime Python version check is stricter than necessary and may reject compatible newer runtimes.
Here `resolveExpectedRuntimeVersion` is effectively treated as a strict `major.minor` pin. If it’s derived from a `requires-python >= X.Y` constraint, that will incorrectly reject newer compatible runtimes (e.g. 3.11 when `>=3.10`) and fail the build.
To align with a minimum-version constraint, the check should use `>=` semantics on `major.minor` (only fail when the runtime is older than the minimum). You could still keep the strict equality check when `ASTRBOT_DESKTOP_EXPECTED_PYTHON` is explicitly set, and only use the lower-bound logic for values inferred from `pyproject.toml`.
Suggested implementation:
```javascript
const { expectedRuntimeVersion, isLowerBoundRuntimeVersion } = resolveExpectedRuntimeVersion();
```
To complete the behavior change you described, you will also need to adjust the implementation of `resolveExpectedRuntimeVersion` and the runtime check:
1. Update `resolveExpectedRuntimeVersion` so it returns an object, e.g.
```js
// when coming from env:
return { expectedRuntimeVersion: parseExpectedRuntimeVersion(envValue, 'ASTRBOT_DESKTOP_EXPECTED_PYTHON'), isLowerBoundRuntimeVersion: false };
// when coming from pyproject requires-python >=X.Y:
return { expectedRuntimeVersion: parseExpectedRuntimeVersion(projectLowerBound, 'pyproject.toml requires-python'), isLowerBoundRuntimeVersion: true };
```
2. Locate where the current Python runtime version is validated (likely something like `if (runtimeVersion !== expectedRuntimeVersion) { ... }`) and change it to:
```js
if (isLowerBoundRuntimeVersion) {
// fail only when runtime is *older* than expectedRuntimeVersion major.minor
if (compareMajorMinor(runtimeVersion, expectedRuntimeVersion) < 0) {
// fail
}
} else {
// strict equality when explicitly set via env
if (runtimeVersion !== expectedRuntimeVersion) {
// fail
}
}
```
You will need a helper like `compareMajorMinor(a, b)` that parses the `major.minor` segments into integers and returns -1/0/1.
3. Ensure all call sites of `resolveExpectedRuntimeVersion` are updated to expect the new `{ expectedRuntimeVersion, isLowerBoundRuntimeVersion }` shape.
</issue_to_address>
### Comment 2
<location> `desktop/lib/backend-manager.js:742` </location>
<code_context>
+ );
+ }
+
+ queryWindowsProcessCommandLineByPowerShell(pid) {
+ const query = `$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}"; if ($null -ne $p) { $p.CommandLine }`;
+ return spawnSync(
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring the new Windows process-inspection and cleanup logic to remove duplication, simplify caching, and consolidate the kill decision into a single helper for easier reasoning and maintenance.
You can keep all the new safety features and still reduce complexity in a few focused spots:
---
### 1. Collapse the two PowerShell helpers
`queryWindowsProcessCommandLineByPowerShell` and `queryWindowsProcessCommandLineByPwsh` are identical apart from the executable name. You can replace both with a single helper and simplify `getWindowsProcessCommandLine`:
```js
// replace both queryWindowsProcessCommandLineByPowerShell/Pwsh with:
queryWindowsProcessCommandLine(shellName, pid) {
const query =
`$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${pid}"; ` +
'if ($null -ne $p) { $p.CommandLine }';
return spawnSync(
shellName,
['-NoProfile', '-NonInteractive', '-Command', query],
{
stdio: ['ignore', 'pipe', 'ignore'],
encoding: 'utf8',
windowsHide: true,
timeout: WINDOWS_PROCESS_QUERY_TIMEOUT_MS,
},
);
}
```
Then `getWindowsProcessCommandLine` becomes:
```js
const queryAttempts = ['powershell', 'pwsh'];
for (const shellName of queryAttempts) {
let result;
try {
result = this.queryWindowsProcessCommandLine(shellName, numericPid);
} catch (error) {
if (error instanceof Error && error.message) {
this.log(
`Failed to query process command line by ${shellName} for pid=${numericPid}: ${error.message}`,
);
}
continue;
}
if (result.error && result.error.code === 'ENOENT') continue;
if (result.error && result.error.code === 'ETIMEDOUT') {
this.log(
`Timed out (${WINDOWS_PROCESS_QUERY_TIMEOUT_MS}ms) querying process command line by ${shellName} for pid=${numericPid}.`,
);
continue;
}
if (result.status === 0) {
const commandLine = this.parseWindowsProcessCommandLine(result);
this.windowsProcessCommandLineCache.set(numericPid, {
commandLine,
timestampMs: Date.now(),
});
return commandLine;
}
}
```
---
### 2. Drop explicit pruning and timestamps from the cache
Given this cache is used only in the unmanaged cleanup path, you can simplify it to a plain `Map<pid, commandLine>` and remove TTL/pruning logic altogether. This eliminates `WINDOWS_PROCESS_COMMAND_LINE_CACHE_TTL_MS`, `pruneWindowsProcessCommandLineCache`, and timestamp handling without changing observable behavior for the cleanup flow.
```js
// constructor:
this.windowsProcessCommandLineCache = new Map(); // pid -> commandLine
// remove pruneWindowsProcessCommandLineCache entirely and its call in stopUnmanagedBackendByPort
getWindowsProcessCommandLine(pid) {
const numericPid = Number.parseInt(`${pid}`, 10);
if (!Number.isInteger(numericPid)) {
return null;
}
if (this.windowsProcessCommandLineCache.has(numericPid)) {
return this.windowsProcessCommandLineCache.get(numericPid);
}
// ... query logic as before ...
if (result.status === 0) {
const commandLine = this.parseWindowsProcessCommandLine(result);
this.windowsProcessCommandLineCache.set(numericPid, commandLine);
return commandLine;
}
this.windowsProcessCommandLineCache.set(numericPid, null);
return null;
}
```
This keeps caching semantics (no repeated queries within a cleanup run) but removes lifecycle management and extra branching.
---
### 3. Inline the matcher object into a single decision helper
`buildWindowsUnmanagedBackendMatcher` + `shouldKillUnmanagedBackendProcess` can be merged into a single, audit-friendly helper that operates directly on `backendConfig`. That removes the intermediary “matcher object” abstraction and the extra indirection.
```js
shouldKillUnmanagedBackendProcess(pid, processInfo, backendConfig) {
const expectedImageName = path
.basename(backendConfig.cmd || 'python.exe')
.toLowerCase();
const actualImageName = processInfo.imageName.toLowerCase();
if (actualImageName !== expectedImageName) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unexpected process image ${processInfo.imageName}.`,
);
return false;
}
if (!this.isGenericWindowsPythonImage(expectedImageName)) {
return true;
}
const expectedCommandLineMarkers = [];
if (Array.isArray(backendConfig.args) && backendConfig.args.length > 0) {
const primaryArg = backendConfig.args[0];
if (typeof primaryArg === 'string' && primaryArg) {
const resolvedPrimaryArg = path.isAbsolute(primaryArg)
? primaryArg
: path.resolve(backendConfig.cwd || process.cwd(), primaryArg);
expectedCommandLineMarkers.push(
this.normalizeWindowsPathForMatch(resolvedPrimaryArg),
);
expectedCommandLineMarkers.push(
this.normalizeWindowsPathForMatch(path.basename(primaryArg)),
);
}
}
if (!expectedCommandLineMarkers.length) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: backend launch marker is unavailable.`,
);
return false;
}
const commandLine = this.getWindowsProcessCommandLine(pid);
if (!commandLine) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process command line.`,
);
return false;
}
const normalizedCommandLine = this.normalizeWindowsPathForMatch(commandLine);
const markerMatched = expectedCommandLineMarkers.some(
(marker) => marker && normalizedCommandLine.includes(marker),
);
if (!markerMatched) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: command line does not match AstrBot backend launch marker.`,
);
return false;
}
return true;
}
```
And `stopUnmanagedBackendByPort` becomes simpler:
```js
async stopUnmanagedBackendByPort() {
if (!this.app.isPackaged || process.platform !== 'win32') {
return false;
}
const port = this.getBackendPort();
if (!port) {
return false;
}
const pids = this.findListeningPidsOnWindows(port);
if (!pids.length) {
return false;
}
this.log(
`Attempting unmanaged backend cleanup by port=${port} pids=${pids.join(',')}`,
);
const backendConfig = this.getBackendConfig();
for (const pid of pids) {
const processInfo = this.getWindowsProcessInfo(pid);
if (!processInfo) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process info.`,
);
continue;
}
if (!this.shouldKillUnmanagedBackendProcess(pid, processInfo, backendConfig)) {
continue;
}
// taskkill as before...
}
return true;
}
```
This keeps all existing checks (image match + optional strict command line markers) but makes the kill decision traceable in a single place.
</issue_to_address>
### Comment 3
<location> `desktop/scripts/build-backend.mjs:14` </location>
<code_context>
+const manifestPath = path.join(outputDir, 'runtime-manifest.json');
+const launcherPath = path.join(outputDir, 'launch_backend.py');
+
+const runtimeSource =
+ process.env.ASTRBOT_DESKTOP_CPYTHON_HOME ||
+ process.env.ASTRBOT_DESKTOP_BACKEND_RUNTIME;
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring this script to centralize failures, streamline path/runtime helpers, and tighten filters so the main build flow is shorter and easier to follow without changing behavior.
You can reduce the complexity noticeably without dropping any behavior by:
---
### 1. Centralize error handling
You currently repeat `console.error` + `process.exit(1)` in many helpers. A small `fail` helper keeps behavior identical but simplifies all callers:
```ts
// Near the top
const fail = (message: string): never => {
console.error(message);
process.exit(1);
};
```
Then replace patterns like:
```ts
if (!runtimeSource) {
console.error(
'Missing CPython runtime source. Set ASTRBOT_DESKTOP_CPYTHON_HOME ' +
'(recommended) or ASTRBOT_DESKTOP_BACKEND_RUNTIME.',
);
process.exit(1);
}
```
with:
```ts
if (!runtimeSource) {
fail(
'Missing CPython runtime source. Set ASTRBOT_DESKTOP_CPYTHON_HOME ' +
'(recommended) or ASTRBOT_DESKTOP_BACKEND_RUNTIME.',
);
}
```
Do the same in `parseExpectedRuntimeVersion`, `resolveExpectedRuntimeVersion`, `validateRuntimePython`, etc. This alone removes a lot of branching noise.
---
### 2. Simplify path overlap logic
You can keep the overlap protection but make it more direct, without separate “normalize” helpers spread around:
```ts
const normalize = (p: string) => {
const resolved = path.resolve(p).replace(/[\\/]+$/, '');
return process.platform === 'win32' ? resolved.toLowerCase() : resolved;
};
const pathsOverlap = (a: string, b: string) => {
const na = normalize(a);
const nb = normalize(b);
return na === nb || na.startsWith(nb + path.sep) || nb.startsWith(na + path.sep);
};
const runtimeSourceReal = path.resolve(rootDir, runtimeSource);
if (!fs.existsSync(runtimeSourceReal)) {
fail(`CPython runtime source does not exist: ${runtimeSourceReal}`);
}
if (pathsOverlap(runtimeSourceReal, outputDir)) {
fail(
`CPython runtime source overlaps with backend output directory. ` +
`runtime=${runtimeSourceReal}, output=${outputDir}. ` +
'Please set ASTRBOT_DESKTOP_CPYTHON_HOME to a separate runtime directory.',
);
}
```
This keeps the cross‑platform behavior and overlap check, but the main flow is easier to read.
---
### 3. Make runtime resolution + validation a single helper
You call `resolveRuntimePython` and then immediately validate it. Merging them into one function clarifies the “find + assert” intent and reduces call graph depth:
```ts
const findAndValidateRuntimePython = (runtimeRoot: string) => {
const candidates =
process.platform === 'win32'
? ['python.exe', path.join('Scripts', 'python.exe')]
: [path.join('bin', 'python3'), path.join('bin', 'python')];
for (const rel of candidates) {
const abs = path.join(runtimeRoot, rel);
if (fs.existsSync(abs)) {
validateRuntimePython(abs);
return {
absolute: abs,
relative: path.relative(outputDir, abs),
};
}
}
fail(
`Cannot find Python executable in runtime: ${runtimeRoot}. ` +
'Expected python under bin/ or Scripts/.',
);
};
```
Then the main flow becomes:
```ts
const sourceRuntimePython = findAndValidateRuntimePython(runtimeSourceReal);
// ...
const runtimePython = findAndValidateRuntimePython(runtimeDir);
```
---
### 4. Simplify `validateRuntimePython` version parsing
You can keep the `pip` import (to assert it’s present) but remove the formatted string + regex round‑trip; just print two numbers and split:
```ts
const validateRuntimePython = (pythonExecutable: string) => {
const probe = spawnSync(
pythonExecutable,
[
'-c',
'import sys, pip; print(sys.version_info[0], sys.version_info[1])',
],
{
stdio: ['ignore', 'pipe', 'pipe'],
encoding: 'utf8',
windowsHide: true,
timeout: 5000,
},
);
if (probe.error) {
const reason =
probe.error.code === 'ETIMEDOUT'
? 'runtime Python probe timed out'
: probe.error.message || String(probe.error);
fail(`Runtime Python probe failed: ${reason}`);
}
if (probe.status !== 0) {
const stderrText = (probe.stderr || '').trim();
fail(
`Runtime Python probe failed with exit code ${probe.status}.` +
(stderrText ? ` stderr: ${stderrText}` : ''),
);
}
const parts = (probe.stdout || '').trim().split(/\s+/);
if (parts.length < 2) {
fail(
`Runtime Python probe did not report a valid version. Output: ${(probe.stdout || '').trim()}`,
);
}
const actual = {
major: Number.parseInt(parts[0], 10),
minor: Number.parseInt(parts[1], 10),
};
if (
actual.major !== expectedRuntimeVersion.major ||
actual.minor !== expectedRuntimeVersion.minor
) {
fail(
`Runtime Python version mismatch: expected ${expectedRuntimeVersion.major}.${expectedRuntimeVersion.minor}, ` +
`got ${actual.major}.${actual.minor}.`,
);
}
};
```
This preserves all functionality (including verifying `pip` is importable) but trims the probe logic and avoids regex.
---
### 5. Slightly tighten copy filtering without changing behavior
`shouldCopy` is fine, but you can collapse the conditionals a bit to reduce visual weight:
```ts
const IGNORED_NAMES = new Set([
'__pycache__',
'.pytest_cache',
'.ruff_cache',
'.git',
'.mypy_cache',
'.DS_Store',
]);
const shouldCopy = (srcPath: string) => {
const base = path.basename(srcPath);
if (IGNORED_NAMES.has(base)) return false;
return !base.endsWith('.pyc') && !base.endsWith('.pyo');
};
```
The call site of `copyTree` stays exactly the same, but the intent is clearer and more compact.
---
Applying these targeted changes would keep all of the new behavior (env/pyproject config, overlap checks, runtime validation, filters, manifest/launcher) while making the script’s main flow and helpers noticeably simpler to read and maintain.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
dosubot
bot
added
size:XXL
zouyonghe
changed the title
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些整体性反馈：
release.yml中的Resolve packaged CPython runtime source逻辑在跨平台和 Windows 这两个 job 里基本是重复的，只存在轻微差异；建议把这段逻辑抽取到一个可复用的脚本或 composite action 中，这样以后调整探测/校验逻辑时，可以避免两处实现之间发生偏差。dashboard/vite.config.ts里内联的normalizeNestedTypeSelectorPluginPostCSS 插件体积较大且逻辑较为专用；建议将其移动到一个独立的模块（例如src/plugins/normalize-nested-type-selector.ts），这样既能提升 Vite 配置文件的可读性，也能让选择器规范化逻辑更易于单元测试和独立演进。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- The `Resolve packaged CPython runtime source` logic in `release.yml` is duplicated with only minor differences between the cross-platform and Windows jobs; consider extracting this into a single reusable script or composite action to avoid divergence when you need to change the probing/validation later.
- The inline `normalizeNestedTypeSelectorPlugin` PostCSS plugin in `dashboard/vite.config.ts` is fairly large and specific; moving it into a dedicated module (e.g. `src/plugins/normalize-nested-type-selector.ts`) would improve readability of the Vite config and make the selector normalization logic easier to unit test or evolve independently.
## Individual Comments
### Comment 1
<location> `desktop/lib/backend-manager.js:670` </location>
<code_context>
- const expectedImageName = (
- path.basename(this.getPackagedBackendPath() || '') || 'astrbot-backend.exe'
- ).toLowerCase();
+ let backendConfig = null;
+ try {
+ backendConfig = this.getBackendConfig();
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the new backend config and unmanaged cleanup helpers by normalizing getBackendConfig, flattening resolveBackendConfig, and moving shared cleanup context into reusable/module-level state.
You can simplify a few of the new pieces without losing functionality.
### 1. Avoid rebuilding a rich context per PID in unmanaged cleanup
You currently build `contextBase` (including a new `Map` and log wrapper) for every cleanup call, and then spread it into each `shouldKillUnmanagedBackendProcess` invocation:
```js
const contextBase = {
backendConfig,
allowImageOnlyMatch: !hasBackendConfig,
commandLineCache: new Map(),
fallbackCmdRaw: process.env.ASTRBOT_BACKEND_CMD || 'python.exe',
spawnSync,
log: (message) => this.log(message),
};
for (const pid of pids) {
const processInfo = this.getWindowsProcessInfo(pid);
...
const shouldKill = shouldKillUnmanagedBackendProcess({
pid,
processInfo,
...contextBase,
});
...
}
```
Two concrete simplifications:
1. Move the “static” pieces into a class‑level field so they aren’t re‑allocated every time.
2. Let the helper module own the `commandLineCache` so the caller only passes in per‑call/per‑pid data.
Example:
```js
// in constructor
this.unmanagedCleanupBaseContext = {
fallbackCmdRaw: process.env.ASTRBOT_BACKEND_CMD || 'python.exe',
spawnSync,
log: (message) => this.log(message),
};
// in cleanup method
const backendConfig = this.getBackendConfig();
const hasBackendConfig = backendConfig && typeof backendConfig === 'object';
for (const pid of pids) {
const processInfo = this.getWindowsProcessInfo(pid);
if (!processInfo) {
this.log(`Skip unmanaged cleanup for pid=${pid}: unable to resolve process info.`);
continue;
}
const shouldKill = shouldKillUnmanagedBackendProcess({
pid,
processInfo,
backendConfig,
allowImageOnlyMatch: !hasBackendConfig,
...this.unmanagedCleanupBaseContext,
});
if (!shouldKill) continue;
spawnSync('taskkill', ['/pid', `${pid}`, '/t', '/f'], {
stdio: 'ignore',
windowsHide: true,
});
}
```
And inside `windows-backend-cleanup`, keep a module‑level `commandLineCache`:
```js
// windows-backend-cleanup.js
const commandLineCache = new Map();
function shouldKillUnmanagedBackendProcess({ pid, processInfo, backendConfig, allowImageOnlyMatch, fallbackCmdRaw, spawnSync, log }) {
// use commandLineCache here, no need to pass it from caller
}
```
This removes the per‑call `Map` and function allocation, and the caller only passes the bits the helper truly needs.
---
### 2. Make `getBackendConfig` always return a normalized shape for cleanup
The unmanaged cleanup path now has defensive try/catch + flags:
```js
let backendConfig = null;
try {
backendConfig = this.getBackendConfig();
} catch (error) {
this.log(`Failed to resolve backend config during unmanaged cleanup: ${...}`);
}
const hasBackendConfig = backendConfig && typeof backendConfig === 'object';
if (!hasBackendConfig) {
this.log(
'Backend config is unavailable during unmanaged cleanup; falling back to image-name-only matching.',
);
}
```
Given you control `getBackendConfig` / `resolveBackendConfig`, you can simplify the call site by normalizing the return value and avoiding exceptions entirely:
```js
getBackendConfig() {
if (!this.backendConfig) {
try {
this.backendConfig = this.resolveBackendConfig();
} catch (error) {
this.log(
`Failed to resolve backend config: ${
error instanceof Error ? error.message : String(error)
}`,
);
// Always return a normalized object
this.backendConfig = {
cmd: null,
args: [],
shell: true,
cwd: process.env.ASTRBOT_BACKEND_CWD || this.resolveBackendCwd(),
webuiDir: this.resolveWebuiDir(),
rootDir: process.env.ASTRBOT_ROOT || this.resolveBackendRoot(),
failureReason: 'Backend command is not configured.',
};
}
}
return this.backendConfig;
}
```
Then unmanaged cleanup can be much simpler:
```js
const backendConfig = this.getBackendConfig();
const hasBackendConfig = Boolean(backendConfig.cmd);
if (!hasBackendConfig) {
this.log(
'Backend command is not configured during unmanaged cleanup; falling back to image-name-only matching.',
);
}
// no try/catch needed here anymore
```
This keeps the “fallback to image‑only match” behavior but removes error‑based control flow from the cleanup routine.
---
### 3. Collapse `buildBackendConfig` into `resolveBackendConfig`
`buildBackendConfig` + `resolveBackendConfig` are now mostly plumbing to thread `failureReason`. You can keep `failureReason` while flattening the helpers into a single method, which makes the flow easier to follow:
Current:
```js
buildBackendConfig() {
const webuiDir = this.resolveWebuiDir();
let launch = null;
let failureReason = null;
...
return { launch, cwd, rootDir, webuiDir, failureReason };
}
resolveBackendConfig() {
const { launch, cwd, rootDir, webuiDir, failureReason } =
this.buildBackendConfig();
ensureDir(cwd);
if (rootDir) {
ensureDir(rootDir);
}
if (failureReason) {
this.log(failureReason);
}
this.backendConfig = {
cmd: launch ? launch.cmd : null,
args: launch ? launch.args : [],
shell: launch ? launch.shell : true,
cwd,
webuiDir,
rootDir,
failureReason,
};
return this.backendConfig;
}
```
Simplified, same behavior:
```js
resolveBackendConfig() {
const webuiDir = this.resolveWebuiDir();
let launch = null;
let failureReason = null;
const customCmd = process.env.ASTRBOT_BACKEND_CMD;
if (customCmd) {
launch = { cmd: customCmd, args: [], shell: true };
} else if (this.app.isPackaged) {
({ launch, failureReason } = this.buildLaunchForPackagedBackend(
this.getPackagedBackendState(),
webuiDir,
));
} else {
launch = this.buildDefaultBackendLaunch(webuiDir);
}
const cwd = process.env.ASTRBOT_BACKEND_CWD || this.resolveBackendCwd();
const rootDir = process.env.ASTRBOT_ROOT || this.resolveBackendRoot();
ensureDir(cwd);
if (rootDir) {
ensureDir(rootDir);
}
if (failureReason) {
this.log(failureReason);
}
this.backendConfig = {
cmd: launch ? launch.cmd : null,
args: launch ? launch.args : [],
shell: launch ? launch.shell : true,
cwd,
webuiDir,
rootDir,
failureReason,
};
return this.backendConfig;
}
```
You keep `failureReason` for reporting, but remove one layer of indirection and an intermediate object, which makes follow‑up changes less error‑prone.
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进之后的 Review。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The
Resolve packaged CPython runtime sourcelogic inrelease.ymlis duplicated with only minor differences between the cross-platform and Windows jobs; consider extracting this into a single reusable script or composite action to avoid divergence when you need to change the probing/validation later. - The inline
normalizeNestedTypeSelectorPluginPostCSS plugin indashboard/vite.config.tsis fairly large and specific; moving it into a dedicated module (e.g.src/plugins/normalize-nested-type-selector.ts) would improve readability of the Vite config and make the selector normalization logic easier to unit test or evolve independently.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `Resolve packaged CPython runtime source` logic in `release.yml` is duplicated with only minor differences between the cross-platform and Windows jobs; consider extracting this into a single reusable script or composite action to avoid divergence when you need to change the probing/validation later.
- The inline `normalizeNestedTypeSelectorPlugin` PostCSS plugin in `dashboard/vite.config.ts` is fairly large and specific; moving it into a dedicated module (e.g. `src/plugins/normalize-nested-type-selector.ts`) would improve readability of the Vite config and make the selector normalization logic easier to unit test or evolve independently.
## Individual Comments
### Comment 1
<location> `desktop/lib/backend-manager.js:670` </location>
<code_context>
- const expectedImageName = (
- path.basename(this.getPackagedBackendPath() || '') || 'astrbot-backend.exe'
- ).toLowerCase();
+ let backendConfig = null;
+ try {
+ backendConfig = this.getBackendConfig();
</code_context>
<issue_to_address>
**issue (complexity):** Consider simplifying the new backend config and unmanaged cleanup helpers by normalizing getBackendConfig, flattening resolveBackendConfig, and moving shared cleanup context into reusable/module-level state.
You can simplify a few of the new pieces without losing functionality.
### 1. Avoid rebuilding a rich context per PID in unmanaged cleanup
You currently build `contextBase` (including a new `Map` and log wrapper) for every cleanup call, and then spread it into each `shouldKillUnmanagedBackendProcess` invocation:
```js
const contextBase = {
backendConfig,
allowImageOnlyMatch: !hasBackendConfig,
commandLineCache: new Map(),
fallbackCmdRaw: process.env.ASTRBOT_BACKEND_CMD || 'python.exe',
spawnSync,
log: (message) => this.log(message),
};
for (const pid of pids) {
const processInfo = this.getWindowsProcessInfo(pid);
...
const shouldKill = shouldKillUnmanagedBackendProcess({
pid,
processInfo,
...contextBase,
});
...
}
```
Two concrete simplifications:
1. Move the “static” pieces into a class‑level field so they aren’t re‑allocated every time.
2. Let the helper module own the `commandLineCache` so the caller only passes in per‑call/per‑pid data.
Example:
```js
// in constructor
this.unmanagedCleanupBaseContext = {
fallbackCmdRaw: process.env.ASTRBOT_BACKEND_CMD || 'python.exe',
spawnSync,
log: (message) => this.log(message),
};
// in cleanup method
const backendConfig = this.getBackendConfig();
const hasBackendConfig = backendConfig && typeof backendConfig === 'object';
for (const pid of pids) {
const processInfo = this.getWindowsProcessInfo(pid);
if (!processInfo) {
this.log(`Skip unmanaged cleanup for pid=${pid}: unable to resolve process info.`);
continue;
}
const shouldKill = shouldKillUnmanagedBackendProcess({
pid,
processInfo,
backendConfig,
allowImageOnlyMatch: !hasBackendConfig,
...this.unmanagedCleanupBaseContext,
});
if (!shouldKill) continue;
spawnSync('taskkill', ['/pid', `${pid}`, '/t', '/f'], {
stdio: 'ignore',
windowsHide: true,
});
}
```
And inside `windows-backend-cleanup`, keep a module‑level `commandLineCache`:
```js
// windows-backend-cleanup.js
const commandLineCache = new Map();
function shouldKillUnmanagedBackendProcess({ pid, processInfo, backendConfig, allowImageOnlyMatch, fallbackCmdRaw, spawnSync, log }) {
// use commandLineCache here, no need to pass it from caller
}
```
This removes the per‑call `Map` and function allocation, and the caller only passes the bits the helper truly needs.
---
### 2. Make `getBackendConfig` always return a normalized shape for cleanup
The unmanaged cleanup path now has defensive try/catch + flags:
```js
let backendConfig = null;
try {
backendConfig = this.getBackendConfig();
} catch (error) {
this.log(`Failed to resolve backend config during unmanaged cleanup: ${...}`);
}
const hasBackendConfig = backendConfig && typeof backendConfig === 'object';
if (!hasBackendConfig) {
this.log(
'Backend config is unavailable during unmanaged cleanup; falling back to image-name-only matching.',
);
}
```
Given you control `getBackendConfig` / `resolveBackendConfig`, you can simplify the call site by normalizing the return value and avoiding exceptions entirely:
```js
getBackendConfig() {
if (!this.backendConfig) {
try {
this.backendConfig = this.resolveBackendConfig();
} catch (error) {
this.log(
`Failed to resolve backend config: ${
error instanceof Error ? error.message : String(error)
}`,
);
// Always return a normalized object
this.backendConfig = {
cmd: null,
args: [],
shell: true,
cwd: process.env.ASTRBOT_BACKEND_CWD || this.resolveBackendCwd(),
webuiDir: this.resolveWebuiDir(),
rootDir: process.env.ASTRBOT_ROOT || this.resolveBackendRoot(),
failureReason: 'Backend command is not configured.',
};
}
}
return this.backendConfig;
}
```
Then unmanaged cleanup can be much simpler:
```js
const backendConfig = this.getBackendConfig();
const hasBackendConfig = Boolean(backendConfig.cmd);
if (!hasBackendConfig) {
this.log(
'Backend command is not configured during unmanaged cleanup; falling back to image-name-only matching.',
);
}
// no try/catch needed here anymore
```
This keeps the “fallback to image‑only match” behavior but removes error‑based control flow from the cleanup routine.
---
### 3. Collapse `buildBackendConfig` into `resolveBackendConfig`
`buildBackendConfig` + `resolveBackendConfig` are now mostly plumbing to thread `failureReason`. You can keep `failureReason` while flattening the helpers into a single method, which makes the flow easier to follow:
Current:
```js
buildBackendConfig() {
const webuiDir = this.resolveWebuiDir();
let launch = null;
let failureReason = null;
...
return { launch, cwd, rootDir, webuiDir, failureReason };
}
resolveBackendConfig() {
const { launch, cwd, rootDir, webuiDir, failureReason } =
this.buildBackendConfig();
ensureDir(cwd);
if (rootDir) {
ensureDir(rootDir);
}
if (failureReason) {
this.log(failureReason);
}
this.backendConfig = {
cmd: launch ? launch.cmd : null,
args: launch ? launch.args : [],
shell: launch ? launch.shell : true,
cwd,
webuiDir,
rootDir,
failureReason,
};
return this.backendConfig;
}
```
Simplified, same behavior:
```js
resolveBackendConfig() {
const webuiDir = this.resolveWebuiDir();
let launch = null;
let failureReason = null;
const customCmd = process.env.ASTRBOT_BACKEND_CMD;
if (customCmd) {
launch = { cmd: customCmd, args: [], shell: true };
} else if (this.app.isPackaged) {
({ launch, failureReason } = this.buildLaunchForPackagedBackend(
this.getPackagedBackendState(),
webuiDir,
));
} else {
launch = this.buildDefaultBackendLaunch(webuiDir);
}
const cwd = process.env.ASTRBOT_BACKEND_CWD || this.resolveBackendCwd();
const rootDir = process.env.ASTRBOT_ROOT || this.resolveBackendRoot();
ensureDir(cwd);
if (rootDir) {
ensureDir(rootDir);
}
if (failureReason) {
this.log(failureReason);
}
this.backendConfig = {
cmd: launch ? launch.cmd : null,
args: launch ? launch.args : [],
shell: launch ? launch.shell : true,
cwd,
webuiDir,
rootDir,
failureReason,
};
return this.backendConfig;
}
```
You keep `failureReason` for reporting, but remove one layer of indirection and an intermediate object, which makes follow‑up changes less error‑prone.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并留下了一些总体反馈：
build-desktop和build-desktop-windows中的Resolve packaged CPython runtime source逻辑几乎是重复的；可以考虑把这部分提取到一个可复用的脚本或 composite action 中，这样能够保持 workflow 的 DRY（不重复）并在 python-build-standalone 参数变化时更容易更新。- 在
desktop/lib/windows-backend-cleanup.js中，全局的commandLineCache和commandLineQueryUnavailable标志位从未被重置，这对于只运行一次的进程来说没问题，但可能会让未来的重构变得更困难；可以考虑把这部分状态限制在清理操作的作用域内，或者提供一个显式的重置方法，以避免隐藏的跨调用耦合。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- The `Resolve packaged CPython runtime source` logic in `build-desktop` and `build-desktop-windows` is nearly duplicated; consider extracting this into a reusable script or composite action to keep the workflow DRY and easier to update when python-build-standalone parameters change.
- In `desktop/lib/windows-backend-cleanup.js`, the global `commandLineCache` and `commandLineQueryUnavailable` flags are never reset, which is fine for a single-run process but could make future refactors harder; consider scoping this state to the cleanup operation or exposing an explicit reset to avoid hidden cross-call coupling.
## Individual Comments
### Comment 1
<location> `astrbot/core/utils/pip_installer.py:491-492` </location>
<code_context>
_prepend_sys_path(target_site_packages)
args.extend(["--target", target_site_packages])
args.extend(["--upgrade", "--force-reinstall"])
+ pip_install_args = _strip_binary_selector_args(pip_install_args)
+ args.append("--only-binary=:all")
- if self.pip_install_arg:
</code_context>
<issue_to_address>
**issue (bug_risk):** The `--only-binary` value looks incorrect; this likely disables wheels instead of forcing them.
Pip uses the special form `--only-binary=:all:` (note the trailing colon) to mean “only use wheels for all packages.” `--only-binary=:all` instead targets a package literally named `all`, so it won’t actually enforce wheels globally and can break installs on systems without a compiler. Please change this to `--only-binary=:all:`.
</issue_to_address>帮我变得更有用！请对每条评论点选 👍 或 👎，我会基于你的反馈改进后续的评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The
Resolve packaged CPython runtime sourcelogic inbuild-desktopandbuild-desktop-windowsis nearly duplicated; consider extracting this into a reusable script or composite action to keep the workflow DRY and easier to update when python-build-standalone parameters change. - In
desktop/lib/windows-backend-cleanup.js, the globalcommandLineCacheandcommandLineQueryUnavailableflags are never reset, which is fine for a single-run process but could make future refactors harder; consider scoping this state to the cleanup operation or exposing an explicit reset to avoid hidden cross-call coupling.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `Resolve packaged CPython runtime source` logic in `build-desktop` and `build-desktop-windows` is nearly duplicated; consider extracting this into a reusable script or composite action to keep the workflow DRY and easier to update when python-build-standalone parameters change.
- In `desktop/lib/windows-backend-cleanup.js`, the global `commandLineCache` and `commandLineQueryUnavailable` flags are never reset, which is fine for a single-run process but could make future refactors harder; consider scoping this state to the cleanup operation or exposing an explicit reset to avoid hidden cross-call coupling.
## Individual Comments
### Comment 1
<location> `astrbot/core/utils/pip_installer.py:491-492` </location>
<code_context>
_prepend_sys_path(target_site_packages)
args.extend(["--target", target_site_packages])
args.extend(["--upgrade", "--force-reinstall"])
+ pip_install_args = _strip_binary_selector_args(pip_install_args)
+ args.append("--only-binary=:all")
- if self.pip_install_arg:
</code_context>
<issue_to_address>
**issue (bug_risk):** The `--only-binary` value looks incorrect; this likely disables wheels instead of forcing them.
Pip uses the special form `--only-binary=:all:` (note the trailing colon) to mean “only use wheels for all packages.” `--only-binary=:all` instead targets a package literally named `all`, so it won’t actually enforce wheels globally and can break installs on systems without a compiler. Please change this to `--only-binary=:all:`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
fix: use --only-binary=:all: and reset windows cleanup probe state
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并且留下了一些整体性的反馈：
- CI workflow 中打包运行时时使用的 Python 冒烟测试代码片段（Linux/macOS 的
build-desktop，以及 Windows 的build-desktop-windows）几乎完全相同；可以考虑把它们抽取到一个共享脚本中，或者复用.github/scripts/resolve_packaged_cpython_runtime.py，以避免重复并保持检查的一致性。 .github/scripts/resolve_packaged_cpython_runtime.py在下载 python-build-standalone 的 tarball 时没有做任何校验和或签名验证；可以考虑增加完整性校验（例如 SHA256），以增强打包运行时供应链的安全性。- 当前打包的后端构建在
desktop/scripts/build-backend.mjs中完全依赖requirements.txt来安装运行时依赖；如果pyproject.toml仍然是依赖项的单一事实来源，可以考虑为这个 requirements 文件增加文档说明，或者自动生成它，以避免两者之间产生偏差。
用于 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- The Python smoke-test snippets for the packaged runtime in the CI workflow (Linux/macOS `build-desktop` and Windows `build-desktop-windows`) are nearly identical; consider extracting them into a shared script or reusing `.github/scripts/resolve_packaged_cpython_runtime.py` to avoid duplication and keep checks consistent.
- `.github/scripts/resolve_packaged_cpython_runtime.py` downloads the python-build-standalone tarball without any checksum or signature verification; consider adding an integrity check (e.g., SHA256) to harden the supply chain for packaged runtimes.
- The packaged backend build currently installs runtime dependencies solely from `requirements.txt` in `desktop/scripts/build-backend.mjs`; if `pyproject.toml` remains the single source of truth for dependencies, consider documenting or automating generation of this requirements file to avoid drift between the two.
## Individual Comments
### Comment 1
<location> `desktop/lib/windows-backend-cleanup.js:6` </location>
<code_context>
+const path = require('path');
+
+const WINDOWS_PROCESS_QUERY_TIMEOUT_MS = 2000;
+const commandLineCache = new Map();
+let commandLineQueryUnavailable = false;
+let commandLineFallbackLogged = false;
</code_context>
<issue_to_address>
**issue (complexity):** Consider encapsulating the mutable globals into a WindowsBackendCleaner class and splitting out shell-invocation logic to simplify state management and make getWindowsProcessCommandLine easier to follow.
You can keep all current behavior while reducing complexity by:
### 1. Encapsulate global state into an object
Move `commandLineCache`, `commandLineQueryUnavailable`, and `commandLineFallbackLogged` into an instance so lifecycle is explicit and there’s no module-level mutable state:
```js
class WindowsBackendCleaner {
constructor() {
this.commandLineCache = new Map();
this.commandLineQueryUnavailable = false;
this.commandLineFallbackLogged = false;
}
resetState() {
this.commandLineCache.clear();
this.commandLineQueryUnavailable = false;
this.commandLineFallbackLogged = false;
}
// existing logic from shouldKillUnmanagedBackendProcess,
// but using this.commandLineCache / this.commandLineQueryUnavailable etc.
shouldKillUnmanagedBackendProcess(args) {
// ... same body as current function, but:
// - call this.getWindowsProcessCommandLine()
// - read/write this.commandLineFallbackLogged
}
getWindowsProcessCommandLine(params) {
// ... same logic as current function but use `this.*` instead of globals
}
}
module.exports = {
WindowsBackendCleaner,
};
```
Then call-site code holds one instance:
```js
const { WindowsBackendCleaner } = require('./windowsBackendCleanup');
const cleaner = new WindowsBackendCleaner();
// instead of resetWindowsBackendCleanupState()
cleaner.resetState();
// instead of shouldKillUnmanagedBackendProcess(...)
cleaner.shouldKillUnmanagedBackendProcess({ /* ... */ });
```
This removes the need for `resetWindowsBackendCleanupState()` as an external discipline and makes behavior easier to reason about across calls.
### 2. Split `getWindowsProcessCommandLine` responsibilities
You can simplify `getWindowsProcessCommandLine` by extracting the “try one shell” logic and the “choose shells” logic. This keeps error handling and logging localized and shortens the main function:
```js
class WindowsBackendCleaner {
// ...
_tryShellForCommandLine(shellName, numericPid, spawnSync, log, timeoutMs) {
const query = `$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${numericPid}"; if ($null -ne $p) { $p.CommandLine }`;
const args = ['-NoProfile', '-NonInteractive', '-Command', query];
const options = {
stdio: ['ignore', 'pipe', 'ignore'],
encoding: 'utf8',
windowsHide: true,
timeout: timeoutMs,
};
let result;
try {
result = spawnSync(shellName, args, options);
} catch (error) {
if (error instanceof Error && error.message) {
log(`Failed to query process command line by ${shellName} for pid=${numericPid}: ${error.message}`);
}
return { hasShell: false, commandLine: null, timedOut: false };
}
if (result.error?.code === 'ENOENT') {
return { hasShell: false, commandLine: null, timedOut: false };
}
if (result.error?.code === 'ETIMEDOUT') {
log(`Timed out (${timeoutMs}ms) querying process command line by ${shellName} for pid=${numericPid}.`);
return { hasShell: true, commandLine: null, timedOut: true };
}
if (result.error) {
if (result.error.message) {
log(`Failed to query process command line by ${shellName} for pid=${numericPid}: ${result.error.message}`);
}
return { hasShell: true, commandLine: null, timedOut: false };
}
if (result.status === 0) {
const commandLine =
result.stdout
.split(/\r?\n/)
.map((item) => item.trim())
.find((item) => item.length > 0) || null;
return { hasShell: true, commandLine, timedOut: false };
}
return { hasShell: true, commandLine: null, timedOut: false };
}
getWindowsProcessCommandLine({ pid, spawnSync, log, timeoutMs }) {
const numericPid = Number.parseInt(`${pid}`, 10);
if (!Number.isInteger(numericPid)) {
return { commandLine: null, commandLineQueryUnavailable: false };
}
if (this.commandLineQueryUnavailable) {
return { commandLine: null, commandLineQueryUnavailable: true };
}
if (this.commandLineCache.has(numericPid)) {
return {
commandLine: this.commandLineCache.get(numericPid),
commandLineQueryUnavailable: false,
};
}
const shells = ['powershell', 'pwsh'];
let hasAvailableShell = false;
for (const shellName of shells) {
const { hasShell, commandLine } = this._tryShellForCommandLine(
shellName,
numericPid,
spawnSync,
log,
timeoutMs,
);
if (!hasShell) {
continue;
}
hasAvailableShell = true;
if (commandLine !== null) {
this.commandLineCache.set(numericPid, commandLine);
this.commandLineQueryUnavailable = false;
return { commandLine, commandLineQueryUnavailable: false };
}
}
if (!hasAvailableShell) {
this.commandLineQueryUnavailable = true;
return { commandLine: null, commandLineQueryUnavailable: true };
}
this.commandLineCache.set(numericPid, null);
this.commandLineQueryUnavailable = false;
return { commandLine: null, commandLineQueryUnavailable: false };
}
}
```
This keeps the overall behavior and logging identical, but reduces nesting and separates spawn-related concerns from cache/state updates.
These two steps alone should make the module much easier to understand and evolve without changing any external behavior.
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The Python smoke-test snippets for the packaged runtime in the CI workflow (Linux/macOS
build-desktopand Windowsbuild-desktop-windows) are nearly identical; consider extracting them into a shared script or reusing.github/scripts/resolve_packaged_cpython_runtime.pyto avoid duplication and keep checks consistent. .github/scripts/resolve_packaged_cpython_runtime.pydownloads the python-build-standalone tarball without any checksum or signature verification; consider adding an integrity check (e.g., SHA256) to harden the supply chain for packaged runtimes.- The packaged backend build currently installs runtime dependencies solely from
requirements.txtindesktop/scripts/build-backend.mjs; ifpyproject.tomlremains the single source of truth for dependencies, consider documenting or automating generation of this requirements file to avoid drift between the two.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The Python smoke-test snippets for the packaged runtime in the CI workflow (Linux/macOS `build-desktop` and Windows `build-desktop-windows`) are nearly identical; consider extracting them into a shared script or reusing `.github/scripts/resolve_packaged_cpython_runtime.py` to avoid duplication and keep checks consistent.
- `.github/scripts/resolve_packaged_cpython_runtime.py` downloads the python-build-standalone tarball without any checksum or signature verification; consider adding an integrity check (e.g., SHA256) to harden the supply chain for packaged runtimes.
- The packaged backend build currently installs runtime dependencies solely from `requirements.txt` in `desktop/scripts/build-backend.mjs`; if `pyproject.toml` remains the single source of truth for dependencies, consider documenting or automating generation of this requirements file to avoid drift between the two.
## Individual Comments
### Comment 1
<location> `desktop/lib/windows-backend-cleanup.js:6` </location>
<code_context>
+const path = require('path');
+
+const WINDOWS_PROCESS_QUERY_TIMEOUT_MS = 2000;
+const commandLineCache = new Map();
+let commandLineQueryUnavailable = false;
+let commandLineFallbackLogged = false;
</code_context>
<issue_to_address>
**issue (complexity):** Consider encapsulating the mutable globals into a WindowsBackendCleaner class and splitting out shell-invocation logic to simplify state management and make getWindowsProcessCommandLine easier to follow.
You can keep all current behavior while reducing complexity by:
### 1. Encapsulate global state into an object
Move `commandLineCache`, `commandLineQueryUnavailable`, and `commandLineFallbackLogged` into an instance so lifecycle is explicit and there’s no module-level mutable state:
```js
class WindowsBackendCleaner {
constructor() {
this.commandLineCache = new Map();
this.commandLineQueryUnavailable = false;
this.commandLineFallbackLogged = false;
}
resetState() {
this.commandLineCache.clear();
this.commandLineQueryUnavailable = false;
this.commandLineFallbackLogged = false;
}
// existing logic from shouldKillUnmanagedBackendProcess,
// but using this.commandLineCache / this.commandLineQueryUnavailable etc.
shouldKillUnmanagedBackendProcess(args) {
// ... same body as current function, but:
// - call this.getWindowsProcessCommandLine()
// - read/write this.commandLineFallbackLogged
}
getWindowsProcessCommandLine(params) {
// ... same logic as current function but use `this.*` instead of globals
}
}
module.exports = {
WindowsBackendCleaner,
};
```
Then call-site code holds one instance:
```js
const { WindowsBackendCleaner } = require('./windowsBackendCleanup');
const cleaner = new WindowsBackendCleaner();
// instead of resetWindowsBackendCleanupState()
cleaner.resetState();
// instead of shouldKillUnmanagedBackendProcess(...)
cleaner.shouldKillUnmanagedBackendProcess({ /* ... */ });
```
This removes the need for `resetWindowsBackendCleanupState()` as an external discipline and makes behavior easier to reason about across calls.
### 2. Split `getWindowsProcessCommandLine` responsibilities
You can simplify `getWindowsProcessCommandLine` by extracting the “try one shell” logic and the “choose shells” logic. This keeps error handling and logging localized and shortens the main function:
```js
class WindowsBackendCleaner {
// ...
_tryShellForCommandLine(shellName, numericPid, spawnSync, log, timeoutMs) {
const query = `$p = Get-CimInstance Win32_Process -Filter "ProcessId = ${numericPid}"; if ($null -ne $p) { $p.CommandLine }`;
const args = ['-NoProfile', '-NonInteractive', '-Command', query];
const options = {
stdio: ['ignore', 'pipe', 'ignore'],
encoding: 'utf8',
windowsHide: true,
timeout: timeoutMs,
};
let result;
try {
result = spawnSync(shellName, args, options);
} catch (error) {
if (error instanceof Error && error.message) {
log(`Failed to query process command line by ${shellName} for pid=${numericPid}: ${error.message}`);
}
return { hasShell: false, commandLine: null, timedOut: false };
}
if (result.error?.code === 'ENOENT') {
return { hasShell: false, commandLine: null, timedOut: false };
}
if (result.error?.code === 'ETIMEDOUT') {
log(`Timed out (${timeoutMs}ms) querying process command line by ${shellName} for pid=${numericPid}.`);
return { hasShell: true, commandLine: null, timedOut: true };
}
if (result.error) {
if (result.error.message) {
log(`Failed to query process command line by ${shellName} for pid=${numericPid}: ${result.error.message}`);
}
return { hasShell: true, commandLine: null, timedOut: false };
}
if (result.status === 0) {
const commandLine =
result.stdout
.split(/\r?\n/)
.map((item) => item.trim())
.find((item) => item.length > 0) || null;
return { hasShell: true, commandLine, timedOut: false };
}
return { hasShell: true, commandLine: null, timedOut: false };
}
getWindowsProcessCommandLine({ pid, spawnSync, log, timeoutMs }) {
const numericPid = Number.parseInt(`${pid}`, 10);
if (!Number.isInteger(numericPid)) {
return { commandLine: null, commandLineQueryUnavailable: false };
}
if (this.commandLineQueryUnavailable) {
return { commandLine: null, commandLineQueryUnavailable: true };
}
if (this.commandLineCache.has(numericPid)) {
return {
commandLine: this.commandLineCache.get(numericPid),
commandLineQueryUnavailable: false,
};
}
const shells = ['powershell', 'pwsh'];
let hasAvailableShell = false;
for (const shellName of shells) {
const { hasShell, commandLine } = this._tryShellForCommandLine(
shellName,
numericPid,
spawnSync,
log,
timeoutMs,
);
if (!hasShell) {
continue;
}
hasAvailableShell = true;
if (commandLine !== null) {
this.commandLineCache.set(numericPid, commandLine);
this.commandLineQueryUnavailable = false;
return { commandLine, commandLineQueryUnavailable: false };
}
}
if (!hasAvailableShell) {
this.commandLineQueryUnavailable = true;
return { commandLine: null, commandLineQueryUnavailable: true };
}
this.commandLineCache.set(numericPid, null);
this.commandLineQueryUnavailable = false;
return { commandLine: null, commandLineQueryUnavailable: false };
}
}
```
This keeps the overall behavior and logging identical, but reduces nesting and separates spawn-related concerns from cache/state updates.
These two steps alone should make the module much easier to understand and evolve without changing any external behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
嗨——我已经审查了你的改动，看起来非常棒！
帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈来改进评审质量。
Original comment in English
Hey - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些高层面的反馈：
- python-build-standalone 的 release/version/target 值现在被硬编码在多个位置（发布工作流矩阵、运行时解析脚本、README 示例）中；建议把它们集中管理（例如通过 workflow env/inputs 或共享配置），以降低未来升级运行时时出现版本漂移的风险。
- WindowsBackendCleaner 目前是针对每个 PID 调用一次 PowerShell/pwsh 来解析命令行；如果需要扫描很多 PID，这可能会带来明显的额外开销，因此你可以考虑对查询进行批处理，或者在更高层级缓存“shell 不可用”的结果，以避免重复创建进程。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- python-build-standalone 的 release/version/target 值现在被硬编码在多个位置（发布工作流矩阵、运行时解析脚本、README 示例）中；建议把它们集中管理（例如通过 workflow env/inputs 或共享配置），以降低未来升级运行时时出现版本漂移的风险。
- WindowsBackendCleaner 目前是针对每个 PID 调用一次 PowerShell/pwsh 来解析命令行；如果需要扫描很多 PID，这可能会带来明显的额外开销，因此你可以考虑对查询进行批处理，或者在更高层级缓存“shell 不可用”的结果，以避免重复创建进程。
## Individual Comments
### Comment 1
<location> `desktop/lib/backend-manager.js:174` </location>
<code_context>
+ };
+ }
+
resolveBackendConfig() {
const webuiDir = this.resolveWebuiDir();
+ let launch = null;
</code_context>
<issue_to_address>
**issue (complexity):** 建议把后端配置解析重构为一个返回可判别联合类型的辅助函数，并把 Windows 清理逻辑改为纯函数，以简化控制流和状态管理。
你可以在不改变行为的前提下，通过收紧两个主要区域来降低新增的复杂度：
---
### 1. 将后端解析集中到一个可判别联合中
目前 `failureReason` 被贯穿在以下路径中：
- `buildLaunchForPackagedBackend`
- `resolveBackendConfig`
- `getBackendConfig`
- `getBackendCommandUnavailableReason`
- 以及它们的调用方。
你可以把这部分封装到一个返回可判别联合的辅助函数中，让 `getBackendConfig()` 成为唯一一个把其结果写入 `this.backendConfig` 的地方。这样就不需要在 config 上存储 `failureReason`，也不需要让 `getBackendCommandUnavailableReason()` 再去重新解释它。
例如：
```js
// new helper, pure, no `this`:
function resolveBackendConfigResult({
app,
env,
resourcesPath,
log,
resolveWebuiDir,
resolveBackendCwd,
resolveBackendRoot,
getPackagedBackendState,
buildDefaultBackendLaunch,
buildLaunchForPackagedBackend,
}) {
const webuiDir = resolveWebuiDir();
const customCmd = env.ASTRBOT_BACKEND_CMD;
let launch = null;
let failureReason = null;
if (customCmd) {
launch = { cmd: customCmd, args: [], shell: true };
} else if (app.isPackaged) {
({ launch, failureReason } = buildLaunchForPackagedBackend(
getPackagedBackendState(),
webuiDir,
));
} else {
launch = buildDefaultBackendLaunch(webuiDir);
}
const cwd = env.ASTRBOT_BACKEND_CWD || resolveBackendCwd();
const rootDir = env.ASTRBOT_ROOT || resolveBackendRoot();
ensureDir(cwd);
if (rootDir) ensureDir(rootDir);
if (failureReason) {
log(failureReason);
return {
ok: false,
reason: failureReason,
config: {
cmd: null,
args: [],
shell: true,
cwd,
webuiDir,
rootDir,
},
};
}
return {
ok: Boolean(launch && launch.cmd),
reason: failureReason,
config: {
cmd: launch ? launch.cmd : null,
args: launch ? launch.args : [],
shell: launch ? launch.shell : true,
cwd,
webuiDir,
rootDir,
},
};
}
```
然后 `getBackendConfig` 和“不可用原因”的实现就会更简单、更具权威性：
```js
getBackendConfig() {
if (!this.backendConfig) {
try {
const result = resolveBackendConfigResult({
app: this.app,
env: process.env,
resourcesPath: process.resourcesPath,
log: (m) => this.log(m),
resolveWebuiDir: () => this.resolveWebuiDir(),
resolveBackendCwd: () => this.resolveBackendCwd(),
resolveBackendRoot: () => this.resolveBackendRoot(),
getPackagedBackendState: () => this.getPackagedBackendState(),
buildDefaultBackendLaunch: (webuiDir) =>
this.buildDefaultBackendLaunch(webuiDir),
buildLaunchForPackagedBackend: (state, webuiDir) =>
this.buildLaunchForPackagedBackend(state, webuiDir),
});
this.backendConfig = {
...result.config,
// keep reason for external consumption if you want
failureReason: result.reason,
};
} catch (error) {
const msg =
error instanceof Error ? error.message : String(error);
this.log(`Failed to resolve backend config: ${msg}`);
this.backendConfig = {
cmd: null,
args: [],
shell: true,
cwd: process.env.ASTRBOT_BACKEND_CWD || this.resolveBackendCwd(),
webuiDir: this.resolveWebuiDir(),
rootDir: process.env.ASTRBOT_ROOT || this.resolveBackendRoot(),
failureReason: 'Backend command is not configured.',
};
}
}
return this.backendConfig;
}
getBackendCommandUnavailableReason() {
const config = this.getBackendConfig();
return config.failureReason || 'Backend command is not configured.';
}
```
这样：
- 只有一个权威位置负责计算 `(cmd | null, reason | null)`。
- 像 `startBackendAndWait`、`restartBackend`、`ensureBackend` 这样的调用方都使用同一个 `getBackendCommandUnavailableReason()`，而不需要了解内部如何传递这些信息。
---
### 2. 把 Windows 清理逻辑改为纯辅助函数，而不是类 + 基础上下文
`this.unmanagedCleanupBaseContext`、`this.windowsBackendCleaner.resetState()`，以及把 `{ ...backendConfig, ...this.unmanagedCleanupBaseContext }` 传给一个有状态类的方法，这些都让一个本该简单的“我们是否应该杀掉这个 PID？”决策路径变得更难理解。
你可以把大部分逻辑下沉到一个纯函数中，该函数只接收显式参数并返回一个布尔值。底层逻辑可以保持不变，只是去掉隐式的共享上下文。
例如，与其这样写：
```js
this.windowsBackendCleaner.resetState();
const backendConfig = this.getBackendConfig();
const hasBackendCommand = Boolean(backendConfig.cmd);
// ...
const shouldKill = this.windowsBackendCleaner.shouldKillUnmanagedBackendProcess({
pid,
processInfo,
backendConfig,
allowImageOnlyMatch: !hasBackendCommand,
...this.unmanagedCleanupBaseContext,
});
```
不如把决策逻辑拆出来变成一个独立辅助函数：
```js
// windows-backend-cleanup.js
function shouldKillUnmanagedBackendProcess({
pid,
processInfo,
backendConfig,
allowImageOnlyMatch,
fallbackCmdRaw,
spawnSync,
log,
}) {
// existing logic from WindowsBackendCleaner, but:
// - no `this`
// - no internal mutable state
// - all needed dependencies are passed in
// return true/false
}
module.exports = {
shouldKillUnmanagedBackendProcess,
};
```
这样调用端就变为：
```js
const backendConfig = this.getBackendConfig();
const hasBackendCommand = Boolean(backendConfig.cmd);
if (!hasBackendCommand) {
this.log(
'Backend command is not configured during unmanaged cleanup; falling back to image-name-only matching.',
);
}
for (const pid of pids) {
const processInfo = this.getWindowsProcessInfo(pid);
if (!processInfo) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process info.`,
);
continue;
}
const shouldKill = shouldKillUnmanagedBackendProcess({
pid,
processInfo,
backendConfig,
allowImageOnlyMatch: !hasBackendCommand,
fallbackCmdRaw: process.env.ASTRBOT_BACKEND_CMD || 'python.exe',
spawnSync,
log: (message) => this.log(message),
});
if (!shouldKill) continue;
spawnSync('taskkill', ['/pid', `${pid}`, '/t', '/f'], {
stdio: 'ignore',
windowsHide: true,
});
}
```
这样做：
- 不再需要 `this.unmanagedCleanupBaseContext`。
- 去掉了 `WindowsBackendCleaner` 的内部状态和 `resetState`。
- 让清理决策变得非常容易测试，也更容易理解（纯函数）。
---
这两个重构可以保留你新增的运行时/清理行为，但会显著降低跨方法耦合，以及读者需要跟踪的“真值来源”数量。
</issue_to_address>帮我变得更有用！请对每条评论点 👍 或 👎，我会根据反馈改进后续的代码审查。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The python-build-standalone release/version/target values are now hardcoded in multiple places (release workflow matrices, runtime resolver script, README examples); consider centralizing them (e.g. workflow env/inputs or a shared config) to reduce the risk of future drift when bumping runtime versions.
- WindowsBackendCleaner queries PowerShell/pwsh once per PID to resolve command lines; if many PIDs are scanned this may add noticeable overhead, so you might want to batch queries or cache negative shell availability at a higher level to avoid repeated process launches.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The python-build-standalone release/version/target values are now hardcoded in multiple places (release workflow matrices, runtime resolver script, README examples); consider centralizing them (e.g. workflow env/inputs or a shared config) to reduce the risk of future drift when bumping runtime versions.
- WindowsBackendCleaner queries PowerShell/pwsh once per PID to resolve command lines; if many PIDs are scanned this may add noticeable overhead, so you might want to batch queries or cache negative shell availability at a higher level to avoid repeated process launches.
## Individual Comments
### Comment 1
<location> `desktop/lib/backend-manager.js:174` </location>
<code_context>
+ };
+ }
+
resolveBackendConfig() {
const webuiDir = this.resolveWebuiDir();
+ let launch = null;
</code_context>
<issue_to_address>
**issue (complexity):** Consider refactoring backend configuration resolution into a single discriminated-union helper and making Windows cleanup a pure function to simplify control flow and state management.
You can reduce the new complexity without changing behaviour by tightening two main areas:
---
### 1. Centralize backend resolution into a discriminated union
Right now `failureReason` is threaded through:
- `buildLaunchForPackagedBackend`
- `resolveBackendConfig`
- `getBackendConfig`
- `getBackendCommandUnavailableReason`
- plus callers.
You can wrap this in a single helper that returns a discriminated union, and let `getBackendConfig()` be the only place that turns that into `this.backendConfig`. That removes the need to store `failureReason` on the config and to have `getBackendCommandUnavailableReason()` reinterpret it.
For example:
```js
// new helper, pure, no `this`:
function resolveBackendConfigResult({
app,
env,
resourcesPath,
log,
resolveWebuiDir,
resolveBackendCwd,
resolveBackendRoot,
getPackagedBackendState,
buildDefaultBackendLaunch,
buildLaunchForPackagedBackend,
}) {
const webuiDir = resolveWebuiDir();
const customCmd = env.ASTRBOT_BACKEND_CMD;
let launch = null;
let failureReason = null;
if (customCmd) {
launch = { cmd: customCmd, args: [], shell: true };
} else if (app.isPackaged) {
({ launch, failureReason } = buildLaunchForPackagedBackend(
getPackagedBackendState(),
webuiDir,
));
} else {
launch = buildDefaultBackendLaunch(webuiDir);
}
const cwd = env.ASTRBOT_BACKEND_CWD || resolveBackendCwd();
const rootDir = env.ASTRBOT_ROOT || resolveBackendRoot();
ensureDir(cwd);
if (rootDir) ensureDir(rootDir);
if (failureReason) {
log(failureReason);
return {
ok: false,
reason: failureReason,
config: {
cmd: null,
args: [],
shell: true,
cwd,
webuiDir,
rootDir,
},
};
}
return {
ok: Boolean(launch && launch.cmd),
reason: failureReason,
config: {
cmd: launch ? launch.cmd : null,
args: launch ? launch.args : [],
shell: launch ? launch.shell : true,
cwd,
webuiDir,
rootDir,
},
};
}
```
Then `getBackendConfig` and the “unavailable reason” become much simpler and canonical:
```js
getBackendConfig() {
if (!this.backendConfig) {
try {
const result = resolveBackendConfigResult({
app: this.app,
env: process.env,
resourcesPath: process.resourcesPath,
log: (m) => this.log(m),
resolveWebuiDir: () => this.resolveWebuiDir(),
resolveBackendCwd: () => this.resolveBackendCwd(),
resolveBackendRoot: () => this.resolveBackendRoot(),
getPackagedBackendState: () => this.getPackagedBackendState(),
buildDefaultBackendLaunch: (webuiDir) =>
this.buildDefaultBackendLaunch(webuiDir),
buildLaunchForPackagedBackend: (state, webuiDir) =>
this.buildLaunchForPackagedBackend(state, webuiDir),
});
this.backendConfig = {
...result.config,
// keep reason for external consumption if you want
failureReason: result.reason,
};
} catch (error) {
const msg =
error instanceof Error ? error.message : String(error);
this.log(`Failed to resolve backend config: ${msg}`);
this.backendConfig = {
cmd: null,
args: [],
shell: true,
cwd: process.env.ASTRBOT_BACKEND_CWD || this.resolveBackendCwd(),
webuiDir: this.resolveWebuiDir(),
rootDir: process.env.ASTRBOT_ROOT || this.resolveBackendRoot(),
failureReason: 'Backend command is not configured.',
};
}
}
return this.backendConfig;
}
getBackendCommandUnavailableReason() {
const config = this.getBackendConfig();
return config.failureReason || 'Backend command is not configured.';
}
```
This way:
- There is one canonical place where `(cmd | null, reason | null)` is computed.
- Callers like `startBackendAndWait`, `restartBackend`, `ensureBackend` all use the same `getBackendCommandUnavailableReason()` without needing to understand the internal plumbing.
---
### 2. Make Windows cleanup a pure helper instead of class + base context
`this.unmanagedCleanupBaseContext`, `this.windowsBackendCleaner.resetState()`, and passing `{ ...backendConfig, ...this.unmanagedCleanupBaseContext }` into a method on a stateful class make a simple “should we kill this PID?” decision path harder to follow.
You can push most of that into a pure function that takes explicit parameters and returns a boolean. You can keep the underlying logic exactly the same; just remove the implicit shared context.
For example, instead of:
```js
this.windowsBackendCleaner.resetState();
const backendConfig = this.getBackendConfig();
const hasBackendCommand = Boolean(backendConfig.cmd);
// ...
const shouldKill = this.windowsBackendCleaner.shouldKillUnmanagedBackendProcess({
pid,
processInfo,
backendConfig,
allowImageOnlyMatch: !hasBackendCommand,
...this.unmanagedCleanupBaseContext,
});
```
Make the decision logic a standalone helper:
```js
// windows-backend-cleanup.js
function shouldKillUnmanagedBackendProcess({
pid,
processInfo,
backendConfig,
allowImageOnlyMatch,
fallbackCmdRaw,
spawnSync,
log,
}) {
// existing logic from WindowsBackendCleaner, but:
// - no `this`
// - no internal mutable state
// - all needed dependencies are passed in
// return true/false
}
module.exports = {
shouldKillUnmanagedBackendProcess,
};
```
Then the call site becomes:
```js
const backendConfig = this.getBackendConfig();
const hasBackendCommand = Boolean(backendConfig.cmd);
if (!hasBackendCommand) {
this.log(
'Backend command is not configured during unmanaged cleanup; falling back to image-name-only matching.',
);
}
for (const pid of pids) {
const processInfo = this.getWindowsProcessInfo(pid);
if (!processInfo) {
this.log(
`Skip unmanaged cleanup for pid=${pid}: unable to resolve process info.`,
);
continue;
}
const shouldKill = shouldKillUnmanagedBackendProcess({
pid,
processInfo,
backendConfig,
allowImageOnlyMatch: !hasBackendCommand,
fallbackCmdRaw: process.env.ASTRBOT_BACKEND_CMD || 'python.exe',
spawnSync,
log: (message) => this.log(message),
});
if (!shouldKill) continue;
spawnSync('taskkill', ['/pid', `${pid}`, '/t', '/f'], {
stdio: 'ignore',
windowsHide: true,
});
}
```
This:
- Removes the need for `this.unmanagedCleanupBaseContext`.
- Removes `WindowsBackendCleaner` state and `resetState`.
- Makes the cleanup decision trivially testable and easier to reason about (pure function).
---
These two refactorings preserve your new runtime/cleanup behaviour but substantially reduce cross-method coupling and the number of “sources of truth” a reader has to track.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
@sourcery-ai review |
Contributor
There was a problem hiding this comment.
Hey - 我已经审查了你的更改，看起来很棒！
帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈来改进后续的审查。
Original comment in English
Hey - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Member
Author
|
桌面app重构已迁移至：AstrBot-desktop |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概述
本 PR 将桌面端后端分发从 PyInstaller 单文件方案迁移为“内置 CPython 运行时 + 后端源码”，并将分发基线切换为
python-build-standalone。同时补齐了 Windows 打包场景下的运行时依赖、编码输出与重启链路稳定性问题。主要变更
desktop/scripts/build-backend.mjs，产出resources/backend/{app,python}。runtime-manifest.json，统一打包后运行时解析与启动参数。cryptography源码编译（改为二进制策略）。msvc-runtime，修复_greenlet/_rust等 DLL 依赖问题。PYTHONUTF8=1、PYTHONIOENCODING=utf-8。launch_backend.py启动阶段增加标准流 UTF-8 配置，减少 GBK 编码报错和乱码。--only-binary=:all。pandas/cryptography安装失败）。直接解决的问题
ImportError: DLL load failed while importing _rustValueError: the greenlet library is required ... DLL load failed while importing _greenletUnicodeEncodeError: 'gbk' codec can't encode character ...验证
uv run ruff format astrbot/core/utils/pip_installer.pyuv run ruff check astrbot/core/utils/pip_installer.pynode --check desktop/main.jsAnalyze (python)、Run smoke tests、build、format-check、CodeQL）。由 Sourcery 提供的总结
将桌面端后端打包方式迁移为内嵌 CPython 运行时和基于源码的后端负载，引入基于清单（manifest）的启动机制，并强化 Windows 构建和运行时行为。
新功能：
缺陷修复：
增强改进：
构建：
CI：
文档：
Original summary in English
由 Sourcery 提供的摘要
将桌面端后端迁移到捆绑的 CPython 运行时，采用基于清单（manifest）的启动方式，强化版本/运行时校验，并重构发布工作流，以使用 python-build-standalone（含独立的 Windows 构建和运行时冒烟测试）。
新特性：
requires-python或显式环境变量的运行时版本检测，用于验证打包的 Python 运行时。缺陷修复：
增强改进：
pip install参数。构建（Build）：
workflow_dispatch的 ref 和 tag 处理，要求显式 tag，并在没有 ref 覆盖时使用github.ref_name。持续集成（CI）：
文档：
Original summary in English
由 Sourcery 提供的摘要
将桌面端后端从 PyInstaller 单文件可执行迁移为：打包的独立 CPython 运行时 + 源代码 + 基于清单（manifest）的启动器，并强化 Windows 运行时行为与发布工作流。
新功能：
pyproject中的requires-python或显式的环境配置确定版本。缺陷修复：
greenlet、cryptography及相关模块的 DLL 依赖问题。增强改进：
构建：
python-build-standalone作为各平台打包运行时的来源，并在 CI 中提供脚本用于解析目标、下载、校验并准备运行时。cryptographywheels，并统一标准化制品命名。CI：
workflow_dispatch必须显式指定 tag，并默认使用github.ref_name作为构建 ref。文档：
python-build-standalone用法、后端布局以及常见构建错误。Original summary in English
Summary by Sourcery
Migrate the desktop backend from a PyInstaller single binary to a bundled CPython runtime plus source with a manifest-driven launcher, and harden Windows runtime behavior and release workflows.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Original summary in English
由 Sourcery 提供的摘要
将桌面端后端从 PyInstaller 单文件可执行迁移为：打包的独立 CPython 运行时 + 源代码 + 基于清单（manifest）的启动器，并强化 Windows 运行时行为与发布工作流。
新功能：
pyproject中的requires-python或显式的环境配置确定版本。缺陷修复：
greenlet、cryptography及相关模块的 DLL 依赖问题。增强改进：
构建：
python-build-standalone作为各平台打包运行时的来源，并在 CI 中提供脚本用于解析目标、下载、校验并准备运行时。cryptographywheels，并统一标准化制品命名。CI：
workflow_dispatch必须显式指定 tag，并默认使用github.ref_name作为构建 ref。文档：
python-build-standalone用法、后端布局以及常见构建错误。Original summary in English
Summary by Sourcery
Migrate the desktop backend from a PyInstaller single binary to a bundled CPython runtime plus source with a manifest-driven launcher, and harden Windows runtime behavior and release workflows.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Original summary in English
由 Sourcery 提供的摘要
将桌面端后端从 PyInstaller 单文件可执行迁移为：打包的独立 CPython 运行时 + 源代码 + 基于清单（manifest）的启动器，并强化 Windows 运行时行为与发布工作流。
新功能：
pyproject中的requires-python或显式的环境配置确定版本。缺陷修复：
greenlet、cryptography及相关模块的 DLL 依赖问题。增强改进：
构建：
python-build-standalone作为各平台打包运行时的来源，并在 CI 中提供脚本用于解析目标、下载、校验并准备运行时。cryptographywheels，并统一标准化制品命名。CI：
workflow_dispatch必须显式指定 tag，并默认使用github.ref_name作为构建 ref。文档：
python-build-standalone用法、后端布局以及常见构建错误。Original summary in English
Summary by Sourcery
Migrate the desktop backend from a PyInstaller single binary to a bundled CPython runtime plus source with a manifest-driven launcher, and harden Windows runtime behavior and release workflows.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Original summary in English
由 Sourcery 提供的摘要
将桌面端后端从 PyInstaller 单文件可执行迁移为：打包的独立 CPython 运行时 + 源代码 + 基于清单（manifest）的启动器，并强化 Windows 运行时行为与发布工作流。
新功能：
pyproject中的requires-python或显式的环境配置确定版本。缺陷修复：
greenlet、cryptography及相关模块的 DLL 依赖问题。增强改进：
构建：
python-build-standalone作为各平台打包运行时的来源，并在 CI 中提供脚本用于解析目标、下载、校验并准备运行时。cryptographywheels，并统一标准化制品命名。CI：
workflow_dispatch必须显式指定 tag，并默认使用github.ref_name作为构建 ref。文档：
python-build-standalone用法、后端布局以及常见构建错误。Original summary in English
Summary by Sourcery
Migrate the desktop backend from a PyInstaller single binary to a bundled CPython runtime plus source with a manifest-driven launcher, and harden Windows runtime behavior and release workflows.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Original summary in English
由 Sourcery 提供的摘要
将桌面端后端迁移到捆绑的 CPython 运行时，采用基于清单（manifest）的启动方式，强化版本/运行时校验，并重构发布工作流，以使用 python-build-standalone（含独立的 Windows 构建和运行时冒烟测试）。
新特性：
requires-python或显式环境变量的运行时版本检测，用于验证打包的 Python 运行时。缺陷修复：
增强改进：
pip install参数。构建（Build）：
workflow_dispatch的 ref 和 tag 处理，要求显式 tag，并在没有 ref 覆盖时使用github.ref_name。持续集成（CI）：
文档：
Original summary in English
由 Sourcery 提供的摘要
将桌面端后端从 PyInstaller 单文件可执行迁移为：打包的独立 CPython 运行时 + 源代码 + 基于清单（manifest）的启动器，并强化 Windows 运行时行为与发布工作流。
新功能：
pyproject中的requires-python或显式的环境配置确定版本。缺陷修复：
greenlet、cryptography及相关模块的 DLL 依赖问题。增强改进：
构建：
python-build-standalone作为各平台打包运行时的来源，并在 CI 中提供脚本用于解析目标、下载、校验并准备运行时。cryptographywheels，并统一标准化制品命名。CI：
workflow_dispatch必须显式指定 tag，并默认使用github.ref_name作为构建 ref。文档：
python-build-standalone用法、后端布局以及常见构建错误。Original summary in English
Summary by Sourcery
Migrate the desktop backend from a PyInstaller single binary to a bundled CPython runtime plus source with a manifest-driven launcher, and harden Windows runtime behavior and release workflows.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Original summary in English
由 Sourcery 提供的摘要
将桌面端后端从 PyInstaller 单文件可执行迁移为：打包的独立 CPython 运行时 + 源代码 + 基于清单（manifest）的启动器，并强化 Windows 运行时行为与发布工作流。
新功能：
pyproject中的requires-python或显式的环境配置确定版本。缺陷修复：
greenlet、cryptography及相关模块的 DLL 依赖问题。增强改进：
构建：
python-build-standalone作为各平台打包运行时的来源，并在 CI 中提供脚本用于解析目标、下载、校验并准备运行时。cryptographywheels，并统一标准化制品命名。CI：
workflow_dispatch必须显式指定 tag，并默认使用github.ref_name作为构建 ref。文档：
python-build-standalone用法、后端布局以及常见构建错误。Original summary in English
Summary by Sourcery
Migrate the desktop backend from a PyInstaller single binary to a bundled CPython runtime plus source with a manifest-driven launcher, and harden Windows runtime behavior and release workflows.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Original summary in English
由 Sourcery 提供的摘要
将桌面端后端从 PyInstaller 单文件可执行迁移为：打包的独立 CPython 运行时 + 源代码 + 基于清单（manifest）的启动器，并强化 Windows 运行时行为与发布工作流。
新功能：
pyproject中的requires-python或显式的环境配置确定版本。缺陷修复：
greenlet、cryptography及相关模块的 DLL 依赖问题。增强改进：
构建：
python-build-standalone作为各平台打包运行时的来源，并在 CI 中提供脚本用于解析目标、下载、校验并准备运行时。cryptographywheels，并统一标准化制品命名。CI：
workflow_dispatch必须显式指定 tag，并默认使用github.ref_name作为构建 ref。文档：
python-build-standalone用法、后端布局以及常见构建错误。Original summary in English
Summary by Sourcery
Migrate the desktop backend from a PyInstaller single binary to a bundled CPython runtime plus source with a manifest-driven launcher, and harden Windows runtime behavior and release workflows.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Original summary in English
由 Sourcery 提供的摘要
将桌面端后端从 PyInstaller 单文件可执行迁移为：打包的独立 CPython 运行时 + 源代码 + 基于清单（manifest）的启动器，并强化 Windows 运行时行为与发布工作流。
新功能：
pyproject中的requires-python或显式的环境配置确定版本。缺陷修复：
greenlet、cryptography及相关模块的 DLL 依赖问题。增强改进：
构建：
python-build-standalone作为各平台打包运行时的来源，并在 CI 中提供脚本用于解析目标、下载、校验并准备运行时。cryptographywheels，并统一标准化制品命名。CI：
workflow_dispatch必须显式指定 tag，并默认使用github.ref_name作为构建 ref。文档：
python-build-standalone用法、后端布局以及常见构建错误。Original summary in English
Summary by Sourcery
Migrate the desktop backend from a PyInstaller single binary to a bundled CPython runtime plus source with a manifest-driven launcher, and harden Windows runtime behavior and release workflows.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation: