feat: OCDS v1.1 conformance — desktop concurrent download + shared update orchestration (OK-56573)#12121
Conversation
…date orchestration Desktop (Node) concurrent downloader: - validate each segment 206 Content-Range vs the requested window + clamp over-long bodies (no cross-segment overrun); reject multipart/byteranges - single-stream 200-on-resume restarts cleanly (no append corruption) - exhaustive HTTP-status classification (416 transient, 401/501/505 permanent, catch-all default); bytes-stalled timeout + jittered backoff + Retry-After - per-destination single-flight Shared AppUpdate orchestration: - exhaustive error taxonomy + §4 catch-all default rule - persisted cross-restart attempt budget + wall-clock deadline + terminal give-up contract (opt-in APIs; production caller wiring is a follow-up) Tests import and exercise the real exported classifiers/validators. Verified against OCDS v1.1 (all four platform classifiers consistent).
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
…oader A shared fault-injection harness (local Range HTTP server + seed/plan helpers) drives the REAL downloadBundleConcurrent / downloadBundleSingleStream end-to-end and maps tests 1:1 to the OCDS §6 scenarios: T1 drop-segment→only-missing · T2 full+resume · T3 200→permanent · T4 429/Retry-After→keep+retry · T5 misaligned/over-long/short 206 → reject/clamp no corruption · T6 corrupt→SHA mismatch terminal (bounded) · T7 monotonic progress · T8 cancel→stop+cleanup · T10 stall watchdog→retry · T11 same-dest single-flight · single-stream resume + G2 200-on-resume. T9 (cross-restart budget) is the shared-JS layer, out of scope here. Production: make the §5.4 stall watchdog timeout injectable via an optional constructor `stallTimeoutMs` (defaults to BUNDLE_STALL_TIMEOUT_MS) so T10 runs in <1s instead of 60s — no behavior change for production callers. 15 tests, 5 suites, all green; tsc clean.
Adds updateRetry.test.ts: a fast, dependency-light suite pinning runDownloadWithRetry's OCDS §5.11 termination contract — entry-guard on an already-exhausted budget, persisted-counter accumulation across simulated relaunches → terminal DownloadGaveUpError(maxAttempts), deadline reason, single-stream fallbackFailed wrapping, success-resets-budget, and the legacy (no-hooks) path. Complements the persistence-half tests in ServiceAppUpdate.test.ts (budget durability/per-target reset). 6 tests, all green.
The config ignored .worktree/ (singular) but not .worktrees/ (plural); a git worktree checked out under <rootDir>/.worktrees/ polluted the haste module map (duplicate @onekeyhq/components / manual mocks), making any component-importing test suite fail to run locally. Add the plural to modulePathIgnorePatterns and testPathIgnorePatterns.
…dPackage Activates the cross-restart attempt budget in production: downloadPackage now passes ServiceAppUpdate's getDownloadAttemptBudget / recordDownloadAttempt / resetDownloadAttemptBudget (and refreshDownloadUrlForRetry for 401/403 URL refresh) to runDownloadWithRetry, keyed per target (appVersion:bundleVersion). Effect: a permanently-failing update eventually gives up (DownloadGaveUpError) and the next relaunch is terminal on entry instead of re-downloading from scratch; success resets the budget and a new target version starts fresh. The download URL is held mutably so a refreshed signed URL is used on the next attempt. Tests: useAppUpdate.test.ts mock gains the budget hooks (now reachable) + a new case proving an exhausted budget gives up without re-downloading. 155/155 pass.
…iendly) The 24h deadline was calendar time measured from the first attempt, so a user who downloaded part of an update and reopened the app days later was denied the (still valid) resume — idle / app-closed time wrongly counted toward give-up. Removed it; the persisted attempt-count budget alone bounds a permanently- failing target (attempts only accrue on real failures, never on idle gaps). evaluateDownloadBudget now gives up on attempt count only; DOWNLOAD_WALL_CLOCK_DEADLINE_MS removed; result reason narrowed to 'maxAttempts'; firstAttemptAt retained for telemetry. The deadline test is replaced by a guard proving a 30-day idle gap does NOT give up. 4/4 budget tests green.
Prettier line-wrapping only (multi-line type unions / long-line wraps) for the desktop e2e suite + harness and the appupdate retry tests. No behavior change; desktop e2e 15/15 + appupdate suites still green.
…date End-to-end guard: reproduce a give-up's persisted effects (exhaust the per- target attempt budget + set downloadPackageFailed), then a newer version arrives → fetchAppUpdateInfo recovers failed→notify (user re-prompted) and the new target reports a fresh budget. Locks in that the give-up reuses the recoverable downloadPackageFailed status (not a special terminal) and that a stale per-target give-up never blocks a newer version's download. 228/228 green.
…ngle-stream handoff - e2e harness: add a status416 fault mode (mirrors status5xx/status429ThenOk). - e2e.transient416: 416 a few times on a segment -> transient retry-in-place, survivors kept, recovers, correct SHA, no restart-from-0 (concurrent); plus the single-stream open-ended-resume 416 path (SPEC #4). - e2e.handoff: drive downloadBundle into a concurrent permanent fallback and assert the concurrent->single-stream HANDOFF completes end-to-end with correct SHA (previously the throw and the single-stream success were only asserted separately) (SPEC #3). All 3 pass against the real DesktopApiBundleUpdate. (N4 true-restart-resume + mutation verification pending — interrupted by a session limit.)
c327056 to
e95269d
Compare
OCDS conformance — adversarial review summary (desktop / shared JS)Independent multi-agent audit against [OCDS SPEC v1.2], each high-severity finding re-checked by a separate skeptic. No blockers — the SHA256 verify is a real backstop on every path, so no corrupt bundle is installed or reported complete. Node e2e coverage confirmed real (18/18, mutation-proven: flipping 416→permanent and Cleanup landed (history rewrite)This branch's Confirmed correct
Backlog — real but accepted (not fixed here)
Full report available on request. |
…oxlint) - DesktopApiBundleUpdate.test.ts: add the JEST_MOCK_BLOCK so importing the real module under node-jest no longer throws on electron's value import. - e2e harness: reach setTimeout().unref via the Node timer shape (the DOM number overload has no unref under this tsconfig lib). - lifecycle e2e: narrow the single-flight result before readFileSync. - add file-level no-unsafe-call/no-unsafe-return (and one no-raw-error) eslint-disable to the e2e specs, matching the repo test convention. - spellCheckerSkipWords: register OCDS download domain terms.
The version-consistency check already skipped .claude/worktrees and the singular .worktree/; align it with jest.config.js by also excluding the plural .worktrees/ so local git worktrees never trip the monorepo check.
refreshDownloadUrlForRetry fires on a 401/403 download retry to re-sign the expired signed URL, but called fetchAppUpdateInfo(false). Within the normal sync-throttle window isNeedSyncAppUpdateInfo short-circuits to the cached atom (skip_sync) and never hits the server, so the dead URL is never re-signed -> after.downloadUrl == beforeUrl -> the retry gives up as urlDead even though the failure was recoverable. Force the refresh (true) so it bypasses both the sync gate and getAppLatestInfo's 5-min cache and the server actually re-signs. Reachable when the deployment's signed-URL TTL is shorter than the throttle window and a slow/interrupted download outlives the TTL. Adds refreshDownloadUrlForRetry coverage (previously only mocked away in useAppUpdate.test.ts), including a mutation-proven guard on the force flag.
On a single-stream resume the mis-aligned-206 branch (server Content-Range start != requested downloadedBytes) dropped the partial and reset downloadedBytes to 0 but kept consuming the SAME response body, which begins at the server's resumeStart. Writing that body from file offset 0 stored object[resumeStart..end] as [0..], leaving the [0..resumeStart) prefix permanently missing -- a short, corrupt file that only fails at SHA verification, and whose leftover partial can compound on later resumes. The size + SHA gate prevents a corrupt bundle from ever being promoted, so this never installs bad data, but it burns retry attempts. Abort the request (destroy response + request), drop the partial, and reject with a transient error; the outer runDownloadWithRetry loop then re-issues a fresh request from byte 0 (no partial => no Range => clean 200/206). Mirrors the existing 416/HTTP-error reject-and-return pattern; do not copy the 200 branch's reset-and-continue, which is only valid because a 200 body legitimately starts at byte 0.
OK-56573
Brings the desktop (Node) concurrent downloader and the shared AppUpdate orchestration into conformance with the OneKey Concurrent Download Standard (OCDS v1.1) (defined in the app-modules repo:
native-modules/react-native-range-downloader/SPEC.md).Desktop concurrent downloader
Content-Rangevs the requested window + clamp over-long bodies (no cross-segment overrun); rejectmultipart/byterangesRetry-AfterShared AppUpdate orchestration
useDownloadPackageis a deliberate follow-up (behavior change)Validation
tscclean; AppUpdate suites passFollow-ups (not in this PR)
Pairs with the app-modules PR (iOS/Android native + the OCDS standard).
✅ Verification record
How every platform's downloader is verified against the standard, and how each
test was proven load-bearing (a green test that survives its own regression
proves nothing). Full methodology + runnable harness:
react-native-range-downloader/conformance/.Approach
flowchart TD SPEC["OCDS SPEC v1.2<br/>§6 — 11 conformance scenarios"] SPEC --> AND["Android<br/>pure-JVM + OkHttp MockWebServer<br/>real fault injection"] SPEC --> NODE["Node / Desktop<br/>jest + real local HTTP/HTTPS server"] SPEC --> IOSU["iOS · unit<br/>SwiftPM over the real RangeDownloadLogic"] SPEC --> IOSE["iOS · end-to-end<br/>real Release .app on Simulator<br/>+ local fault server (silent auto-download)"] AND --> MUT["Mutation verification<br/>inject the regression each test targets<br/>→ the test MUST go red"] NODE --> MUT IOSE --> EVID["evidence: server log + app-latest.log<br/>+ on-disk .segN + final SHA"] EVID --> ADV["adversarial verify<br/>(default FAIL unless 3 sources agree)"] MUT --> OK(["load-bearing ✔"]) ADV --> OKSPEC §6 coverage matrix
200to a Range → permanent → single-stream416/5xxtransient (re-evaluate, retry)206download()→ single-flight¹ resume tested via a persisted-manifest proxy — a true SIGKILL-mid-write isn't reproducible in-process; covered by the manifest-resume + intra-call-resume tests.
² Android progress: the clamp/guard math is unit-tested, but the monotonic CAS primitive stays inline in the JNI adapter (not unit-testable).
³ iOS #8/#11 need a temp shim (the iOS app has no cancel trigger at all — #8 would test dead code); both behaviors are fully covered on Android + Node.
⁴ Node #9 give-up budget lives in the shared kit caller (
useAppUpdate.test.ts), not the downloader.Test counts & strength
.segN, real SHA).progress.json.appon a booted SimulatorMutants planted & killed (load-bearing proof)
flowchart LR subgraph Android a1["503 → permanent"] --> ka(["🔴 caught"]) a2["416 → permanent"] --> ka a3["bypass multipart"] --> ka a4["drop total check"] --> ka a5["501 → transient"] --> ka a6["retry budget +2"] --> ka a7["registry identity-drop"] --> ka a8["progress clamp removed"] --> ka a9["seg-glob narrowed"] --> ka end subgraph Node n1["416 → permanent"] --> kn(["🔴 caught"]) n2["break single-stream 416"] --> kn n3["isConcurrentFallback→false"] --> kn endEvery mutant produced a real test failure (not a compile error) and was reverted.
Honesty / scope
#6is now conformant everywhere.RangeDownloadLogic.kt, iOSRangeDownloadLogic.swift) are pure moves — proven by the pre-existing real-path suites staying green.🔩 Android APK updater — audit + fixes
The Android APK download path (
react-native-app-update) shares the OCDS core, but its ~1346-line caller was never audited and had zero tests. An adversarial audit found 4 issues.Issues
213acf0f3)cancelHandle→clearCachedeleted.segNwhile 8 workers were still writing. WiredCancelHandle+ cancel-before-delete (mirrors the bundle caller).24d5973cb)update/errorwhen the run was intentionally cancelled.011c24a58)erroron every retry; suppressed (the Promise still rejects → JS still retries).Coverage (
b9caf71f9)Extracted the dependency-free pure logic into
AppUpdateLogic.kt(segment names + count, 416/206Content-Rangeparse, completeness/alignment predicates); the adapter now delegates (pure move). Added junit + 17 JVM unit tests, with all 10 planted mutants killed (mutation-proven). Also wired a previously-untested inline 206-parse duplicate through the helper.Honesty / scope
Still device/Robolectric-only: the JNI download loop, the actual
.segNdeletion + §5.8 cancel-race execution, and the promote/verify/install File-I/O are not exercised by these JVM tests.