Repository navigation
Conversation
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)
Astro-Han
left a comment
There was a problem hiding this comment.
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.
Summary
The Software update panel latched the first failure forever. Once
AppUpdateStatusreachederror, 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.errorlistener and the retry catch — still publishes a single status.verifyoperation, so a signature failure no longer renders under Check for updates or reads as a download failure.verifyin en / zh-CN / zh-TW.Fixes #4708
Verification
node --test dist/main/__tests__/app-update-service.test.js— 22/22 pass.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 --noEmitandtsc -p tsconfig.renderer.json --noEmit— pass.biome checkon the four changed files — pass.app-update-attestation.test.jsneed the Electron binary, which is not installed here; they do not touch this change.AI use
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
Does this PR entail a change in behavior?