diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml new file mode 100644 index 0000000..c07258e --- /dev/null +++ b/.github/workflows/claude-review.yml @@ -0,0 +1,40 @@ +name: Claude Code Review + +on: + pull_request: + types: [opened, synchronize, ready_for_review, reopened] + +jobs: + claude-review: + # Repo secrets are not available to PRs from forks; skip them. + if: github.event.pull_request.head.repo.full_name == github.repository + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write + id-token: write + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 1 + + - uses: anthropics/claude-code-action@v1 + with: + claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} + prompt: | + REPO: ${{ github.repository }} + PR NUMBER: ${{ github.event.pull_request.number }} + + Review this pull request as a senior Dart/Flutter engineer. Focus on: + - Correctness bugs and edge cases in the changed code + - API misuse and error-handling gaps + - Security issues (credential handling, injection, unsafe file I/O) + - Backwards compatibility for existing users of this package + + Use `gh pr comment` for overall feedback and + `mcp__github_inline_comment__create_inline_comment` (with confirmed: true) + for line-specific issues. Only post GitHub comments — do not submit + review text as plain messages. Be concise; skip pure style nits. + claude_args: | + --model claude-sonnet-4-6 + --allowedTools "mcp__github_inline_comment__create_inline_comment,Bash(gh pr comment:*),Bash(gh pr diff:*),Bash(gh pr view:*)" diff --git a/lib/src/code_push.dart b/lib/src/code_push.dart index 03b888c..36a5c5b 100644 --- a/lib/src/code_push.dart +++ b/lib/src/code_push.dart @@ -12,8 +12,10 @@ import 'package:flutter/services.dart'; import 'models.dart'; /// The platform channel used to communicate with the code push engine. -const MethodChannel _channel = - MethodChannel('flutter/codepush', JSONMethodCodec()); +const MethodChannel _channel = MethodChannel( + 'flutter/codepush', + JSONMethodCodec(), +); /// Channel for the SDK's own native plugin (reads Info.plist, etc.). const MethodChannel _pluginChannel = MethodChannel('flutterplaza_code_push'); @@ -81,12 +83,7 @@ abstract final class CodePush { /// Expected first 4 bytes of every iOS patch payload after /// unwrapping the on-disk header. Opaque format marker. - static const List _iosPayloadHeader = [ - 0x33, - 0x43, - 0x42, - 0x44, - ]; + static const List _iosPayloadHeader = [0x33, 0x43, 0x42, 0x44]; /// Debug status notifier — shows what code push is doing. static final ValueNotifier status = ValueNotifier('init'); @@ -151,6 +148,11 @@ abstract final class CodePush { // so a rollback doesn't get immediately overwritten by a success report. _startLaunchTimer(); + // Report any rollback that was recorded natively before Dart started + // (crash-loop protection runs before main(), so without this the + // server never hears about it). Best-effort, fire-and-forget. + _reportPendingNativeRollback(serverUrl: serverUrl, appId: appId); + // Immediate check is now *after* crash protection so a bad patch // on disk gets a chance to increment the boot counter before we // replace it. @@ -204,7 +206,8 @@ abstract final class CodePush { // (further down) still protects us. final deviceBaselineHash = await _computeBaselineHash(); final deviceBaselineId = await _readBaselineId(); - final url = '$serverUrl/api/v1/updates' + final url = + '$serverUrl/api/v1/updates' '?app_id=$appId' '&version=${Uri.encodeComponent(releaseVersion)}' '&platform=$_platform' @@ -240,20 +243,23 @@ abstract final class CodePush { final rbFile = File('$patchDir/rolled_back_patch'); if (rbFile.existsSync()) { try { - final rb = jsonDecode(rbFile.readAsStringSync()) - as Map; + final rb = + jsonDecode(rbFile.readAsStringSync()) as Map; final rbId = rb['patch_id']?.toString(); final rbHash = rb['patch_hash']?.toString(); final serverPatchId = patchId; final serverPatchHash = data['patch_hash']?.toString(); final idMatch = rbId != null && rbId == serverPatchId; - final hashMatch = rbHash != null && + final hashMatch = + rbHash != null && serverPatchHash != null && rbHash == serverPatchHash; - print('[CP] rollback check: rbId=$rbId serverId=$serverPatchId ' - 'idMatch=$idMatch rbHash=${rbHash?.substring(0, 8)}... ' - 'serverHash=${serverPatchHash?.substring(0, 8)}... ' - 'hashMatch=$hashMatch'); + print( + '[CP] rollback check: rbId=$rbId serverId=$serverPatchId ' + 'idMatch=$idMatch rbHash=${rbHash?.substring(0, 8)}... ' + 'serverHash=${serverPatchHash?.substring(0, 8)}... ' + 'hashMatch=$hashMatch', + ); if (idMatch || hashMatch) { status.value = 'Skipping rolled-back patch $patchId'; print('[CP] SKIPPING rolled-back patch'); @@ -291,7 +297,8 @@ abstract final class CodePush { serverUrl: serverUrl, appId: appId, patchId: patchId, - reason: 'Engine has no code push support (stock Flutter engine ' + reason: + 'Engine has no code push support (stock Flutter engine ' 'or missing flutter/codepush method channel).', expectedFingerprint: expectedEngineFingerprint, actualFingerprint: null, @@ -306,7 +313,8 @@ abstract final class CodePush { if (expectedEngineFingerprint != null && actualEngineFingerprint != 'unknown' && expectedEngineFingerprint != actualEngineFingerprint) { - status.value = 'Incompatible baseline: engine ABI mismatch ' + status.value = + 'Incompatible baseline: engine ABI mismatch ' '($actualEngineFingerprint vs $expectedEngineFingerprint)'; await _reportIncompatibleBaseline( serverUrl: serverUrl, @@ -337,7 +345,8 @@ abstract final class CodePush { serverUrl: serverUrl, appId: appId, patchId: patchId, - reason: 'Baseline hash mismatch (soft gate — proceeding ' + reason: + 'Baseline hash mismatch (soft gate — proceeding ' 'with download; engine ABI check is the hard gate)', expectedFingerprint: expectedBaselineHash, actualFingerprint: actualBaselineHash, @@ -372,9 +381,7 @@ abstract final class CodePush { // if the load crashes the process. if (patchDir != null) { try { - final patchHash = sha256 - .convert(patchBytes) - .toString(); + final patchHash = sha256.convert(patchBytes).toString(); File('$patchDir/patch_info.json').writeAsStringSync( jsonEncode({ 'patch_id': patchId, @@ -417,7 +424,8 @@ abstract final class CodePush { 'HEADER_MISMATCH patchId=$patchId payload=${payload.length}', ); } - status.value = 'Patch format is unexpected — rolling back. ' + status.value = + 'Patch format is unexpected — rolling back. ' 'Upgrade flutter_compile to the latest version and ' 'rebuild the patch.'; await _iosImmediateRollback( @@ -483,7 +491,9 @@ abstract final class CodePush { 'moduleLoaded=$_moduleLoaded', ); } - print('[CP] MODULE LOADED OK — result=$result moduleLoaded=$_moduleLoaded'); + print( + '[CP] MODULE LOADED OK — result=$result moduleLoaded=$_moduleLoaded', + ); return true; } catch (e) { if (patchDir != null) { @@ -588,8 +598,9 @@ abstract final class CodePush { final client = HttpClient(); try { final uri = Uri.parse('$serverUrl/api/v1/telemetry/client-error'); - final req = - await client.postUrl(uri).timeout(const Duration(seconds: 5)); + final req = await client + .postUrl(uri) + .timeout(const Duration(seconds: 5)); req.headers.set('Content-Type', 'application/json'); req.write(jsonEncode(payload)); await req.close().timeout(const Duration(seconds: 5)); @@ -601,6 +612,65 @@ abstract final class CodePush { } } + /// Best-effort: report a rollback that the native side recorded in the + /// patch directory (automatic crash-loop rollbacks happen before any + /// Dart code runs, so they can only be reported after the fact). The + /// marker is cleared once the server has received the report; if the + /// device is offline, we retry on the next launch. + static Future _reportPendingNativeRollback({ + required String serverUrl, + required String appId, + }) async { + try { + final patchDir = await _getPatchDir(); + if (patchDir == null) return; + final marker = File('$patchDir/rollback_info.json'); + if (!marker.existsSync()) return; + + Map info; + try { + info = jsonDecode(marker.readAsStringSync()) as Map; + } catch (_) { + // Unreadable marker — clear it so it can't wedge future launches. + try { + marker.deleteSync(); + } catch (_) {} + return; + } + + final payload = { + 'app_id': appId, + 'kind': 'auto_rollback', + 'reason': info['reason'] ?? 'unknown', + 'platform': _platform, + if (info['patch_version'] != null) + 'patch_version': info['patch_version'], + if (info['boot_count'] != null) 'boot_count': info['boot_count'], + if (info['rolled_back_at'] != null) + 'rolled_back_at': info['rolled_back_at'], + }; + final client = HttpClient(); + try { + final uri = Uri.parse('$serverUrl/api/v1/telemetry/client-error'); + final req = await client + .postUrl(uri) + .timeout(const Duration(seconds: 5)); + req.headers.set('Content-Type', 'application/json'); + req.write(jsonEncode(payload)); + final res = await req.close().timeout(const Duration(seconds: 5)); + if (res.statusCode >= 200 && res.statusCode < 300) { + try { + marker.deleteSync(); + } catch (_) {} + } + } finally { + client.close(force: true); + } + } catch (_) { + // Telemetry is best-effort. Never crash over it. + } + } + /// In-memory cache of the running app's baseline hash. /// Computed lazily on first use and reused for the rest of the /// session, since the AOT snapshot can't change while the app is @@ -684,10 +754,8 @@ abstract final class CodePush { /// Checks the engine for available updates (delegates to Dart side HTTP). static Future checkForUpdate() async { try { - final Map? result = - await _channel.invokeMapMethod( - 'CodePush.checkForUpdate', - ); + final Map? result = await _channel + .invokeMapMethod('CodePush.checkForUpdate'); if (result == null) { throw CodePushException( 'Failed to check for update: no response from engine.', @@ -730,10 +798,8 @@ abstract final class CodePush { /// Returns information about the currently installed patch, or null if /// no patch is active. static Future get currentPatch async { - final Map? result = - await _channel.invokeMapMethod( - 'CodePush.getCurrentPatch', - ); + final Map? result = await _channel + .invokeMapMethod('CodePush.getCurrentPatch'); if (result == null) return null; return PatchInfo( version: result['version'] as String, @@ -760,8 +826,9 @@ abstract final class CodePush { static Future rollback() async { // Try engine-side rollback first (works on Android/desktop). try { - final bool? success = - await _channel.invokeMethod('CodePush.rollback'); + final bool? success = await _channel.invokeMethod( + 'CodePush.rollback', + ); if (success == true) return; } catch (_) {} @@ -792,8 +859,9 @@ abstract final class CodePush { /// /// Throws [CodePushException] if the download or application fails. static Future downloadAndApply() async { - final result = - await _channel.invokeMethod('CodePush.downloadAndApply'); + final result = await _channel.invokeMethod( + 'CodePush.downloadAndApply', + ); if (result != true) { throw CodePushException('Failed to download and apply patch.'); } @@ -831,8 +899,9 @@ abstract final class CodePush { /// which breaks Apple Clang LTO. static Future _installPatchFromDart(Uint8List patchBytes) async { // Ask the engine for its configured patch directory path. - final patchDir = - await _channel.invokeMethod('CodePush.getPatchDir'); + final patchDir = await _channel.invokeMethod( + 'CodePush.getPatchDir', + ); if (patchDir == null || patchDir.isEmpty) { throw CodePushException('Engine returned no patch directory.'); } @@ -926,8 +995,8 @@ abstract final class CodePush { try { final infoFile = File('$patchDir/patch_info.json'); if (infoFile.existsSync()) { - final info = jsonDecode(infoFile.readAsStringSync()) - as Map; + final info = + jsonDecode(infoFile.readAsStringSync()) as Map; File('$patchDir/rolled_back_patch').writeAsStringSync( jsonEncode({ 'patch_id': info['patch_id'], @@ -1059,8 +1128,8 @@ abstract final class CodePush { final infoFile = File('$patchDir/patch_info.json'); if (infoFile.existsSync()) { try { - final info = jsonDecode(infoFile.readAsStringSync()) - as Map; + final info = + jsonDecode(infoFile.readAsStringSync()) as Map; File('$patchDir/rolled_back_patch').writeAsStringSync( jsonEncode({ 'patch_id': info['patch_id'], @@ -1150,8 +1219,11 @@ class CodePushOverlay extends StatefulWidget { /// Optional custom banner builder. If null, uses the default banner. /// Return `null` to hide the banner. final Widget Function( - BuildContext context, VoidCallback onRestart, VoidCallback onDismiss)? - bannerBuilder; + BuildContext context, + VoidCallback onRestart, + VoidCallback onDismiss, + )? + bannerBuilder; /// Whether to show the debug status bar at the top. Defaults to false. final bool showDebugBar; @@ -1237,10 +1309,7 @@ class _CodePushOverlayState extends State textDirection: TextDirection.ltr, child: Stack( children: [ - KeyedSubtree( - key: ValueKey(_patchActive), - child: widget.child, - ), + KeyedSubtree(key: ValueKey(_patchActive), child: widget.child), if (widget.showDebugBar && !_patchActive) Positioned( top: 0, @@ -1255,11 +1324,14 @@ class _CodePushOverlayState extends State child: Container( color: const Color(0xFF1A237E), padding: const EdgeInsets.fromLTRB(12, 50, 12, 6), - child: Text('CP: $status', - style: const TextStyle( - color: Colors.white, - fontSize: 11, - decoration: TextDecoration.none)), + child: Text( + 'CP: $status', + style: const TextStyle( + color: Colors.white, + fontSize: 11, + decoration: TextDecoration.none, + ), + ), ), ), ), @@ -1308,18 +1380,10 @@ class _DefaultUpdateBanner extends StatelessWidget { children: [ const Icon(Icons.system_update, size: 20), const SizedBox(width: 12), - const Expanded( - child: Text('Update ready. Restart to apply.'), - ), - TextButton( - onPressed: onDismiss, - child: const Text('LATER'), - ), + const Expanded(child: Text('Update ready. Restart to apply.')), + TextButton(onPressed: onDismiss, child: const Text('LATER')), const SizedBox(width: 4), - FilledButton( - onPressed: onRestart, - child: const Text('RESTART'), - ), + FilledButton(onPressed: onRestart, child: const Text('RESTART')), ], ), ), @@ -1358,7 +1422,7 @@ class CodePushPatchBuilder extends StatelessWidget { /// Builder called with the patch data string (or null if no patch). final Widget Function(BuildContext context, String? patchData, Widget? child) - builder; + builder; /// Optional child widget passed to the builder (typically the default/baseline UI). final Widget? child; @@ -1372,7 +1436,10 @@ class CodePushPatchBuilder extends StatelessWidget { if (patchKey != null) { if (result.startsWith('$patchKey:')) { return builder( - context, result.substring(patchKey!.length + 1), child); + context, + result.substring(patchKey!.length + 1), + child, + ); } return builder(context, null, child); } @@ -1398,8 +1465,10 @@ Future<_HttpResult> _httpGet(String url) async { try { final request = await client.getUrl(Uri.parse(url)); final response = await request.close(); - final bytes = await response - .fold>([], (prev, chunk) => prev..addAll(chunk)); + final bytes = await response.fold>( + [], + (prev, chunk) => prev..addAll(chunk), + ); return _HttpResult(response.statusCode, utf8.decode(bytes), bytes); } finally { client.close(); @@ -1411,8 +1480,10 @@ Future<_HttpResult> _httpGetBytes(String url) async { try { final request = await client.getUrl(Uri.parse(url)); final response = await request.close(); - final bytes = await response - .fold>([], (prev, chunk) => prev..addAll(chunk)); + final bytes = await response.fold>( + [], + (prev, chunk) => prev..addAll(chunk), + ); return _HttpResult(response.statusCode, '', bytes); } finally { client.close(); @@ -1428,8 +1499,10 @@ Future<_HttpResult> _httpPostJson(String url, Map body) async { request.contentLength = encoded.length; request.add(encoded); final response = await request.close(); - final bytes = await response - .fold>([], (prev, chunk) => prev..addAll(chunk)); + final bytes = await response.fold>( + [], + (prev, chunk) => prev..addAll(chunk), + ); return _HttpResult(response.statusCode, utf8.decode(bytes), bytes); } finally { client.close();