fix: Android code push config refreshes on every launch#18
Conversation
The generated CodePushApp copied codepush.yaml from assets to the files dir only when no copy existed, so configuration shipped in an app update was never picked up. The copy now runs on every launch, and re-running 'fcp codepush init' upgrades a previously generated CodePushApp.kt in place.
Replaces the engineering post-mortem entries on main with the user-facing versions already published with the 0.19.15 rc series. Aligns main's changelog with what pub.dev ships.
fcp codepush init now writes the local signing public key into assets/codepush.yaml as a block scalar, and fcp codepush keys register injects it into an existing project's config — parity with the FLTCodePushPublicKey Info.plist entry on iOS. With a key in the config, devices require a valid patch signature before loading a patch.
Code ReviewBug: double file read in the upgrade path
Bug:
|
… surfaced copy failures - Read CodePushApp.kt once in the upgrade branch (was read twice). - _publicKeyYamlBlock uses F.homeDir() so the key is found on Windows. - The generated copy block logs failures to logcat instead of swallowing them silently (fully qualified android.util.Log, so the in-place upgrade needs no new import). - CHANGELOG notes the manual-integration migration step.
|
Addressed in ab2387a:
|
Code ReviewOverall: The config-refresh fix is correct and well-motivated — overwriting on every launch is the right call for a tiny YAML file that changes with each app update. The in-place upgrade path for existing A few issues worth addressing before merge: Bugs / correctness1. Silent upgrade no-op gives no user feedback 2. Unguarded file I/O in 3. No empty-PEM guard before writing in Security4. Mutable action tags in CI workflow 5. MinorThe on-every-launch overwrite of |
Code Review — fix: Android code push config refreshes on every launchOverviewThe core fix (copy Issues🔴 Key rotation silently broken in
|
- keys register now replaces an existing public_key block instead of bailing out, so key rotation reaches devices; guards empty PEM and file I/O failures with a non-fatal warning. - init preserves a previously injected public_key when no local key exists (CI, different machine), instead of silently dropping it and disabling device-side verification. - init warns when CodePushApp.kt cannot be upgraded automatically. - _publicKeyYamlBlock tolerates an unreadable pem file.
|
Round-2 findings addressed in 64274a6:
Two deliberate non-changes:
|
|
Code Review summary — see inline comments for specifics. What this PR does: fixes Android code-push config refresh (CodePushApp.kt now copies codepush.yaml on every launch, not only when missing), adds public-key embedding into the Android YAML on init and keys register, and rewrites the CHANGELOG. Correctness issues:
Security: id-token: write in claude-review.yml is not needed for CLAUDE_CODE_OAUTH_TOKEN auth — drop it for least privilege. (inline comment on the file) What is correct: the copy-on-every-launch fix is the right approach; in-place upgrade strategy is sensible; manual-integration snippet matches the generated code. |
|
Round-3 triage under the new review policy (only Critical blocks merge):
Merging. |
What
Two changes:
1. Android config refresh fix
The Android setup generated by
fcp codepush initcopiedcodepush.yamlfrom the APK assets into the app files directory only when no copy existed yet. Configuration shipped in an app update (e.g. a new release version) was therefore never picked up — the app kept running with the configuration from its first install.CodePushApp.ktnow performs the copy on every launch, overwriting the previous copy. The file is tiny, so the cost is negligible.fcp codepush initin an existing project upgrades a previously generatedCodePushApp.ktin place (exact-match replacement, so user-modified files are left untouched).Applicationclass are updated to match.2. CHANGELOG cleanup
Replaces the verbose engineering-notes entries for 0.19.11–0.19.14 on main with the concise user-facing versions already shipped with the 0.19.15 rc series on pub.dev, and records the rc history so main's changelog matches the published package.
Notes
dart analyzeclean on the touched file.