fix(harmony): 修复首次加载标记重复消费导致误触发 rollback#586
Conversation
- 新增 consumeFirstLoadMarker,消费后清除标记,避免同一进程多次解析 bundleURL 误触发 rollback - 新增 ignoreRollback 标志位,对齐安卓 UpdateContext.java 同名逻辑 - resolveLaunch 接入 ignoreRollback 参数,switchVersion 切换版本时重置该标志 - clearFirstTime 与 persistState 维护 firstLoadMarked 标记同批原子落盘 Committed at: 2026-06-29 18:46 Committed by: liuqiang Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthrough
ChangesFirst-load marker and ignoreRollback coordination
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
harmony/pushy/src/main/ets/PushyTurboModule.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: .eslintrc.js » harmony/pushy/src/main/ets/UpdateContext.tsOops! Something went wrong! :( ESLint: 8.57.1 Error: .eslintrc.js » Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
harmony/pushy/src/main/ets/PushyTurboModule.ts (1)
78-85: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winConsume the marker only after fallible constants work succeeds.
Line 78 deletes the first-load marker before
clearRollbackMark()runs. If that later state operation throws,getConstants()will not return to JS, but the marker is already gone, so a retry reportsisFirstTime: false.Proposed fix
- const isFirstTime = this.context.consumeFirstLoadMarker(); const rolledBackVersion = this.context.rolledBackVersion(); const uuid = this.context.getKv('uuid'); const isUsingBundleUrl = this.context.getIsUsingBundleUrl(); if (rolledBackVersion) { this.context.clearRollbackMark(); } + + const isFirstTime = this.context.consumeFirstLoadMarker();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@harmony/pushy/src/main/ets/PushyTurboModule.ts` around lines 78 - 85, Move the first-load marker consumption in PushyTurboModule.getConstants so it happens only after the fallible state work succeeds, especially after clearRollbackMark() in the rolledBackVersion branch. Use the existing symbols consumeFirstLoadMarker(), clearRollbackMark(), and getConstants() to reorder the logic so a throw leaves the marker intact for retry.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@harmony/pushy/src/main/ets/UpdateContext.ts`:
- Around line 29-31: Make `ignoreRollback` process-wide instead of being reset
per `UpdateContext` instance; the current `private ignoreRollback = false` in
`UpdateContext` lets a new context in the same process re-enable rollback
checks. Update the `UpdateContext` class so the flag is shared across instances
(or stored on a process-level singleton/static), and ensure `getBundleUrl()` and
the rollback path around `shouldRollback`/`firstTime` consult and update that
shared state consistently so a second resolution in the same process cannot
trigger rollback again.
---
Outside diff comments:
In `@harmony/pushy/src/main/ets/PushyTurboModule.ts`:
- Around line 78-85: Move the first-load marker consumption in
PushyTurboModule.getConstants so it happens only after the fallible state work
succeeds, especially after clearRollbackMark() in the rolledBackVersion branch.
Use the existing symbols consumeFirstLoadMarker(), clearRollbackMark(), and
getConstants() to reorder the logic so a throw leaves the marker intact for
retry.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1944a829-970a-4b7c-aec7-56fad44cedaa
📒 Files selected for processing (2)
harmony/pushy/src/main/ets/PushyTurboModule.tsharmony/pushy/src/main/ets/UpdateContext.ts
| // 对齐安卓 UpdateContext.java:30。bundleURL 可能在同一进程内被解析多次, | ||
| // 首次消费 firstTime 后置 true,避免后续解析误触发 rollback。 | ||
| private ignoreRollback = false; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Make ignoreRollback process-wide, not instance-local.
Line 31 resets to false for every new UpdateContext, but the provider path constructs a new UpdateContext before resolving getBundleUrl(). A second resolution through a new context in the same process can still pass false at Line 392 and re-trigger the rollback this PR is trying to suppress.
Proposed fix
- private ignoreRollback = false;
+ private static ignoreRollback = false;
...
- this.ignoreRollback = false;
+ UpdateContext.ignoreRollback = false;
...
- this.ignoreRollback,
+ UpdateContext.ignoreRollback,
...
- this.ignoreRollback = true;
+ UpdateContext.ignoreRollback = true;Also applies to: 379-405
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@harmony/pushy/src/main/ets/UpdateContext.ts` around lines 29 - 31, Make
`ignoreRollback` process-wide instead of being reset per `UpdateContext`
instance; the current `private ignoreRollback = false` in `UpdateContext` lets a
new context in the same process re-enable rollback checks. Update the
`UpdateContext` class so the flag is shared across instances (or stored on a
process-level singleton/static), and ensure `getBundleUrl()` and the rollback
path around `shouldRollback`/`firstTime` consult and update that shared state
consistently so a second resolution in the same process cannot trigger rollback
again.
1 participant
Committed at: 2026-06-29 18:46
Committed by: liuqiang
Summary by CodeRabbit