Repository navigation
refactor: cross-layer cleanup and sink duplicated native logic into the shared C++ cores - #657
Merged
Merged
Conversation
Move check-constant-parity.js from harmony/pushy/src/test to scripts/ and extend it to compare archive_limits.h / install_record.h / error_codes.h against ArchiveLimits.java / InstallRecord.java / ErrorCodes.java and the error codes known to src/error.ts. Runs as part of bun lint. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
jsonDeepEqual (native config equivalence, C1) called Object.hasOwn, which throws on iOS < 15.4 JSC. Use Object.prototype.hasOwnProperty.call and add a test that runs the comparison with Object.hasOwn removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
client.ts carried its own deep-equal (jsonValuesEqual) for the native check-cache request comparison; the only difference to updateFlowCore's jsonDeepEqual was skipping undefined-valued members. Fold that rule into jsonDeepEqual (a no-op for the JSON-parsed native configs it already compared; flow vectors unchanged) and export it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The union type and KNOWN_CODES listed every code twice; keep a single 'as const' array and derive both from it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The native cold-start orchestrator retried every mirror of a strategy even when the artifact downloaded fine but could not be applied (PATCH_FAILED). Every mirror serves the same bytes, so match JS client.ts and the C++ IsMirrorRetryableCode: break out to the next strategy instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On web PushyModule is a noop Proxy, so every 'typeof PushyModule.x === "function"' feature detect was true and each call site carried its own Platform.OS === 'web' guard plus try/catch to turn a synchronous bridge throw into a rejection. nativeMethod(name) returns undefined on web and for older natives, and always returns a promise; the per-call guards and the dead web branch of ping() (web never reaches a download) are gone. The VM-evaluated client harness in nativeConfiguration.test.ts depended on client.ts's free identifiers; its assertions now live in client.test.ts (native ownership suppresses constructor/setOptions writes until handed back), so it is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- UpdateContext isFirstTime()/clearFirstTime() and the ArkTS STATE_OP_CLEAR_FIRST_TIME mirror (the C++ enum stays). - DownloadTask types that are never created (TASK_TYPE_CLEANUP, TASK_TYPE_PLAIN_DOWNLOAD): doCleanUp, their switch cases, the plain download failure branch and the now always-true isPatchTask checks. The params default type becomes TASK_TYPE_PATCH_FULL. - Test-only ErrorCodes.parseErrorCodePrefix, unused ARCHIVE_PATCH_TYPE_FULL, an empty extern "C" block in pushy.cpp. - PushyPackage.h used a copy-pasted GEO_LOCATION_PACKAGE_H guard. - BuildProfile.ets is regenerated by hvigor on every build (nothing imports it): untrack and ignore it. OAT.xml was an unused template. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- getRemoteEndpoints no longer filters out configured endpoints: the endpoint fallback already dedupes candidates and drops failed ones. - getConfiguredCheckEndpoints returns server.main as is (deduplicated on assignment and again by the endpoint plan). - Inline the single-caller getCheckUrl; the per-endpoint failure log moves from fetchCheckResult's tryEndpoint wrapper into requestCheckResult. - endpoint.ts no longer re-exports dedupeEndpoints; fetchWithTimeout drops the unused params.signal fallback; drop explicit default timeouts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…flags Expo SDK 50+ (the only caller of the node fallback) needs Gradle 8, so the Project.exec branch is dead. Set version once, drop the commented useExpoPublishing, and declare Java 8 explicitly so lambdas compile on AGP < 4.2. Android.mk repeated -std=c++17 and --exclude-libs that Application.mk already applies to every module. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ewStringUTF update_core_android.cpp passed std::string values to NewStringUTF, which expects JNI modified UTF-8 and mangles supplementary (4-byte) code points. Move update_flow_jni.cpp's correct UTF-8 -> UTF-16 ToJString into the shared jni_util.h and use it for every Java string the patch/state glue returns. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
reportExceptionToServer and reportToServer each built the same POST to
{endpoint}/report/{appKey}; both now go through postReport. The event
shape {type,message,code,data} is a single ReportEvent type, the
single-caller send closure is inlined, and captureException's guard absorbs
the duplicated opt-out checks (plus the no-hot-update check).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- delta::BuildPatchManifest (patch_core.h): one pure builder for the parallel copy_froms/copy_tos lists with the length-mismatch check, so the Harmony bridge can reuse it too; jni_util::ReadPatchManifest wraps it for the three JNI entry points that built it by hand. - jni_util::GuardJni replaces the six copies of the catch(std::exception)/catch(...) -> RuntimeException block. - Drop the pure forwarders ToArchivePatchType and MakeStateResult (NewStateCoreResult takes the default arguments now). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The toUpdateError -> emitError -> throw/throwIfEnabled sequence was spelled out at every catch site; fail() does the first two and returns the error. runBeforeReload now wraps a beforeReload throw as USER_HOOK_ERROR itself (event type/data derived from the context) instead of switchVersion and restartApp each doing it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…round The cold-start orchestrator re-downloaded the same artifact from every mirror even when the failure was a patch failure, which no mirror can change. Break out to the next strategy like update_flow_core's IsMirrorRetryableCode and the JS client do. The download-failure code mapping moves to ErrorCodes so the module and the orchestrator share it, and the attempt loop takes a DownloadStarter so it is JVM-testable. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Same rule as Harmony's SyncCoordinator (isHttpsOnly): when every configured endpoint is https, endpoints fetched from queryUrls and artifact URLs in the decision must not downgrade the round to http. Non-https remote endpoints and cleartext artifact URLs are skipped. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- patchFailed/patchErrorMessages are exactly the exhausted strategies (only PATCH_FAILED exhausts one, and an exhausted strategy never runs again), so derive them instead of tracking them in parallel. - Drop the unreachable 'failed without lastError' branch: every attempt runs at least one download, so a failed download always has an error. - Progress listeners are local subscriptions released in finally (also on an unexpected throw) instead of sharedState.progressHandlers entries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The four promise-returning bindings (applyDeltaFromSource, cleanupOldEntries, sha256HexFileAsync, sha256HexRawFdAsync) each repeated ~70 lines of create-promise / create-async-work / queue / settle / free. QueueWork<W>(env, name, data, execute, settle) now owns that lifecycle; every failure path (promise creation, work creation, queueing, aborted work) still settles the promise exactly once and frees the call. NewStateResult takes a StateResultExtras struct with defaults instead of five positional extras, and NewString absorbs MakeUtf8String (it now throws on failure like the latter did). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Method names were written twice (host-function macro + registerMethod) and could drift. PUSHY_METHODS(X) now lists (name, argc, SYNC|ASYNC) once and generates both the host functions and the registrations. check-native-spec-parity.js parses that table: a spec method missing from it, a Promise method registered SYNC, a SYNC entry wrapping an async ArkTS method, or a duplicated row all still fail the check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
scripts/lib/cpp-test-common.sh holds CORE_WARNINGS, the SANITIZE=1 flags and the libFuzzer probe that test-patch-core.sh and test-update-flow-core.sh each carried a copy of. The probe also tries Homebrew LLVM (Apple clang has no libFuzzer runtime) and honours FUZZ_CXX. test.yml installs clang with the other build dependencies instead of a second apt-get update. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- checkUpdate/downloadUpdate share surfaceCaughtError for errors that bypassed the client pipeline (user hooks). - switchVersion/switchVersionLater come from one forUpdateHash factory. - restartApp/resetToPackagedBundle are the client's own stable arrow methods, like markSuccess. - downloadAndInstallApk drops the Android check the client already does. - Linking cleanup prefers subscription.remove() and only falls back to removeEventListener on RN < 0.65 (no deprecation warning on 0.65-0.69). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The allowed prefixes were a hand copy of package.json `files`. Build them from `files` instead (globs included) and also flag a packed path that a negated `files` entry should have excluded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- writeFileContent writes through SaveFile.writeFully (now exported, and checks short writes) instead of its own 64KB chunk loop; toArrayBufferSlice and FILE_COPY_BUFFER_SIZE go away. - SaveFile.unlinkIfExists replaces the access-then-unlink pattern in DownloadTask and InstallRecord. - One buildResumeMeta closure builds the resume sidecar for both the initial and the final write. - 'bundle.harmony.js' literals use HARMONY_BUNDLE_FILE_NAME. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
downloadFullUpdate/downloadPatchFromPackage wrapped executeTask in a try/log/rethrow that duplicated DownloadTask's own failure log (downloadPatchFromPpk never had it). Preferences.getAllSync exists since API 10 and the HAR targets API 12, so reset calls it directly; only the API 14 flushSync keeps its structural probe. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- i18n: getLocale/addTranslations and the I18n class export were only used by tests; t() resolves its template in one expression; the stale usage comment is gone. Tests use the real singleton instead. - locales: checking_update / download_progress were only referenced by the i18n test and that comment. - telemetry ServerReportPayload, type.ts Up-to-date/UpdateAvailable/ Expired CheckResult variants (not exported from index), core isFirstTimeDebug, the utils joinUrls re-export (tests import it from updateFlowCore) and the defaultContext export. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No caller ever passed it (the client uuid is inside the request body's cInfo). The fingerprint is in-memory only and not part of the flow vectors (unchanged). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
runQueryRequest walked configured and remotely discovered endpoint bases with two copies of the normalize / dedupe / budget / request / validate sequence; one tryBase closure (plus claimAttempt for the shared 8-request budget) now serves both. The three commitSyncResult calls share one commit helper that also maps "reset during the round" to cancelled/reset, and the response cache JSON is built once. harmonySyncQuery.test.ts pins the fallback order, dedupe, cleartext filter, single remote list and request budget (it passes against the previous implementation too). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… uuid The four hilog levels shared one body, now Logger.log(fn, args). getConstants logged a hand-copied subset of its result to keep the install uuid out of hilog; it now logs the whole result without uuid (plus uuidSet), so new constants are not silently left out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
isRecordForVersion (presence) and expectedBundleSha256ForActivation (activation) each spelled out the legacy-empty and schema/version rules. Activation now reuses isRecordForVersion and only adds the mandatory bundle digest (#655) on top, so the two cannot drift. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
restartApp is API 12, the HAR's minimum (compatibleSdkVersion 12), so the structural typeof probe is dead. A throwing restartApp still falls back to startAbility/terminateSelf exactly as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
src/oldarch now only holds a hand-written NativePushySpec mirroring the codegen spec (name, constants hook, abstract methods), so UpdateModule in src/main absorbs UpdateModuleImpl and extends NativePushySpec under either architecture. Every spec method carries @ReactMethod for the old bridge, which reads the annotations from the module class itself. The unused UpdateModuleImpl(ReactApplicationContext) constructor goes with it; removeListeners takes the codegen's double on the old bridge too. scripts/check-native-spec-parity.js still reads the deleted per-arch UpdateModule files and needs to follow. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…izer pushy.cpp exports configsEquivalent / runnableConfig / normalizeConfiguration over cpp/update_flow_core/native_config.h. UpdateContext's ArkTS canonicalizer, SyncCoordinator's hand-rolled gate and PushyConfiguration's validator (with the default addresses and POLICY_NEXT_LAUNCH) are gone; the public PushyConfiguration type stays. The bun tests no longer evaluate those ArkTS copies: the C1 vectors run against the JS client only (the core replays them in C++), the normalization cases moved to native-config-vectors.json, and the evaluated Harmony modules get a NativePatchCore stand-in whose gate is checked against the same vectors. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
flowjson::Parse gains a ParseLimits overload (input bytes, node count) for documents bounded by something other than a checkUpdate response; the patch core's __diff.json parser needs its 16 MB manifest cap. Objects with more than 16 members are deduplicated through a hash index while parsing instead of Value::Set's linear scan, so a manifest's copies map of thousands of members no longer parses in quadratic time. Duplicate keys keep the first position and the last value as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ry point cpp/patch_core/patch_manifest.h: ParsePatchManifest(text, options) is the single parser of a patch archive's manifest, taking the shape pushy-go and the CLI emit (copies to->from, copiesCrc per target, deletes object, hbcTransform per bundle patch entry) under these rules, where the three platform parsers used to differ: - deletes: an object's keys or an array of strings (Android ignored arrays); any other type, or a non-string element, is an error; - copiesCrc: keyed by copy target, an integer in [0, 2^32) or null. Android keyed it by source with the last target winning; now two targets of one source with different CRCs fail the manifest, and a string/fractional/oversized/boolean CRC fails instead of being coerced or skipping the content check; - types: copies must be an object of strings (Harmony coerced with String(), Android threw, iOS rejected); a null or empty source means the target's own path on every platform (Harmony ppk kept ""); - copy_source_prefix (HarmonyOS rawfiles) is stripped only at the start (Harmony replaced its first occurrence anywhere); - hbcTransform and its bundle entry must be objects when present; the platforms silently dropped other types and applied the patch untransformed. Every path still goes through ValidateManifest. The text is capped at kMaxManifestBytes and 4 * kMaxEntries values. PatchManifest now carries bundle_hbc_transform_meta (moved from FileSourcePatchOptions). archive_patch gains PlanArchivePatch (entry check, parse, merge plan) and ApplyArchivePatch (plan + apply; with host_copies_resources it only rebuilds the bundle and hands the copies back as CopyGroups, which now carry the source's CRC). BuildCopyGroups is linear in the number of copies. The test replays tests/patch-manifest-vectors.json (server shapes, the drift cases, hostile paths) and covers the caps, the plan and both apply modes, including a transform-domain patch driven by the manifest; FUZZ=1 adds a libFuzzer target for the parser. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DownloadTask no longer parses the manifest: extractPatchArchive keeps the raw __diff.json bytes and both patch tasks call one native, applyArchiveDelta (JNI over archive_patch::ApplyArchivePatch), which checks the entries, parses and validates the manifest, rebuilds the bundle and, for ppk patches, copies/deletes/merges. The APK task passes hostCopiesResources and gets back the copy groups, now with the CRC each source must match, for BundledResourceCopier. The manifest crosses JNI as byte[] rather than a jstring, so paths with supplementary characters are no longer re-encoded as modified UTF-8. appendManifestEntries, the buildArchivePlan/buildCopyGroups/ applyDeltaFromSource natives, ArchivePatchPlanResult and buildCopyList's canonical-path check are gone: ValidateManifest rejects every lexical escape (absolute, '..', '.', empty segment, backslash, control bytes), and the staging directory holds only what SafeZipFile extracted, which never creates symlinks. SafeZipFile's own entry checks stay, since zip entry names never reach the C++ validation. Behaviour changes, from the shared rules: copiesCrc is read per target (conflicting CRCs for one source fail the patch instead of the last one winning), a deletes array is honoured, and type errors are patch failures. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
applyDeltaForHash reads the manifest (size-capped as before) and calls archive_patch::ApplyArchivePatch with it, so PushyPatchManifestFromJson, the NSJSONSerialization round trip and the hbcTransform extraction are gone. The rules are the shared ones: a CRC must be an integer in [0, 2^32) (true, 1.5 or 2^32 are no longer read as 1, 1 or 0) and non-object hbcTransform metadata fails the patch instead of being skipped. A missing manifest is now reported as "diff.json not found". Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pushy.cpp exports applyArchiveDelta (async, over archive_patch::ApplyArchivePatch) in place of buildArchivePlan, buildCopyGroups and applyDeltaFromSource; the manifest is parsed on the worker thread with the rest of the patch. DownloadTask reads the manifest text and passes it with the entry names: the ppk task lets the core copy, delete and merge, the package task passes hostCopiesResources and the "resources/rawfile/" prefix and copies the returned groups, each with the CRC its source must match. parseManifestToArrays and its bun tests are gone; the cases moved to cpp/patch_core/tests/patch-manifest-vectors.json. Behaviour changes, from the shared rules: the rawfile prefix is stripped only at the start of a source, an empty source means the target's path in ppk patches too, and values of the wrong type fail the patch instead of being coerced with String() or ignored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
No binding assembles a manifest from parallel copy lists any more; every platform hands __diff.json to ApplyArchivePatch. CODE_AUDIT 3.14 records the manifest parser as sunk into the shared core. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ord.cpp
cpp/patch_core/install_record.h was a header of constants with the logic
kept three times (InstallRecord.java, RCTPushyInstallRecord.mm,
InstallRecord.ts). It now carries the whole contract on POSIX IO and
digest::Sha256File:
- Parse / Read / Names / HasRecordFile / RecordedBundleSha256;
- Build (refuses an empty bundleSha256) and Write: temp file + fsync,
rename over the record, directory fsync (best-effort). Android wrote in
place with fsyncs, iOS atomically without fsync, Harmony in place with a
file fsync only;
- Seal (hash the staged bundle unless the digest is known, build, write;
tells a file-system failure from a bundle failure) and Promote (rename
the staging directory, fsync the versions root, which only Android did);
- IsComplete (bundle is a regular file + legacy marker or a schema-1
record naming the version) for launch/dedup;
- CheckActivation, the switchVersion gate: bundle present; no record file
is grandfathered only for the current/last version; the zero-byte
legacy marker passes; otherwise schema 1, versionHash and a non-empty
bundleSha256 matching the bundle, which must not change while hashed
(iOS's stamp check, now everywhere). Unchanged re-checks cheaply that
the directory still passes on the same basis (iOS's under-lock check).
Where the platforms disagreed, the stricter reading wins: only a zero-byte
file is the legacy marker (all three trusted "{}" too), "schema" must be
the number 1 (Android took "1" and 1.5, iOS true and 1.5), versionHash and
bundleSha256 must be strings (Android coerced numbers), the bundle must be
a regular file, and a grandfathered version still needs its bundle (iOS
skipped that check).
tests/install-record-vectors.json holds 31 record shapes with what each
means for IsComplete, CheckActivation and RecordedBundleSha256; the C++
test replays them and covers the size cap, Build, atomic Write, Seal,
Promote and Unchanged. check-constant-parity.js learns `only`, so the
mirrors are held to the constants they still use.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
InstallRecord.java keeps the paths (bundleFile, stagingDirectoryFor) and wraps five natives (JNI over cpp/patch_core/install_record.h, in cpp/android_jni/install_record_jni.cpp): isInstalled, verifyInstalled (CheckActivation, now taking the grandfathering decision), seal, promote and recordedBundleSha256. build/write/read/isComplete/verifyForActivation/ syncDirectory and the org.json parsing are gone. - DownloadTask.promoteStaging seals the staging directory (reusing the digest taken while extracting a full package) and promotes it; the record is now written atomically (temp + rename) and the directory fsync no longer needs API 26. - verifySwitchTarget hands current/last to the core, which grandfathers only a directory without a record, as before; a markerless directory outside current/last now fails with "is incomplete." like iOS/Harmony. - The bundle hash moves from MessageDigest to the core's SHA-256 on the same threads as before (state serial thread, sync coordinator, download tasks); nothing on the UI thread hashes. Launch-time callers only stat and read the record. InstallRecordTest keeps the path helpers; the record logic is covered by the C++ vectors (the JVM cannot load the library). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
install_record.cpp reports a delta::Status but needs nothing else of patch_core.cpp; with the two factories in the header it links on its own, which lets the iOS purge-restore harness compile the real record core without hpatch, HDiffPatch and lzma. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RCTPushyInstallRecord.mm keeps the paths and becomes a thin layer over cpp/patch_core/install_record.h: PushyHasCompletedVersionAtPath is IsComplete, PushyRecordedBundleSha256 (constants, crash attribution) is RecordedBundleSha256, and PushyPromoteStagingDir seals the staging directory (Seal: bundle failures stay PATCH_FAILED, a record that cannot be written is FILE_OPERATION_FAILED instead of a raw Cocoa error) and promotes it (Promote, which now also fsyncs the record and the versions root; NSDataWritingAtomic did neither). switchVersion:error: calls CheckActivation outside the state lock with the current/last decision taken under it, and re-checks with Unchanged under the lock (same grandfathering, record text and bundle stamp) before committing. PushyReadInstallRecord, PushyInstallRecordMatches, PushyHasInstallRecordFile, PushyStampFile, PushyFileStamp and PushyVerifyInstallForActivation are gone. A grandfathered version now needs its bundle like on Android/Harmony. The purge-restore harness compiles the extractor's copy of install_record.cpp next to the extracted switchVersion:error:, so install_record_activation and switch_version_records exercise the shared core; the trust-digestless and grandfather-recorded mutants now alter that copy and are still caught by their named assertions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The struct moved into the shared record core (install_record::FileStamp). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pushy.cpp exports the record core (cpp/patch_core/install_record.h): isInstallComplete and recordedBundleSha256 synchronously (a stat and a read of the record, as before), sealInstallRecord, promoteStaging and checkActivation as promises on libuv workers, so neither the bundle hash nor the fsyncs run on the JS thread. InstallRecord.ts keeps the paths and the reserved-entry rule and maps rejections to the stable codes (install steps PATCH_FAILED, activation SWITCH_VERSION_FAILED). parse/read/build/write, the record matching and assertActivatable are gone; UpdateContext.verifyActivation hands current/last to the core, DownloadTask seals and promotes. The record is now written atomically with a directory fsync (it was unlinked and rewritten in place) and the versions root is fsynced after the rename. The bun tests now cover only the binding: paths, the arguments handed to the NAPI module, the error-code mapping, and that NativePatchCore.ts declares exactly the functions pushy.cpp exports. The record logic is the C++ vectors'. check-constant-parity holds both mirrors to kStagingSuffix alone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sponse
cpp/update_flow_core/native_round.{h,cpp} takes over the pure parts the
Android, iOS and Harmony orchestrators each carried around the shared
decision layer:
- PrepareRound(config, env, sample): the checkUpdate request body (identity,
cInfo and input assembled from the persisted config and the launch env,
diffV from this core) plus the first endpoint-query state.
- NextQueryStep(state, text): the sequential endpoint fallback — configured
endpoints in sampled order, then the first queryUrls list that parses,
https-only for remote bases, the 8-request budget and the response schema
gate. The request path now lives here, encoded.
- HandleRoundResponse(...): the decision with attempts planned for the device
(no diff without a running version, no cleartext URL under an https-only
config), the hashInfo to persist (incl. forceBootRescue) and the
response-cache text.
tests/native-round-vectors.json pins it and records, per case, where the
three platforms disagreed: non-string packageVersion/rnu/rn overrides,
absent rnu/rn on iOS, Android's optBoolean forceBoot marker and lenient
remote-list JSON, non-string endpoint/queryUrls handling, and Harmony's
failures on non-array endpoints or a non-string afterDownload. The core
takes the JS-equivalent or strictest behaviour. The fuzz target walks the
new entry points.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…core SyncCoordinator hands the config text and a launch env to FlowBridge.prepareRound / nextQueryStep / handleResponse (JNI over cpp/update_flow_core/native_round.h) instead of assembling identity, cInfo and the request input, walking the endpoints (QueryRun, isHttpsOnly), building hashInfo and the response-cache JSON, and filtering attempts itself. The buildRequestBody / orderEndpointCandidates / isValidResponse natives are gone; the config gate returns just the rejection (configRejection), since the core reads the appKey itself. HttpUtils loses QUERY_PATH, stripTrailingSlashes and isCleartextUrl; the request path lives encoded in the core. Behaviour aligned with iOS/Harmony/JS: a truthy non-boolean forceBoot (1, "false") now marks forceBootRescue like the activation it already caused; non-string packageVersion/rnu/rn are no longer coerced to strings; non-string endpoints and queryUrls entries no longer spend a request; a remote endpoint list must be strict JSON; a config with duplicate keys is no longer rejected by org.json after the shared gate accepted it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RCTPushyOrchestrator passes the config text and a launch env to flowcore::PrepareRound / NextQueryStep / HandleRoundResponse (cpp/update_flow_core/native_round.h) instead of assembling identity, cInfo and the request input, walking the endpoints with its own tried-set and budget, projecting the version info and filtering attempts. commitRoundWithGeneration takes the core's response-cache text (cache:) in place of the four fields it serialized itself; nil still marks the crash handler's activation-only commit. PushyLoadRunnableConfig only hands back the config text, RCTPushyQueryPath and the https/endpoint helpers are gone. Behaviour aligned with Android/Harmony/JS: absent rnu/rn are sent as "" (cInfo used to omit them) and non-string ones as "", like the core's C1 defaults. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…core SyncCoordinator passes the config text and a launch env to the new NAPI prepareRound / nextQueryStep / handleResponse (cpp/update_flow_core/ native_round.h) instead of assembling identity, cInfo and the request input, walking the endpoints itself, projecting hashInfo, building the response-cache JSON and filtering attempts. The buildRequestBody / orderEndpointCandidates / isValidResponse exports and Texts.QUERY_PATH are gone; the request path now lives only in the core. harmonySyncQuery checks the glue follows the core's steps (the walk's policy moved to native-round-vectors.json); harmonySyncAttempts and the host-API harness use the new signatures, and nativeTextEncoding expects the request path in cpp/ only. Behaviour aligned with Android/iOS: non-array endpoints still try the list discovery (the round used to stop), a non-string endpoint is skipped (it used to throw and fail the round), a non-string afterDownload never activates (it used to fail the round as invalid_response), non-string queryUrls entries no longer spend a request, and non-string packageVersion/rn overrides are ignored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ed core Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
….cpp cpp/patch_core/archive_limits.h was a header of constants with the rules kept three times (SafeZipFile.inspect, PushyUnzipGuard, ArchiveLimits.ts). It now carries them: - EntryAccumulator(archiveBytes).Accept(size, compressed, symlink, name): entry count, reserved name, symlink, per-entry cap, running total and whole-archive ratio, per-entry ratio; AcceptExtracted for an entry already on disk whose compressed size was never seen (no per-entry ratio); - CheckEntryCount, CheckUncompressedSize(archiveBytes, uncompressedBytes) and IsReservedEntryName (install_record::kReservedPrefix, new); - CheckExtractedDirectory: an iterative lstat walk of a freshly unpacked tree through the accumulator (Harmony, whose zlib exposes no entries); - PlanUnknownLengthFreeSpaceProbe: the chunked-download disk probe plan all three platforms implemented identically. Where the platforms disagreed, the strictest reading wins: - the ratio is size > compressed * 100 (Android/iOS divided, so up to 101:1 minus one byte passed), and a zero compressed size past the 1 MiB threshold is an unbounded ratio (Android/iOS skipped the check); - the whole-archive ratio (Harmony only) now applies on every platform; - a reserved `.pushy-` prefix is refused in any path component (Android checked the base name, iOS the last component of each unpacked path); - symlinks are refused everywhere (iOS only; Harmony's walk followed them); - an entry of unknown (negative) size is refused (Android skipped it); - a non-positive archive size bounds nothing (Harmony skipped the ratio). tests/archive-limits-vectors.json pins the accumulator, the whole-archive check, the probe plan and the reserved names (ported from the Harmony bun cases and SafeZipFileTest, with `was` notes); the C++ test also walks real trees (reserved directory, symlink, 20000/20001 entries). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hive_limits PushyUnzipGuard feeds an archive_limits::EntryAccumulator (sized by the archive file, for the whole-archive ratio) from the should-unzip callback and checks the entry count up front with CheckEntryCount; its own size, total and ratio arithmetic is gone. The reserved-name check in the did-unzip callback now covers every component of the path below the destination (IsReservedEntryName), not just the last one; an unpacked path that is not below the destination is a violation. The downloader's unknown-length disk probe is PlanUnknownLengthFreeSpaceProbe. Behaviour aligned with the shared rules: ratio as size > compressed * 100 (was integer division), a zero compressed size past 1 MiB is refused, the whole-archive ratio applies, a `.pushy-` directory component is refused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…over JNI SafeZipFile.inspect(archiveBytes) collects the central directory once and hands names, declared and compressed sizes to ArchiveLimits.checkEntries (one coarse JNI call per archive, cpp/android_jni/archive_limits_jni.cpp, over archive_limits::EntryAccumulator), which returns the total for the free-space check or throws the first violation. The Inspection class, the Java count/size/ratio arithmetic and isReservedEntryName are gone; the reserved-name rule now comes from the core during inspect, which both extraction paths run before any entry is written (the iterator keeps the traversal check, unzipToPath the canonical-path check). The download loop asks ArchiveLimits.planFreeSpaceProbe (JNI over PlanUnknownLengthFreeSpaceProbe) before each chunk. Behaviour aligned with the shared rules: ratio as size > compressed * 100, a zero compressed size past 1 MiB is refused, the whole-archive ratio applies, a `.pushy-` directory component is refused, an entry of unknown size is refused instead of skipped. ArchiveLimits.java keeps the five caps it still compares against itself; check-constant-parity holds it to exactly those. SafeZipFileTest keeps the traversal and extraction cases; the accounting cases moved to cpp/patch_core/tests/archive-limits-vectors.json (the JVM cannot load the library). The JNI registration test accepts a throws clause on a native. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… NAPI pushy.cpp exports the extraction accounting (cpp/patch_core/archive_limits.h): checkUncompressedSize (whole-archive expansion and ratio, synchronous, returns the reason), planFreeSpaceProbe (the unknown-length download probe plan, per chunk) and checkExtractedDirectory (the post-unpack walk, a promise on a libuv worker). zlib exposes no entries, so the accumulator runs over the unpacked tree there. ArchiveLimits.ts keeps checkOriginalSize (the getOriginalSize gate, Harmony only) and ensureFreeSpace, and maps refusals to PATCH_FAILED. planUnknownLengthFreeSpaceProbe and measureExtractedDirectory are gone, and so are InstallRecord.ts's RESERVED_ENTRY_PREFIX / isReservedEntryName. The walk no longer runs on the JS thread and uses lstat: a symlink in the unpacked tree is refused instead of followed. ArchiveLimits.ts mirrors only the caps it still compares against (MAX_ARCHIVE_BYTES, MAX_MANIFEST_BYTES, FREE_DISK_MARGIN_BYTES); check-constant-parity holds it to exactly those. The bun tests keep the getOriginalSize gate and cover only the binding (arguments, error code); the probe and ratio cases are the C++ vectors'. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
NDK 28.2.13676358 (scripts/build-android-so.sh pin). Picks up the native_config, archive delta, install record, native round and archive limit bindings added on this branch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Important Review skippedToo many files! This PR contains 202 files, which is 52 over the limit of 150. To get a review, reduce the PR to 150 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
⛔ Files ignored due to path filters (4)
📒 Files selected for processing (202)
You can disable this status message by setting the
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 |
- Android: skip the per-chunk free-space plan (a JNI call) when the download length is known; nothing is ever probed then. - iOS: an undefined round decision fails the round as invalid_response, as on Android, instead of committing as 'none'. - archive_limits: CheckExtractedDirectory walks directories without counting them, matching iOS/Android which count only declared zip entries. Reserved directory names are still rejected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ing forceBoot The relaunch right after the 'no directive' check starts its own delayed round (5s after launch). The test flipped forceBoot as soon as the base label showed, so that in-flight round could read the flipped server and activate one launch early (seen on CI: request at +5s, flip at +4s). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The beforeAll only uninstalls, reinstalls and launches the app; on slow macOS runners simctl alone took 5-9 minutes and the 300s testTimeout failed the suite before any step ran. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sunnylqm
added a commit
that referenced
this pull request
Oct 12, 2026
Bump package.json and harmony/pushy/oh-package.json5 together. Ships the cross-layer cleanup (#657): the native round, patch-manifest parsing, install records and archive limits now live once in the shared C++ core, plus the unreleased #654/#655 fixes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
一轮全仓清理和三端收敛，分两部分：
cpp/里保留一份，三端只剩薄绑定。顺带修了几处三端行为不一致的问题，详见下文。
规模
src/（不含测试）android/srcios/harmony/cpp/（不含 tests）cpp/**/tests（新增向量和用例）docs/平台代码合计净减约 3500 行。新增的 C++ 大部分是共享内核，替代了三端各自的实现，并且有测试向量和 fuzz 覆盖。
一、按层清理
fail()。nativeMethod。error.ts的错误码只写一份。helpers.ts/nativeSources.ts），mock 一律透传真实模块；provider 渲染测试改用真实 client。UpdateModule：oldarch 只留约 70 行手写的NativePushySpec。Threads工具，合并UiThreadRunner。RCTPushy.mm（约 2140 行）按职责拆成 State / InstallRecord / Installer / Launch 四个文件。RCTPushyInstaller自己持有文件队列，+engine已删除。dispatch_once改为函数内 static。+rollback、RN 0.62 以前的 reload 兜底分支，以及 podspec 里失效的分支和路径。isFirstTime/clearFirstTime，以及提交进仓库的生成文件BuildProfile.ets和OAT.xml。QueueWork；TurboModule 方法表改为 X-macro，只写一遍。cpp/android_jni/，hpatch挪到cpp/hpatch/。android/lib/*/librnpushy.so（NDK 28.2.13676358）。e2e-common和publish-npm，e2e workflow 去重。check-constant-parity.js移到scripts/，新增 Java 和 JS 常量的比对，并接入bun lint。check-packlist改为从files推导，C++ 测试脚本抽出公共部分。NATIVE_CHECK_FOLLOWUPS.md（529 → 44 行）、CODE_AUDIT.md和BUNDLEHASH_MIGRATION.md，修正悬空引用。二、下沉到共享 C++
update_flow_core/native_configdisabled一律按 JS 真值判断（Android 原来只认true/"true"）；appKey 按 JS 的 trim 规则；配置 JSON 后面跟多余内容一律拒绝__diff.json清单解析patch_core/patch_manifest，三端共用一个applyArchiveDelta入口deletes接受对象或数组；copiesCrc按 copy 目标取值并严格校验类型；类型错误一律报错，不再强转；hbcTransform不是对象时报错，原来会被静默忽略patch_core/install_recordupdate_flow_core/native_roundpatch_core/archive_limits每处差异都在测试向量里记下了改动前各端的实际输出。
修复的问题
PATCH_FAILED后不再逐个镜像重下重打，直接换下一个策略，和 JS 一致。Object.hasOwn改为hasOwnProperty.call：iOS 15.4 以前的 JSC 没有这个方法，原生缓存比对会抛错并静默判为不命中。NewString，非 BMP 字符不再出错。patch_core.cpp改为用相对路径引用hpatch.h，use_frameworks!构建不再解析失败。验证
bun lint（biome、tsc、鸿蒙类型、spec 一致性、常量一致性）bun test src/__tests__（含--randomize）test:patch-core/test:flow-core（含SANITIZE=1），新增 fuzz 目标SANITIZE=1 VERIFY_REGRESSIONS=1）check-packlist（普通和--release）、verify-android-so、check-binary-strings<init>、字段都保留；混淆包装机后完整走通一次更新use_frameworks!dynamic 构建没有做的（需要另行决定）
lint.yml并入test.yml：required check 的名字会变，要同步改分支保护。domains.json：已部署的老客户端可能还会去 master 拉它。hbc_transform_wire的手写 JSON 解析器：差异测试发现新旧解析器在 6 类边界输入上行为不同。ensureFreeSpace在可用空间为 0 时的处理三端仍不一致：iOS 当作真实值，Android 和鸿蒙当作"未知"放行。改成最严格有误报风险。Example/e2etest/ios/Podfile.lock没更新：本机pod install生成的结果带着 worktree 的绝对路径。CI 不用--deployment，不受影响。🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.