feat: refactor release workflow and add special update handling for electron app#4969
Merged
feat: refactor release workflow and add special update handling for electron app#4969
Conversation
dosubot
bot
added
the
size:L
|
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.
Hey - 我发现了 1 个问题，并给出了一些整体反馈：
- 在
VerticalHeader.vue的onMounted钩子中，isElectronApp.value在 try/catch 之后总是被设置为true，这会覆盖运行时检测逻辑，并很可能在浏览器中强制启用仅适用于 Electron 的行为；这看起来是无意的，应当移除或加以保护。 - 在
release.yml中的多个 job 里都重复了Resolve tag的 shell 逻辑；建议将其抽取为一个可复用的 composite action，或者单独的步骤（例如run: ./scripts/resolve-tag.sh），以避免不同实现之间出现偏差。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `VerticalHeader.vue`'s `onMounted` hook, `isElectronApp.value` is always set to `true` after the try/catch, which overrides the runtime detection logic and likely forces Electron-only behavior in the browser; this looks unintended and should be removed or guarded.
- The `Resolve tag` shell logic is duplicated across multiple jobs in `release.yml`; consider extracting it into a reusable composite action or a separate step with `run: ./scripts/resolve-tag.sh` to avoid drift between implementations.
## Individual Comments
### Comment 1
<location> `dashboard/src/layouts/full/vertical-header/VerticalHeader.vue:384` </location>
<code_context>
+ } catch {
+ isElectronApp.value = false;
+ }
+ isElectronApp.value = true
+});
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Unconditional `isElectronApp = true` overrides the runtime check and likely breaks browser-only behavior.
The runtime check against `window.astrbotDesktop` is immediately overridden by `isElectronApp.value = true`, so all environments are treated as Electron, which can break web deployments.
If this was only for debugging, please remove the forced assignment. Otherwise, keep just the detection logic, e.g.:
```ts
onMounted(async () => {
try {
isElectronApp.value = !!window.astrbotDesktop?.isElectron ||
!!(await window.astrbotDesktop?.isElectronRuntime?.());
} catch {
isElectronApp.value = false;
}
});
```
</issue_to_address>帮我变得更有用！请对每条评论点 👍 或 👎，我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- In
VerticalHeader.vue'sonMountedhook,isElectronApp.valueis always set totrueafter the try/catch, which overrides the runtime detection logic and likely forces Electron-only behavior in the browser; this looks unintended and should be removed or guarded. - The
Resolve tagshell logic is duplicated across multiple jobs inrelease.yml; consider extracting it into a reusable composite action or a separate step withrun: ./scripts/resolve-tag.shto avoid drift between implementations.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `VerticalHeader.vue`'s `onMounted` hook, `isElectronApp.value` is always set to `true` after the try/catch, which overrides the runtime detection logic and likely forces Electron-only behavior in the browser; this looks unintended and should be removed or guarded.
- The `Resolve tag` shell logic is duplicated across multiple jobs in `release.yml`; consider extracting it into a reusable composite action or a separate step with `run: ./scripts/resolve-tag.sh` to avoid drift between implementations.
## Individual Comments
### Comment 1
<location> `dashboard/src/layouts/full/vertical-header/VerticalHeader.vue:384` </location>
<code_context>
+ } catch {
+ isElectronApp.value = false;
+ }
+ isElectronApp.value = true
+});
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Unconditional `isElectronApp = true` overrides the runtime check and likely breaks browser-only behavior.
The runtime check against `window.astrbotDesktop` is immediately overridden by `isElectronApp.value = true`, so all environments are treated as Electron, which can break web deployments.
If this was only for debugging, please remove the forced assignment. Otherwise, keep just the detection logic, e.g.:
```ts
onMounted(async () => {
try {
isElectronApp.value = !!window.astrbotDesktop?.isElectron ||
!!(await window.astrbotDesktop?.isElectronRuntime?.());
} catch {
isElectronApp.value = false;
}
});
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } catch { | ||
| isElectronApp.value = false; | ||
| } | ||
| isElectronApp.value = true |
Contributor
There was a problem hiding this comment.
issue (bug_risk): 无条件设置 isElectronApp = true 会覆盖运行时检查，并很可能破坏仅浏览器环境下的行为。
针对 window.astrbotDesktop 的运行时检查会立即被 isElectronApp.value = true 覆盖，因此所有环境都会被视为 Electron，从而可能破坏 Web 部署。
如果这只是用于调试，请移除这个强制赋值。否则，请仅保留检测逻辑，例如：
onMounted(async () => {
try {
isElectronApp.value = !!window.astrbotDesktop?.isElectron ||
!!(await window.astrbotDesktop?.isElectronRuntime?.());
} catch {
isElectronApp.value = false;
}
});Original comment in English
issue (bug_risk): Unconditional isElectronApp = true overrides the runtime check and likely breaks browser-only behavior.
The runtime check against window.astrbotDesktop is immediately overridden by isElectronApp.value = true, so all environments are treated as Electron, which can break web deployments.
If this was only for debugging, please remove the forced assignment. Otherwise, keep just the detection logic, e.g.:
onMounted(async () => {
try {
isElectronApp.value = !!window.astrbotDesktop?.isElectron ||
!!(await window.astrbotDesktop?.isElectronRuntime?.());
} catch {
isElectronApp.value = false;
}
});
dosubot
bot
added
the
feature:updater
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
1 participant
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.
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
重构发布流水线，在统一的工作流中构建并分发 dashboard、desktop 和 PyPI 包，同时为 Electron dashboard 客户端添加特殊的更新处理逻辑。
新功能：
改进：
CI：
Original summary in English
Summary by Sourcery
Refactor the release pipeline to build and distribute dashboard, desktop, and PyPI packages in a unified workflow while adding special update handling for the Electron dashboard client.
New Features:
Enhancements:
CI: