feat(desktop): updater progress indicator and reworked failure dialog by Astro-Han · Pull Request #222 · Astro-Han/pawwork

Astro-Han · 2026-04-24T12:27:07Z

Summary

Implements the v5.2 design in #213:

During a download, broadcast setProgressBar(info.percent / 100) to every open BrowserWindow so macOS dock and Windows 10/11 taskbar icons show motion.
New windows opened mid-download reapply the current progress on ready-to-show.
Menu path's failure dialog now shows reason-specific copy for check / download / metadata / cache, includes the controller's own error message in the detail line, reassures that the current version is unaffected, and offers Retry / Open Download Page / Later.
Install failure has its own dedicated dialog with installFailedMessage and only Open Download Page / Later (retry does not help after install failure).

No renderer changes. No new IPC. No new UI primitives.

Follow-up for network reliability (CDN / mirror) is tracked in #219.

Test plan

bun test packages/desktop-electron/src/main/updater-dialog-labels.test.ts
bun test packages/desktop-electron/src/main/index-updater-source.test.ts
bun --cwd packages/desktop-electron test (282 pass)
bun --cwd packages/desktop-electron typecheck
Biome lint on touched files (1 pre-existing unrelated warning on import { type MenuLocale }, not in this PR's scope)
Manual smoke on packaged dev build (requires real UPDATER_ENABLED + real feed; not runnable from bun dev) — gating before release

Summary by CodeRabbit

New Features
- Download progress shown via window progress bars and reapplied when windows become visible.
- Failure dialogs now show localized, reason-specific messaging and structured action buttons; retry triggers another update check and opens releases when chosen.
Bug Fixes
- Progress cleared on completion, terminal updater states, and errors.
- Progress listeners not registered when updater is disabled.
Localization
- Simplified Chinese and English failure labels updated and expanded.
Tests
- Added tests validating progress broadcast, clearing behavior, retry flow, and dialog content.

… download fallbacks

coderabbitai · 2026-04-24T12:27:20Z

📝 Walkthrough

Walkthrough

Adds download-progress tracking (percent→currentProgress), broadcasts progress to all BrowserWindows via setProgressBar and reapplies on ready-to-show, clears progress on terminal/error events, replaces generic alerts with localized, reason-specific failure dialogs with action buttons (retry, open download page, later), and expands tests/labels accordingly.

Changes

Cohort / File(s)	Summary
Tests `packages/desktop-electron/src/main/index-updater-source.test.ts`, `packages/desktop-electron/src/main/updater-dialog-labels.test.ts`	Adds tests for download-progress computation (info.percent/100), broadcasting to windows and use of `setProgressBar`, clearing progress on terminal/error events, no listener registration when `UPDATER_ENABLED=false`, reapplying progress on `ready-to-show`, and strict failure-dialog/label expectations including retry/open-download-page/later flows and install-failure variant.
Main process updater `packages/desktop-electron/src/main/index.ts`	Records `download-progress` into `currentProgress`, broadcasts `BrowserWindow.setProgressBar(currentProgress)` to all windows, reapplies progress for windows on `ready-to-show`, clears progress on `update-downloaded`/`update-not-available`/`update-cancelled`/`error`, and replaces simple alerts with localized, reason-specific dialogs wired to actions (retry → `checkForUpdates`, open releases/latest).
Localized labels `packages/desktop-electron/src/main/updater-dialog-labels.ts`	Introduces richer `FailedLabels` shape and updates English/Chinese entries: `failed.title`, `installFailedMessage`, reason-specific messages (check/download/metadata/cache), `currentVersionUnaffected`, and action labels `retry`, `openDownloadPage`, `later`.

Sequence Diagram(s)

sequenceDiagram
    participant Updater as Updater
    participant Main as Main Process
    participant Window as BrowserWindow
    participant User as User

    Updater->>Main: emit "download-progress" (info.percent)
    Main->>Main: currentProgress = info.percent / 100
    Main->>Window: broadcast setProgressBar(currentProgress)
    Window->>User: OS/taskbar progress reflects download

    Updater->>Main: emit "update-downloaded" / "update-not-available" / "update-cancelled"
    Main->>Main: clear currentProgress
    Main->>Window: broadcast setProgressBar(-1)

    Updater->>Main: emit "error" (reason)
    Main->>Main: clear currentProgress
    Main->>Window: broadcast setProgressBar(-1)
    Main->>User: show localized failure dialog (message + detail, buttons)

    User->>Main: click "retry"
    Main->>Updater: call checkForUpdates(alertOnFail)
    Main->>Main: log rejection on failure

    User->>Main: click "open download page"
    Main->>User: open URL (releases/latest)

    Note over Main,Window: New windows on "ready-to-show" reapply setProgressBar(currentProgress)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

[Feature] Improve updater feedback for slow GitHub access #213: Implements the same updater behaviors (progress → setProgressBar, clearing on terminal events, ready-to-show reapplication, reason-specific failure dialogs, retry/open-download-page actions).

Possibly related PRs

fix: harden updater stale pending handling #178: Related updater logic changes (cache handling, expanded UpdateFailureReason) that intersect with the updated failure reasons and dialog flows in this PR.

Poem

🐰
Bars hop along each window's face,
Percent to progress, keeping pace.
If updates fail, I whisper “try,”
Or open releases — off you fly.
A rabbit cheers the updater race!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main changes: updater progress indicator display and reworked failure dialogs.
Description check	✅ Passed	The description covers Summary, Why (context), test plan, and links related issues; however, it is missing Screenshots/Recordings section and incomplete Checklist items.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/updater-feedback

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

gemini-code-assist

Code Review

This pull request implements progress bar tracking for the auto-updater and introduces more detailed, localized error dialogs for update and installation failures. Feedback focuses on improving maintainability and robustness by centralizing the release URL as a constant, implementing a custom type guard for error handling instead of relying on instanceof Error, and using regular expressions in tests that inspect source code to ensure they are resilient to formatting changes.

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/desktop-electron/src/main/index-updater-source.test.ts`:
- Around line 52-54: The test assumes the ready-to-show hook exists but slices
source using hookIndex which can be -1; add an explicit guard asserting
hookIndex !== -1 (e.g., expect(hookIndex).not.toBe(-1) or throw with a clear
message) before calling source.slice so the test fails clearly if the
win.once("ready-to-show") hook is missing; update the test around the
hookIndex/source.slice/reapplySlice logic to perform this check prior to
slicing.

In `@packages/desktop-electron/src/main/index.ts`:
- Around line 685-686: The call to shell.openExternal(LATEST_RELEASE_URL)
(invoked from checkForUpdates and the menu handler) can reject and is currently
awaited with no local error handling, causing unhandled promise rejections; wrap
the shell.openExternal(...) invocation inside a try/catch (or attach a .catch)
within checkForUpdates (and the menu handler invocation) to catch errors and log
them (e.g., via processLogger.error or console.error) and avoid propagating the
rejection, ensuring the function still returns/continues gracefully.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 65aaab6e-2f12-4ea3-913b-f93f7fa73d03

📥 Commits

Reviewing files that changed from the base of the PR and between 6a4315b and 1c0b8ed.

📒 Files selected for processing (2)

packages/desktop-electron/src/main/index-updater-source.test.ts
packages/desktop-electron/src/main/index.ts

📜 Review details

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)

GitHub Check: unit-windows-opencode-server-tools
GitHub Check: unit-windows-app
GitHub Check: unit-windows-desktop
GitHub Check: typecheck
GitHub Check: unit-windows-opencode-session
GitHub Check: unit-windows-opencode-config-project
GitHub Check: unit-opencode
GitHub Check: unit-app
GitHub Check: unit-desktop
GitHub Check: smoke-macos-arm64
GitHub Check: analyze-js-ts
GitHub Check: e2e-artifacts

🧰 Additional context used

📓 Path-based instructions (1)

packages/desktop-electron/src/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (packages/desktop-electron/AGENTS.md)

Renderer process should only call window.api from src/preload

Files:

packages/desktop-electron/src/main/index.ts
packages/desktop-electron/src/main/index-updater-source.test.ts

🧠 Learnings (10)

📚 Learning: 2026-04-20T14:36:08.774Z

Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/desktop-electron/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:08.774Z
Learning: Applies to packages/desktop-electron/src/main/ipc.ts : Main process should register IPC handlers in `src/main/ipc.ts`

Applied to files:

packages/desktop-electron/src/main/index.ts

📚 Learning: 2026-04-20T14:21:56.373Z

Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 71
File: packages/app/src/components/session/session-status-connections.tsx:146-147
Timestamp: 2026-04-20T14:21:56.373Z
Learning: In the Astro-Han/pawwork repository (SolidJS app), `sync.data.config` is always initialized to `{}` at `packages/app/src/context/global-sync.tsx` line 71 and is never `undefined` at runtime. Non-optional property access like `sync.data.config.plugin` is intentional and consistent with the pattern used in `packages/app/src/components/status-popover-body.tsx` line 243. Do not flag `sync.data.config.plugin` as needing optional chaining.

Applied to files:

packages/desktop-electron/src/main/index.ts

📚 Learning: 2026-04-20T14:36:21.288Z

Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/opencode/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:21.288Z
Learning: Applies to packages/opencode/**/*.ts : In `Effect.gen` / `Effect.fn`, prefer `yield* new MyError(...)` over `yield* Effect.fail(new MyError(...))` for direct early-failure branches

Applied to files:

packages/desktop-electron/src/main/index.ts

📚 Learning: 2026-04-22T08:49:47.800Z

Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/desktop-electron/src/main/index-sidecar-source.test.ts:3-11
Timestamp: 2026-04-22T08:49:47.800Z
Learning: In `packages/desktop-electron/src/main/index-sidecar-source.test.ts` (Astro-Han/pawwork), the test intentionally uses `expect(source).toContain` / `expect(source).not.toContain` string matching against the raw `index.ts` source text as a lightweight sidecar contract guard. The maintainer has explicitly chosen not to introduce an AST parser (e.g., `babel/parser` or acorn) for this purpose. Do not flag these string-based assertions as fragile or suggest converting them to AST-based matching.

Applied to files:

packages/desktop-electron/src/main/index-updater-source.test.ts

📚 Learning: 2026-04-22T09:32:54.556Z

Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/opencode/test/provider/provider.test.ts:64-85
Timestamp: 2026-04-22T09:32:54.556Z
Learning: In `packages/opencode/test/provider/provider.test.ts`, the file intentionally uses AppRuntime-based async helpers (`run`, `list`, `getProvider`, etc.) rather than `testEffect(...)` for all tests. Converting individual tests to `testEffect` while leaving the rest on the async pattern would create internal inconsistency. A full harness migration of this file is the right approach if the pattern needs to change, but that should be a separate PR.

Applied to files:

packages/desktop-electron/src/main/index-updater-source.test.ts