Skip to content

fix(desktop): replace a latched update error with the latest failure - #6532

Open
sharpca wants to merge 1 commit into
apache:mainfrom
sharpca:fix/4708-update-error-latch
Open

sharpca wants to merge 1 commit into
apache:mainfrom
sharpca:fix/4708-update-error-latch

Conversation

@sharpca

@sharpca sharpca commented Nov 1, 2026

Copy link
Copy Markdown

Summary

The Software update panel latched the first failure forever. Once AppUpdateStatus reached error, both the post-retry catch and its retry catch returned the existing status instead of publishing the new one, so a transient failure (for example a trust-root error that later recovered) stayed on screen for the life of the process, and pressing the button looked like it did nothing.

  • Publish the later failure instead of returning the latched status. Identical repeats stay idempotent, so one failed check — reported through both the updater error listener and the retry catch — still publishes a single status.
  • Give provenance verification its own verify operation, so a signature failure no longer renders under Check for updates or reads as a download failure.
  • Add the matching localised copy for verify in en / zh-CN / zh-TW.

Fixes #4708

Verification

  • node --test dist/main/__tests__/app-update-service.test.js — 22/22 pass.
  • With the fix reverted, the new case fails with expected: 'second failure' / actual: 'first failure', so the test does exercise the latch.
  • node --test dist/main/__tests__/app-update-*.test.js — 48/48 pass across 7 files.
  • tsc -p tsconfig.main.json --noEmit and tsc -p tsconfig.renderer.json --noEmit — pass.
  • biome check on the four changed files — pass.
  • Not run: the two packaged-runtime attestation tests in app-update-attestation.test.js need the Electron binary, which is not installed here; they do not touch this change.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: 灵犀 (WPS AI agent) wrote the fix, the regression test and the localised strings. The commit carries a Generated-by: 灵犀 (WPS AI agent) trailer; @sharpca is the human contributor of record and reviewed the change.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

The update panel kept the first failure forever: once the status reached
`error`, both the post-retry catch and its retry catch returned the
existing status instead of publishing the new one, so a stale
"root was signed by 0/3 keys" stayed on screen long after the trust root
recovered.

Publish the later failure, and give provenance verification its own
`verify` operation so a signature failure no longer reads as a download
or check failure. Identical repeats stay idempotent, which keeps one
failed check - reported through both the updater error listener and the
retry catch - at a single status.

Fixes apache#4708
Generated-by: 灵犀 (WPS AI agent)
@github-actions github-actions Bot added the effort/S Under 100 readable lines label Nov 1, 2026

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

I reviewed all four changed files at 7909fd38d720004cff3a76971e86658c5e5d7f85, plus the surrounding updater event/retry/install flow and the renderer's copy.updateFailed[status.operation] consumers. I found no confirmed P0–P2 issue in this change.

app-update-service.ts:261 suppresses only an identical operation/message pair. The retry catches at lines 452 and 467 now publish a later failure instead of returning an unrelated earlier error. Verification failures use verify; the shared status union and English, Simplified Chinese and Traditional Chinese labels include that value. Installation still requires the verified downloaded state, so the new classification does not bypass provenance verification.

All 22 original update-service tests passed locally against the production TypeScript. The harness injects its FakeUpdater and clock; I stubbed only the electron-updater singleton import and resolved local .js imports to their original .ts sources. Restoring just the old error-latching guards makes the new regression fail with actual first failure, expected second failure, while the submitted code passes. This independently confirms the stale-error premise in the implemented path.

A static merge into current main (85a1031c10d104456127f35aac1523b9ffb1a384) succeeds. The PR is currently OPEN/MERGEABLE. The label check succeeded, but CI and Release Windows check are ACTION_REQUIRED, rather than successful test runs. I have not run a packaged Electron updater, real downloads/installations, the full renderer build or TypeScript projects. The author's broader results remain author-reported. No protocol epoch changes here; the substantive-tool declaration and commit trailer are present. This is a COMMENT, not merge or release approval.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/S Under 100 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The update panel latches one error and labels a provenance failure as a check failure

2 participants