feat: disableNativeCheck opt-out, 10.51.0 docs and native check e2e - #623
Conversation
The cold-start check shipped as unconditional behavior: every integrator got one extra background request per launch with no way to decline. That contradicts how the rest of this SDK treats integrator intent, and the plumbing to honor a decline already existed — all three orchestrators already bail on a `disabled` config before any IO. ClientOptions gains disableNativeCheck, documented as the trade it is: one fewer request per cold start in exchange for giving up the recovery path a bricked device depends on. It stays orthogonal to checkStrategy, which governs activation authority rather than whether the check runs. Both READMEs describe the check itself — that it never blocks startup, when it may activate a version, how the console's force-boot rescue works and how to turn the whole thing off — and the version moves to 10.51.0, which is also the floor the consoles gate their force-boot entry on. The feature also shipped with no e2e coverage of its own, which is why it took breaking someone else's suite to surface a contract conflict. The new suite covers the capability minus the one step Detox cannot stage (it cannot attach to an app whose JS never boots): the app performs no JS check at all, yet a force-boot version still installs and activates — an outcome only the native orchestrator can produce. A second case pins the other direction: without the directive, and with automatic checks off, the check may download but must never activate on its own. The mock server grows a /control/force-boot endpoint so the directive is opt-in per test and the other suites keep full control over activation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe client can disable native cold-start checks through ChangesNative cold-start update checks
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant DetoxSuite
participant LocalE2EServer
participant App
DetoxSuite->>LocalE2EServer: Set force-boot state
DetoxSuite->>App: Launch and reset packaged bundle
App->>LocalE2EServer: Perform native update check
LocalE2EServer-->>App: Return available update with forceBoot
App-->>DetoxSuite: Report activated version and hash
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
The first version spent about nine app launches across two tests: a beforeEach that reset to the packaged bundle and relaunched, then two launches per test. On the iOS simulator every launch is real wall clock, and this suite shares a 40-minute job budget whose green baseline is already ~16 minutes. A fresh install already sits on the packaged bundle, so the reset round-trip buys nothing, and both directions of the assertion can share one install: settle once with no directive and confirm nothing activated, then flip the directive on and confirm the version installs. Four launches instead of nine, with both assertions intact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The iOS e2e job had no room left: its last green run took 36 of its 40 allotted minutes, and the native check suite needs about three more on a healthy runner (186s measured in a retry). Every run since has died at the 40-minute wall — not because the suite fails (it passes on both Android matrices and passed on iOS in that retry) but because the job can no longer hold everything. The suite moves to e2e/native/ with its own runner config, mirroring how the harmony and smoke suites are already separated, and the iOS job gains a matrix so core and native run as independent legs with independent budgets and fail-fast off — a timeout in one no longer hides the other's verdict. New test files still land in core automatically; only this directory is carved out. Android keeps both suites in one job by invoking the second config after the first: that job finishes in about nine of its thirty minutes, so it has the budget iOS lacks and splitting it would only cost another emulator. Cost note: the built-app cache key covers package.json and ios/**, so a run that changes those (like this one) now builds twice in parallel instead of once. That is the price of the wall-clock win; runs that only touch JS restore the cache in both legs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.github/workflows/e2e_android.yml:
- Around line 144-149: In both Android job sites at
.github/workflows/e2e_android.yml lines 144-149 and 381-384, update every Detox
command to change into Example/e2etest within that command before execution; do
not rely on the standalone cd line persisting between script shells.
🪄 Autofix
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 Plus
Run ID: ef5f2fa3-ed12-4989-9124-ebc06ab8aff3
📒 Files selected for processing (5)
.github/workflows/e2e_android.yml.github/workflows/e2e_ios.ymlExample/e2etest/e2e/jest.config.jsExample/e2etest/e2e/native/jest.config.jsExample/e2etest/e2e/native/native-check.test.ts
android-emulator-runner runs each line of a multi-line script in its own shell, so the standalone `cd` I added never reached the second detox invocation: it ran from the repository root and died with "Could not resolve jest package from the current working directory", taking both Android release jobs down. CodeRabbit flagged exactly this on the diff and CI confirmed it minutes later. Each command now carries its own cd. The iOS job also gains an explicit name, since a matrix with two keys was composing checks called "e2e-ios (native, --config e2e/native/jest.config.js)". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two failures on the same run, both self-inflicted by our own caps rather than by anything under test: - The native leg died in "Prepare local update artifacts" with spawnSync ETIMEDOUT. runPushy capped every CLI call at 120s, and a cold Metro bundle on a macOS runner routinely needs longer — the banner printed at 07:05:54 and the process was killed at 07:07:52, making progress the whole time. Raised to 300s; it is a stuck-process guard, and the step/job timeouts still bound the happy path. - The core leg was cancelled at the 40-minute wall with its --retries 1 rerun still in flight (build was 1 min from cache, so the test phase alone ate 30). Budgeting less than one full retry makes retries useless. Raised to 60. Neither masks a product defect: bundleHash's 341s (vs ~2s on Android) is a pre-existing iOS runner pathology tracked separately.
承接 #620:把原生冷启动检测从「合入」推到「可发版」。
1. 新增
disableNativeCheck检测目前是无条件行为——每个接入方都多了一次冷启动后台请求,且无法拒绝。这与本 SDK 对待接入方意图的一贯方式相悖,而且支持拒绝的管道本来就是通的:三端编排器早已在读配置里的
disabled并在任何 IO 之前返回,缺的只是公开选项。文档里把它写成一笔明账:少一次冷启动请求,代价是放弃救砖能力(被坏热更卡死的设备无法自愈),仅在这次请求本身构成问题时(流量/耗电预算、隐私清单申报、需用户同意后才可联网)才建议关闭。
它与
checkStrategy正交:后者管激活权(为null时原生只下载不激活,仅 forceBoot 可激活),前者管检测本身跑不跑。2. 10.51.0 + README
版本号对齐两个控制台 forceBoot 入口的门槛(
rnu >= 10.51.0)。README / README-CN 各加一条「优势」与一节《原生冷启动检测》:不阻塞启动、什么条件下才会自动激活、控制台强制启动如何救砖(含"本地回滚保护优先")、以及如何关闭。3. 原生检测的 e2e 用例
这个功能此前没有自己的 e2e,这正是它在 #620 里要靠撞坏别人的用例才暴露出契约冲突的原因。
真·坏 bundle 场景做不了——Detox 无法连上 JS 起不来的 app——所以本 suite 覆盖的是去掉那一步之后的同一能力:应用全程不做任何 JS 检查(
checkStrategy: null且不点检查按钮),版本却装上并生效了,这个结果只有原生编排器能产生。激活由服务端按版本的 forceBoot 指令驱动,与真实救砖的触发方式一致。第二条用例钉反向:没有指令、且自动检查已关时,原生可以下载但绝不能自行激活。
mock server 新增
/control/force-boot控制端点,默认关闭 —— 其它 suite 对激活时机的完全控制不受影响。一处主动撤回
原计划顺带修第四轮遗留的「配置变无效时不要覆盖上一份好配置」,实现后撞红了一条已有测试。复盘后是我判断错了:
server.main为空时 JS 侧自身也会NO_ENDPOINTS抛错,那不是"瞬时不完整"而是真的失配;相反保留旧配置会让已注销/已解除配置的应用继续用旧 appKey 后台检查。维护者的语义更对,已撤回,只保留 opt-out。验证
本地:179 JS 测试、biome/tsc/DevEco strict、e2e 工程 tsc。本 PR 的重点是让 CI 真跑一次新增的 e2e(本地无法跑 detox)。
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit
New Features
Documentation
Tests
Release