Merged
Conversation
Contributor
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并给出了一些整体层面的反馈:
- 现在在
run_action和mxu_launch_action_fn中构建带/不带cmd.exe的Command的逻辑是重复的;建议抽取一个小的帮助函数来构建该命令,以保持行为一致并让维护更容易。 useCmd开关在所有平台的 UI 中都会显示并发送到后端,但只在 Windows 上有效;你可能希望在非 Windows 平台上有条件地隐藏/禁用这个开关(或者在客户端直接短路该参数),以避免让用户感到困惑。
给 AI Agents 的提示
Please address the comments from this code review:
## Overall Comments
- The logic for building the `Command` with/without `cmd.exe` is now duplicated in both `run_action` and `mxu_launch_action_fn`; consider extracting a small helper to construct the command to keep behavior consistent and easier to maintain.
- The `useCmd` flag is surfaced in the UI and sent to the backend on all platforms but only has effect on Windows; you might want to conditionally hide/disable this switch (or short‑circuit the param client‑side) on non‑Windows platforms to avoid user confusion.
## Individual Comments
### Comment 1
<location path="src/i18n/locales/en-US.ts" line_range="148" />
<code_context>
skipYes: 'Skip launch if already running',
skipNo: 'Always launch new instance',
+ cmdLabel: 'Launch via cmd',
+ cmdDescription: 'When enabled, launches the program via cmd /c to detach from the current process tree. Some games may detect the process tree (Windows only)',
+ cmdYes: 'Launch via cmd /c',
+ cmdNo: 'Launch as direct subprocess',
</code_context>
<issue_to_address>
**suggestion:** `cmd /c` 实际上并不会从当前进程树中分离;目前的描述可能夸大了这个选项的作用。
`cmd.exe /c ...` 仍然是作为当前进程的子进程运行,只是多插入了一层 `cmd.exe` 作为额外的父进程。它并不会让被启动的程序完全从原始进程树中分离,对遍历进程树的工具来说,其进程来源依然是可见的。
为了避免夸大效果,可以考虑类似这样的措辞:
> "launches the program via `cmd /c`, making `cmd.exe` the direct parent process (Windows only)"
并在本次变更中新增的其他语言版本中也体现这一差异。
建议的实现:
```typescript
cmdDescription: 'When enabled, launches the program via cmd /c, making cmd.exe the direct parent process (Windows only)',
```
```typescript
useCmdHint: 'When enabled, launches the program via cmd /c, making cmd.exe the direct parent process (Windows only)',
```
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会基于这些反馈改进后续的 Review。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The logic for building the
Commandwith/withoutcmd.exeis now duplicated in bothrun_actionandmxu_launch_action_fn; consider extracting a small helper to construct the command to keep behavior consistent and easier to maintain. - The
useCmdflag is surfaced in the UI and sent to the backend on all platforms but only has effect on Windows; you might want to conditionally hide/disable this switch (or short‑circuit the param client‑side) on non‑Windows platforms to avoid user confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The logic for building the `Command` with/without `cmd.exe` is now duplicated in both `run_action` and `mxu_launch_action_fn`; consider extracting a small helper to construct the command to keep behavior consistent and easier to maintain.
- The `useCmd` flag is surfaced in the UI and sent to the backend on all platforms but only has effect on Windows; you might want to conditionally hide/disable this switch (or short‑circuit the param client‑side) on non‑Windows platforms to avoid user confusion.
## Individual Comments
### Comment 1
<location path="src/i18n/locales/en-US.ts" line_range="148" />
<code_context>
skipYes: 'Skip launch if already running',
skipNo: 'Always launch new instance',
+ cmdLabel: 'Launch via cmd',
+ cmdDescription: 'When enabled, launches the program via cmd /c to detach from the current process tree. Some games may detect the process tree (Windows only)',
+ cmdYes: 'Launch via cmd /c',
+ cmdNo: 'Launch as direct subprocess',
</code_context>
<issue_to_address>
**suggestion:** `cmd /c` does not actually detach from the current process tree; the description may overstate what this option does.
`cmd.exe /c ...` still runs as a child of the current process, with `cmd.exe` inserted as an extra parent. It does not fully detach the launched program from the original process tree, and ancestry is still visible to tools that walk the process tree.
To avoid overstating the effect, consider wording like:
> "launches the program via `cmd /c`, making `cmd.exe` the direct parent process (Windows only)"
and mirror this nuance in the other locales added in this change.
Suggested implementation:
```typescript
cmdDescription: 'When enabled, launches the program via cmd /c, making cmd.exe the direct parent process (Windows only)',
```
```typescript
useCmdHint: 'When enabled, launches the program via cmd /c, making cmd.exe the direct parent process (Windows only)',
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Owner
|
@sourcery-ai review |
Owner
|
CI 没跑过,看一哈 |
Contributor
There was a problem hiding this comment.
Hey - 我在这里给出了一些整体性的反馈:
- 新的
run_action参数在 Rust 中命名为use_cmd,但从前端传入时使用的是useCmd;请再次检查 Tauri 的 invoke 参数映射,并考虑统一命名(或者添加明确的 serde/InvokeArgs 重命名)以避免在不同平台之间出现细微的不匹配问题。 - 构建带可选
cmd /c的Command并应用CREATE_BREAKAWAY_FROM_JOB的逻辑在run_action和mxu_launch_action_fn中都有重复;建议将其抽取为一个共享的辅助函数,以降低未来逻辑不一致的风险。 - UI 文案说明 cmd 开关仅适用于 Windows,但该开关实际在所有平台上都无条件显示;你可能需要在非 Windows 平台上对该控件进行条件显示或禁用,使实际行为与描述保持一致。
给 AI 代理的提示
Please address the comments from this code review:
## Overall Comments
- The new `run_action` argument is named `use_cmd` in Rust but passed as `useCmd` from the frontend; double-check the Tauri invoke argument mapping and consider aligning the naming (or adding explicit serde/InvokeArgs renames) to avoid subtle mismatches across platforms.
- The logic for building the `Command` with optional `cmd /c` and applying `CREATE_BREAKAWAY_FROM_JOB` is duplicated in both `run_action` and `mxu_launch_action_fn`; consider factoring this into a shared helper to reduce the chance of future divergence.
- The UI text says the cmd toggle is Windows-only, but the switch appears unconditionally; you may want to gate or disable this control on non-Windows platforms so the behavior matches the description.帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've left some high level feedback:
- The new
run_actionargument is nameduse_cmdin Rust but passed asuseCmdfrom the frontend; double-check the Tauri invoke argument mapping and consider aligning the naming (or adding explicit serde/InvokeArgs renames) to avoid subtle mismatches across platforms. - The logic for building the
Commandwith optionalcmd /cand applyingCREATE_BREAKAWAY_FROM_JOBis duplicated in bothrun_actionandmxu_launch_action_fn; consider factoring this into a shared helper to reduce the chance of future divergence. - The UI text says the cmd toggle is Windows-only, but the switch appears unconditionally; you may want to gate or disable this control on non-Windows platforms so the behavior matches the description.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `run_action` argument is named `use_cmd` in Rust but passed as `useCmd` from the frontend; double-check the Tauri invoke argument mapping and consider aligning the naming (or adding explicit serde/InvokeArgs renames) to avoid subtle mismatches across platforms.
- The logic for building the `Command` with optional `cmd /c` and applying `CREATE_BREAKAWAY_FROM_JOB` is duplicated in both `run_action` and `mxu_launch_action_fn`; consider factoring this into a shared helper to reduce the chance of future divergence.
- The UI text says the cmd toggle is Windows-only, but the switch appears unconditionally; you may want to gate or disable this control on non-Windows platforms so the behavior matches the description.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.
Pull request overview
本 PR 在 Windows 上引入“通过 cmd /c 启动程序”的可配置开关,并将该配置从前端动作配置与 MXU 特殊任务一路传递到 Tauri 后端的进程创建逻辑,以支持(尝试)从当前进程树/Job 中分离启动的子进程。
Changes:
- 为前置动作(pre-action)新增
useCmd配置项,并在 UI、service 调用与 Taurirun_action命令中透传。 - 为 MXU_LAUNCH 特殊任务新增 “use_cmd” 开关选项,并在后端 MXU 自定义 action 执行中支持该参数。
- 增加多语言文案(en/ja/ko/zh-CN/zh-TW)以覆盖新开关的 UI 文本。
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/types/specialTasks.ts | 为 MXU_LAUNCH 增加 use_cmd 开关选项定义并挂载到 optionDefs |
| src/types/interface.ts | 为 ActionConfig 增加 useCmd 字段 |
| src/services/maaService.ts | runAction 增加 useCmd 参数并通过 invoke('run_action') 传递 |
| src/components/Toolbar.tsx | 调用 runAction 时把 preAction.useCmd 传入 |
| src/components/AddTaskPanel.tsx | 默认前置动作配置补齐 useCmd: false |
| src/components/ActionItem.tsx | UI 增加“通过 cmd 启动”开关并写回 useCmd |
| src/i18n/locales/en-US.ts | 增加新开关相关文案 |
| src/i18n/locales/ja-JP.ts | 增加新开关相关文案 |
| src/i18n/locales/ko-KR.ts | 增加新开关相关文案 |
| src/i18n/locales/zh-CN.ts | 增加新开关相关文案 |
| src/i18n/locales/zh-TW.ts | 增加新开关相关文案 |
| src-tauri/src/mxu_actions.rs | MXU_LAUNCH action 支持读取 use_cmd 并在 Windows 下通过 cmd 启动+设置 breakaway flag |
| src-tauri/src/commands/system.rs | run_action 命令新增 use_cmd 参数,并在 Windows 下通过 cmd 启动+设置 breakaway flag |
Comments suppressed due to low confidence (2)
src-tauri/src/commands/system.rs:468
- Windows + use_cmd 走
cmd.exe /c时,args_vec会被直接拼到 cmd 的命令行里;但 std::process::Command 的参数 quoting 规则是为 CreateProcess 服务的,并不会按 cmd 的解析规则转义元字符(例如& | < > ^ ( ) !)。因此如果参数或 program 路径中包含这些字符(哪怕没有空格),cmd 可能把它们当作操作符解析,导致启动失败或执行到意外的额外命令。建议:在 use_cmd 分支对 program/args 做 cmd 兼容的 quoting/escaping(例如把整条命令构造成一个字符串传给/c,并对上述元字符进行转义),或明确限制/校验不允许这些字符。
// 设置工作目录
if let Some(ref dir) = cwd {
cmd.current_dir(dir);
} else {
// 默认使用程序所在目录作为工作目录
if let Some(parent) = std::path::Path::new(&program).parent() {
if parent.exists() {
src-tauri/src/mxu_actions.rs:268
- use_cmd 模式下通过
cmd.exe /c启动时,当前把 program/args 作为独立参数交给 cmd,但没有按 cmd 语法对元字符做转义(如& | < > ^ ( ) !)。std::process::Command 的 quoting 主要是给 CreateProcess 用的,遇到这些字符时 cmd 仍可能把它们当操作符解析,导致启动异常或执行到意外命令。建议在 use_cmd 分支实现 cmd 兼容的 quoting/escaping(例如构造完整命令行字符串传给/c并转义元字符),或在启用 use_cmd 时对输入做限制/提示。
let mut cmd = crate::commands::utils::build_launch_command(&program, &args_vec, use_cmd);
// 默认使用程序所在目录作为工作目录
if let Some(parent) = std::path::Path::new(&program).parent() {
if parent.exists() {
cmd.current_dir(parent);
}
}
Contributor
Author
|
这个平台特定有必要加吗 |
Owner
|
加上吧,仅windows有这个选项 |
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
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.
Summary by Sourcery
在 Windows 上新增可配置选项,用于通过
cmd /c启动操作,并将该设置从 UI 传递到 MXU 启动任务配置,再到 Tauri 后端的进程执行。新功能:
cmd /c启动程序。run_action命令和 MXU 启动操作中支持可选的use_cmd参数,用于决定是否通过cmd.exe /c启动进程并将其从当前进程树中分离。增强:
Original summary in English
Summary by Sourcery
Add a configurable option to launch actions via
cmd /con Windows and propagate this setting from the UI through the MXU launch task configuration to the Tauri backend process execution.New Features:
cmd /con Windows.use_cmdparameter in the Taurirun_actioncommand and MXU launch action to determine whether to spawn processes throughcmd.exe /cand detach them from the current process tree.Enhancements:
新功能:
cmd /c启动程序。改进:
Original summary in English
Summary by Sourcery
在 Windows 上新增可配置选项,用于通过
cmd /c启动操作,并将该设置从 UI 传递到 MXU 启动任务配置,再到 Tauri 后端的进程执行。新功能:
cmd /c启动程序。run_action命令和 MXU 启动操作中支持可选的use_cmd参数,用于决定是否通过cmd.exe /c启动进程并将其从当前进程树中分离。增强:
Original summary in English
Summary by Sourcery
Add a configurable option to launch actions via
cmd /con Windows and propagate this setting from the UI through the MXU launch task configuration to the Tauri backend process execution.New Features:
cmd /con Windows.use_cmdparameter in the Taurirun_actioncommand and MXU launch action to determine whether to spawn processes throughcmd.exe /cand detach them from the current process tree.Enhancements: