fix(desktop): localize update fetch-failure message on Windows#1088
fix(desktop): localize update fetch-failure message on Windows#1088teddyli18000 wants to merge 10 commits intonexu-io:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses a desktop i18n UX issue on Windows by normalizing fetch/network-related auto-update errors so localized UIs (notably zh-CN) don’t surface the raw English "fetch failed" string in update failure notifications.
Changes:
- Exported
normalizeUpdateErrorMessagefrom the desktop auto-update hook to enable direct regression testing. - Added normalization for fetch-related failure messages (
fetch failed,failed to fetch,NetworkError ... fetch) mapping them to localized, user-friendly strings. - Added Vitest regression tests covering both normal updates and
local-test-feedupdate experiences.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/desktop/src/hooks/use-auto-update.ts | Exposes and extends update error normalization to localize fetch/network failures. |
| tests/desktop/update-error-message-normalization.test.ts | Adds regression tests ensuring fetch-failure messages are normalized in both update experiences. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| describe("update error message normalization", () => { | ||
| it("normalizes fetch failures in normal update mode", () => { | ||
| const result = normalizeUpdateErrorMessage("fetch failed", "normal"); | ||
| expect(result).not.toMatch(/fetch failed/i); | ||
| }); | ||
|
|
||
| it("normalizes fetch failures in local test feed mode", () => { | ||
| const result = normalizeUpdateErrorMessage( | ||
| "TypeError: fetch failed", | ||
| "local-test-feed", | ||
| ); | ||
| expect(result).not.toMatch(/fetch failed/i); | ||
| }); |
There was a problem hiding this comment.
These assertions only check that the output no longer contains the substring "fetch failed", which would still pass if the function returns a different raw English fetch-related message (e.g., "NetworkError when attempting to fetch…") or an empty/placeholder string. Consider asserting the exact expected normalized message (or a stable key phrase), and explicitly controlling the locale (e.g., temporarily set/define navigator.language to en-US and zh-CN) so the test validates the localized output deterministically.
| describe("update error message normalization", () => { | |
| it("normalizes fetch failures in normal update mode", () => { | |
| const result = normalizeUpdateErrorMessage("fetch failed", "normal"); | |
| expect(result).not.toMatch(/fetch failed/i); | |
| }); | |
| it("normalizes fetch failures in local test feed mode", () => { | |
| const result = normalizeUpdateErrorMessage( | |
| "TypeError: fetch failed", | |
| "local-test-feed", | |
| ); | |
| expect(result).not.toMatch(/fetch failed/i); | |
| }); | |
| const withNavigatorLanguage = <T>(language: string, run: () => T): T => { | |
| const originalDescriptor = Object.getOwnPropertyDescriptor( | |
| window.navigator, | |
| "language", | |
| ); | |
| Object.defineProperty(window.navigator, "language", { | |
| configurable: true, | |
| value: language, | |
| }); | |
| try { | |
| return run(); | |
| } finally { | |
| if (originalDescriptor) { | |
| Object.defineProperty(window.navigator, "language", originalDescriptor); | |
| } else { | |
| delete (window.navigator as Navigator & { language?: string }).language; | |
| } | |
| } | |
| }; | |
| describe("update error message normalization", () => { | |
| it("normalizes fetch failures in normal update mode for en-US", () => { | |
| const result = withNavigatorLanguage("en-US", () => | |
| normalizeUpdateErrorMessage("fetch failed", "normal"), | |
| ); | |
| expect(result).toContain("network"); | |
| expect(result).not.toMatch(/fetch failed/i); | |
| }); | |
| it("normalizes fetch failures in normal update mode for zh-CN", () => { | |
| const result = withNavigatorLanguage("zh-CN", () => | |
| normalizeUpdateErrorMessage("fetch failed", "normal"), | |
| ); | |
| expect(result).toContain("网络"); | |
| expect(result).not.toMatch(/fetch failed/i); | |
| }); | |
| it("normalizes fetch failures in local test feed mode for en-US", () => { | |
| const result = withNavigatorLanguage("en-US", () => | |
| normalizeUpdateErrorMessage("TypeError: fetch failed", "local-test-feed"), | |
| ); | |
| expect(result).toContain("local"); | |
| expect(result).toContain("server"); | |
| expect(result).not.toMatch(/fetch failed/i); | |
| }); | |
| it("normalizes fetch failures in local test feed mode for zh-CN", () => { | |
| const result = withNavigatorLanguage("zh-CN", () => | |
| normalizeUpdateErrorMessage("TypeError: fetch failed", "local-test-feed"), | |
| ); | |
| expect(result).toContain("本地"); | |
| expect(result).toContain("服务器"); | |
| expect(result).not.toMatch(/fetch failed/i); | |
| }); |
| it("normalizes fetch failures in local test feed mode", () => { | ||
| const result = normalizeUpdateErrorMessage( | ||
| "TypeError: fetch failed", | ||
| "local-test-feed", | ||
| ); | ||
| expect(result).not.toMatch(/fetch failed/i); | ||
| }); |
There was a problem hiding this comment.
This test also only asserts that "fetch failed" is absent; it doesn't verify the actual user-facing string for the local-test-feed experience or that the output is localized under a zh locale. Strengthen it by forcing the locale and asserting the expected normalized message for both normal and local-test-feed inputs.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
@teddyli18000 👋你好呀~ 我们收到您今天提交了 2 次 Pr,我们非常感激你的贡献 🎉 |
…u-io#1094) * fix(controller): restart OpenClaw on every provider/cloud change Provider-level changes to openclaw.json were not hot-reload-safe: OpenClaw built its provider/model registry once at boot, so cloud login/logout and BYOK mutations left stale entries resident. In packaged desktop (launchd-managed OpenClaw) the existing openclawProcess.stop()/start() path was a no-op, so the registry never refreshed. Visible symptoms: - After cloud logout, bot kept replying using Link-backed models. - After cloud re-login, bot errored with "FailoverError: Unknown model: link/gemini-3-flash-preview" even though openclaw.json on disk was correct. Fix: - New OpenClawProcessManager.restart(reason) handling both dev-managed (stop + start) and launchd-managed (launchctl kickstart -k gui/<uid>/<label>) modes. - container.ts onCloudStateChanged now always calls ensureValidDefaultModel + syncAll + openclawProcess.restart on every login/logout transition. - model-provider-service restartRuntime() uses the new restart() and is invoked from setModelProviderConfigDocument, refreshNexuOfficialModels, and deleteProvider so BYOK and OAuth mutations also bounce OpenClaw. Supporting fixes surfaced during investigation: - openclaw-config-compiler + shared schema: apiKey is optional; stop emitting apiKey:"" for OAuth providers (which caused OpenClaw to reject the whole models.providers block and took link down with it). - openclaw-auth-profiles-writer: merge primary + fallback credentials (primary wins on collision) instead of either-or, so Link auth stays written when other providers coexist. - nexu-config-store: disconnectDesktopCloud clears the managed default model only when it was actually cloud-backed. - openclaw-sync-service: strip agents.defaults.model and per-agent model when no providers exist; write locale-aware writeNoModelState(). - openclaw-runtime-model-writer: add writeNoModelState() + clear(); export resolveNoModelConfiguredMessage(locale) for bilingual EN / zh-CN guidance text. - nexu-runtime-model plugin: no-op hooks when selectedModelRef is empty (previously tried to parse an empty string). Validated in packaged desktop (launchctl kickstart fires on every transition): - Cloud login/logout: openclaw_restart_launchd_kickstarted reason=cloud_state_changed - BYOK add/delete: reason=model_provider_config_changed - No "Unknown model: link/*" errors across the smoke test. * fix(controller): address codex review on no-provider sync + cloud logout Two independent issues surfaced by the PR nexu-io#1094 review: 1. openclaw-sync-service.doSync applied the no-provider normalization (stripping agents.defaults.model and per-agent model) AFTER shouldPushConfig() ran, while noteConfigWritten() recorded the normalized shape. In the zero-provider state the pre-normalized hash diverged from the stored (normalized) hash on every sync, so configPushed stayed true and touchAnySkillMarker() kept firing — continuous SKILL.md churn + snapshotVersion bumps while idle. Normalize immutably BEFORE the hash check so both shouldPushConfig and noteConfigWritten see the same final shape. 2. disconnectDesktopCloud only cleared runtime.defaultModelId when the global default was a managed cloud id. Per-bot modelId overrides set to link/* were left untouched, so a user with a BYOK default plus a bot explicitly pinned to Link kept a stale link/* ref after logout, which compiles into agents.list[*].model.primary and fails later with "Unknown model: link/...". Walk every bot on logout and clear any bot.modelId that resolves as managed-cloud; BYOK/OAuth bot selections stay intact. * docs: add design notes for registry cache invalidation + model id usage - 2026-04-14-openclaw-registry-cache-invalidation.md: full write-up of the OpenClaw provider/model registry cache bug, fix shape, smoke-test evidence, and the debugging retrospective. Referenced by the new hard rule in AGENTS.md. - 2026-04-14-model-id-field-usage.md: survey of how the model-level id field flows through the codebase (schemas, compiler, OAuth remapping, UI), noting collisions, sanitization gaps, and open design questions.
…#1096) * fix: surface skill install failures with retry and cancel UI Skill installs that hit the ClawHub rate limit (or any other terminal error) silently failed: the spinner vanished and users had no idea why the skill was missing, so they kept clicking install. The queue already classified errors and stored the message — only the frontend was hiding the result. - Failed queue items now appear in both Yours and ClawHub tabs with a red border, an inline alert, a Retry button (rate_limit / unknown), and a Cancel button to dismiss the failed card. - skill_not_found shows a "not available" message with Cancel only (retry would hit the same error). - Unknown errors render the raw underlying message under the friendly text so users can self-diagnose network / permission failures. - Backend: cancel() also evicts terminally-failed entries; new POST /api/v1/skillhub/cancel route exposes it. enqueue() clears any prior failed row for the same slug so retry produces one clean queue item. cleanupDelayMs raised to 60s for retry breathing room. - New skillhub-install-queue tests cover retry-cleanup and cancel. Closes nexu-io#663 * fix: address review feedback on skill install failure UI - handleCancel now catches mutation rejections and surfaces a toast instead of leaving the click flow with an unhandled promise. Cancel is the failure-recovery path itself, so silent failures here are particularly bad in flaky/offline conditions. - Catalog polling continues while failed cards are visible. Previously the poll stopped on transition to "failed", which left stale cards on screen long after the 60s backend cleanup window evicted them.
…yli18000/nexu into fix/994-windows-update-i18n
|
Apologies for the noise in this PR. The diff has ballooned to 4,000+ lines due to my AI assistant accidentally rebasing my branch on top of 6 unrelated prior commits during a revision pass. My actual changes remain the same as originally described:
As this is my first contribution to this project, I appreciate your patience. I'll do my best to ensure cleaner PRs going forward. Please let me know if there's anything else you'd like me to address in the meantime. Apologize again. I will do my best to contribute to nexu in the future. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 72 out of 73 changed files in this pull request and generated 4 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
apps/desktop/src/hooks/use-auto-update.ts:38
- The PR description/title focus on desktop update error localization, but this diff also includes substantial unrelated functional changes (SkillHub cancel/retry UX + new controller routes, OpenClaw restart/nudge behavior, schema changes, bundled-skill metadata rewrites, etc.). Please either narrow the PR to the update-message fix or update the PR description/scope so reviewers can assess the additional risk appropriately.
export function normalizeUpdateErrorMessage(
message: string,
experience: DesktopUpdateExperience,
): string {
const trimmedMessage = message.trim();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // JSON-in-YAML: "requires": { "bins": [...] }, or "requires": { "anyBins": [...] }, | ||
| // Remove the entire "requires": { ... }, block (with optional trailing comma) | ||
| let patched = content.replace(/\s*"requires"\s*:\s*\{[^}]*\}\s*,?/g, ""); | ||
|
|
||
| // Standard YAML: requires:\n bins:\n - x\n - y | ||
| // Remove the requires block and all its indented children | ||
| patched = patched.replace(/^[ \t]*requires:\s*\n(?:[ \t]+.*\n)*/gm, ""); | ||
|
|
||
| if (patched !== content) { |
There was a problem hiding this comment.
stripRequiresBins() applies global regex replacements across the entire SKILL.md file, not just the YAML frontmatter. That can inadvertently delete legitimate markdown content or code blocks containing requires: / "requires": {...} outside the frontmatter. Consider restricting the edit to the extracted frontmatter region only (parse/replace inside the --- ... --- block, then reassemble).
| // JSON-in-YAML: "requires": { "bins": [...] }, or "requires": { "anyBins": [...] }, | |
| // Remove the entire "requires": { ... }, block (with optional trailing comma) | |
| let patched = content.replace(/\s*"requires"\s*:\s*\{[^}]*\}\s*,?/g, ""); | |
| // Standard YAML: requires:\n bins:\n - x\n - y | |
| // Remove the requires block and all its indented children | |
| patched = patched.replace(/^[ \t]*requires:\s*\n(?:[ \t]+.*\n)*/gm, ""); | |
| if (patched !== content) { | |
| const frontmatterMatch = content.match(/^---\n([\s\S]*?)\n---/); | |
| if (!frontmatterMatch || frontmatterMatch.index == null) return; | |
| const frontmatter = frontmatterMatch[1] ?? ""; | |
| // JSON-in-YAML: "requires": { "bins": [...] }, or "requires": { "anyBins": [...] }, | |
| // Remove the entire "requires": { ... }, block (with optional trailing comma) | |
| let patchedFrontmatter = frontmatter.replace( | |
| /\s*"requires"\s*:\s*\{[^}]*\}\s*,?/g, | |
| "", | |
| ); | |
| // Standard YAML: requires:\n bins:\n - x\n - y | |
| // Remove the requires block and all its indented children | |
| patchedFrontmatter = patchedFrontmatter.replace( | |
| /^[ \t]*requires:\s*\n(?:[ \t]+.*\n)*/gm, | |
| "", | |
| ); | |
| if (patchedFrontmatter !== frontmatter) { | |
| const frontmatterStart = frontmatterMatch.index; | |
| const frontmatterEnd = frontmatterStart + frontmatterMatch[0].length; | |
| const patched = | |
| `${content.slice(0, frontmatterStart)}---\n${patchedFrontmatter}\n---` + | |
| content.slice(frontmatterEnd); |
| mutationFn: async (slug: string) => { | ||
| const { data, error } = await postApiV1SkillhubCancel({ | ||
| body: { slug }, | ||
| }); | ||
| if (error) throw new Error("Cancel request failed"); | ||
| const result = data as { ok: boolean; cancelled: boolean }; | ||
| await Promise.all([ |
There was a problem hiding this comment.
useCancelInstall() throws a generic "Cancel request failed" when the SDK returns an error, which means the UI toast in SkillsPage will never show any actionable detail from the backend (HTTP status / message). Prefer propagating the SDK error message (or include it in the thrown Error) so users get a meaningful failure reason.
| const slashIndex = state.selectedModelRef.indexOf("/"); | ||
| const providerOverride = state.selectedModelRef.slice(0, slashIndex); | ||
| const modelOverride = state.selectedModelRef.slice(slashIndex + 1); | ||
| const modelOverride = | ||
| slashIndex > 0 | ||
| ? state.selectedModelRef.slice(slashIndex + 1) | ||
| : state.selectedModelRef; | ||
| return { | ||
| providerOverride, | ||
| ...(slashIndex > 0 ? { providerOverride } : {}), |
There was a problem hiding this comment.
In the runtime-model plugin, providerOverride is computed even when selectedModelRef has no "/" (so slashIndex is -1). Although it's currently not spread into the return object when slashIndex <= 0, computing it with slice(0, -1) is confusing and easy to misuse later. Consider only computing providerOverride inside the slashIndex > 0 branch.
| constructor(private readonly env: ControllerEnv) {} | ||
| constructor( | ||
| private readonly env: ControllerEnv, | ||
| private readonly openclawProcess: OpenClawProcessManager, |
There was a problem hiding this comment.
Changing OpenClawWatchTrigger to require an OpenClawProcessManager in the constructor is a breaking API change, but there are still call sites constructing it with a single argument (e.g. controller tests like apps/controller/tests/openclaw-sync.test.ts and apps/controller/tests/route-compat.test.ts). Those will fail to typecheck/run. Please update all instantiations (tests + any helpers) to pass a process manager (or provide a backwards-compatible default).
| private readonly openclawProcess: OpenClawProcessManager, | |
| private readonly openclawProcess?: OpenClawProcessManager, |
What
Normalize update fetch/network failure errors in the desktop renderer so Chinese users no longer see the raw English string
fetch failedin update failure notifications.Why
Closes #994.
On Windows with zh-CN UI, update failures could surface as mixed-language notifications (
更新失败with rawfetch faileddetail), which is confusing and breaks localization consistency.How
normalizeUpdateErrorMessagefrom the update hook for direct regression testing.fetch failedfailed to fetchNetworkError ... fetchNetwork connection failed while checking for updates. Check your connection and try again.网络连接失败,请检查网络后重试。Affected areas
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpassespnpm generate-typesrun (if API routes/schemas changed)anytypes introduced (useunknownwith narrowing)Screenshots / recordings
Reproduction:
Fixed:
Notes for reviewers
apps/desktop/src/hooks/use-auto-update.tstests/desktop/update-error-message-normalization.test.ts