fix: desktop update banner for desktop releases#117
fix: desktop update banner for desktop releases#117zouyonghe merged 2 commits intoAstrBotDevs:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a patching mechanism to suppress the backend update banner in VerticalHeader.vue when the application is running in desktop release mode. The feedback highlights that the current string-replacement logic is fragile due to its reliance on exact indentation and line endings, suggesting the use of regular expressions and better error reporting to ensure the patch applies correctly across different environments.
| const target = " hasNewVersion.value = res.data.data.has_new_version;\n\n if (res.data.data.has_new_version) {"; | ||
| const replacement = | ||
| " const backendHasNewVersion = !isDesktopReleaseMode.value && res.data.data.has_new_version;\n" + | ||
| " hasNewVersion.value = backendHasNewVersion;\n\n" + | ||
| " if (backendHasNewVersion) {"; | ||
|
|
||
| if (!source.includes(target)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
The patch logic is fragile because it relies on an exact string match with hardcoded indentation and Unix-style line endings (\n\n). If the file uses CRLF (common on Windows) or different indentation, the patch will fail. Additionally, the function returns silently when the target is not found, which can lead to the bug persisting without any build-time warning.
Consider using a regular expression to handle whitespace and line ending variations, and log a warning if the patch cannot be applied.
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
patchDesktopReleaseUpdateIndicatorhelper relies on an exact multi-line string match fortarget, which is brittle against minor upstream formatting changes; consider using a more flexible pattern (e.g., a regex or AST-based approach) to make the patch more resilient. - In the patch helper, failing to find the
targetstring causes a silent return; adding a warning or log when the expected pattern is not found would make it easier to detect when upstream changes have broken the patch.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `patchDesktopReleaseUpdateIndicator` helper relies on an exact multi-line string match for `target`, which is brittle against minor upstream formatting changes; consider using a more flexible pattern (e.g., a regex or AST-based approach) to make the patch more resilient.
- In the patch helper, failing to find the `target` string causes a silent return; adding a warning or log when the expected pattern is not found would make it easier to detect when upstream changes have broken the patch.
## Individual Comments
### Comment 1
<location path="scripts/prepare-resources/desktop-bridge-checks.mjs" line_range="76-91" />
<code_context>
+ return;
+ }
+
+ const target = " hasNewVersion.value = res.data.data.has_new_version;\n\n if (res.data.data.has_new_version) {";
+ const replacement =
+ " const backendHasNewVersion = !isDesktopReleaseMode.value && res.data.data.has_new_version;\n" +
</code_context>
<issue_to_address>
**suggestion:** The exact string target makes this patch brittle to minor formatting or code changes in the Vue file.
This depends on an exact substring match (indentation, newlines, and precise code sequence), so even small refactors in `VerticalHeader.vue` (spacing changes, added variables, comments) will break the patch with no clear signal. Please use a more robust match (e.g., a regex for the assignment and the `if` near each other, or a shorter, essential substring) so it remains stable across minor formatting changes.
```suggestion
const source = await readFile(file, 'utf8');
if (source.includes('const backendHasNewVersion = !isDesktopReleaseMode.value && res.data.data.has_new_version;')) {
return;
}
// Match the assignment to hasNewVersion and the immediately following if-check,
// allowing for minor whitespace/formatting changes.
const targetPattern =
/hasNewVersion\.value\s*=\s*res\.data\.data\.has_new_version;\s*\n\s*if\s*\(\s*res\.data\.data\.has_new_version\s*\)\s*{/;
const replacement =
" const backendHasNewVersion = !isDesktopReleaseMode.value && res.data.data.has_new_version;\n" +
" hasNewVersion.value = backendHasNewVersion;\n\n" +
" if (backendHasNewVersion) {";
if (!targetPattern.test(source)) {
return;
}
const patched = source.replace(targetPattern, replacement);
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sourcery-ai review |
This fixes a desktop-only update UX mismatch where the top header could show
AstrBot 有新版本!while the desktop updater dialog reported that the current app was already up to date.In desktop release mode, the upstream dashboard still used
/api/update/checkto drive the header badge. That API reports backend update availability, not desktop shell update availability. Clicking the badge in desktop mode opens the desktop app updater dialog, so users could see a backend-originated update hint followed by a desktop updater result that correctly said there was no newer app build.The root cause was that
VerticalHeader.vueassignedhasNewVersiondirectly fromres.data.data.has_new_versioneven whenisDesktopReleaseModewas true. The desktop shell already routes the actual update action throughwindow.astrbotAppUpdater, so the header badge needed to stop reflecting backend release checks in that mode.This change patches the upstream dashboard source during
prepare:webuiso the backend update badge is only enabled outside desktop release mode. It also adds a regression test for the patch helper and wires the helper into the WebUI prepare pipeline.Validation:
node --test "scripts/prepare-resources/desktop-bridge-checks.test.mjs"node --test "scripts/prepare-resources/mode-dispatch.test.mjs" "scripts/prepare-resources/context.test.mjs" "scripts/prepare-resources/bridge-bootstrap-updater-contract.test.mjs"pnpm test:prepare-resourcesSummary by Sourcery
Ensure the desktop app header update banner no longer reflects backend update availability when running in desktop release mode.
Bug Fixes:
Enhancements:
Tests: