fix(cli): Ensure pub cache is fixed during startup (#337)

dnys1 · web-flow · commit bbb5c3532a95 · 2025-04-19T13:26:26.000-07:00
Perform pub cache fixes during initialization if needed to ensure projects do not default to using Flutter SDK.
diff --git a/apps/cli/CHANGELOG.md b/apps/cli/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 - fix: Pass `flutter_assets` to backend when using `celest deploy`
 - fix: Sentry integration
+- fix: Ensure pub cache is fixed during startup
 
 ## 1.0.10+2
 
diff --git a/apps/cli/lib/src/commands/auth/logout_command.dart b/apps/cli/lib/src/commands/auth/logout_command.dart
@@ -25,11 +25,8 @@ final class LogoutCommand extends CelestCommand with Authenticate {
 
     // Remove local cloud cache
     try {
-      final config = await CelestConfig.load();
-      final cloudDb = config.configDir.childFile('cloud.db');
-      if (cloudDb.existsSync()) {
-        await cloudDb.delete();
-      }
+      final config = CelestConfig();
+      await config.secureSettings.clear();
     } on Object catch (e, st) {
       performance.captureError(e, stackTrace: st);
     }
diff --git a/apps/cli/lib/src/config/celest_config.dart b/apps/cli/lib/src/config/celest_config.dart
@@ -1,5 +1,3 @@
-import 'dart:convert';
-
 import 'package:celest_cli/src/config/find_application_home.dart';
 import 'package:celest_cli/src/context.dart';
 import 'package:file/file.dart';
@@ -9,42 +7,26 @@ import 'package:native_storage/native_storage.dart';
 final class CelestConfig {
   CelestConfig._(this.configDir);
 
-  static final Logger logger = Logger('CelestConfig');
-
-  static Future<CelestConfig> load({Directory? configHome}) async {
+  factory CelestConfig({Directory? configHome}) {
     configHome ??= fileSystem.directory(applicationConfigHome('Celest'));
-    logger.finest('Loading configuration from $configHome');
-    if (!await configHome.exists()) {
-      await configHome.create(recursive: true);
-    }
-
-    // Migrate the old config JSON to local storage if it exists.
-    final configJson = configHome.childFile('config.json');
-    if (configJson.existsSync()) {
-      logger.finest('Migrating configuration to local storage');
-      final config =
-          jsonDecode(await configJson.readAsString()) as Map<String, Object?>;
-      await Future.wait(
-        config.entries.map(
-          (entry) => _settings.write(entry.key, entry.value.toString()),
-        ),
-      );
-      await configJson.delete();
-      logger.finest('Successfully migrated configuration to local storage');
-    } else {
-      logger.finest('Configuration already migrated to local storage');
+    if (!configHome.existsSync()) {
+      configHome.createSync(recursive: true);
     }
     return CelestConfig._(configHome);
   }
 
-  static CelestConfigValues get _settings =>
-      CelestConfigValues(storage.isolated);
+  static final Logger logger = Logger('CelestConfig');
+
   final Directory configDir;
-  CelestConfigValues get settings => _settings;
+
+  CelestSettings get settings => CelestSettings(storage);
+  CelestSecureSettings get secureSettings =>
+      CelestSecureSettings(storage.isolated);
 
   Future<void> delete() async {
-    await _settings.clear();
-    if (await configDir.exists()) {
+    settings.clear();
+    await secureSettings.clear();
+    if (configDir.existsSync()) {
       logger.fine('Removing Celest config dir: $configDir');
       await configDir.delete(recursive: true);
     }
@@ -54,7 +36,7 @@ final class CelestConfig {
   String toString() => 'CelestConfig: $configDir';
 }
 
-extension type CelestConfigValues(IsolatedNativeStorage settings)
+extension type CelestSecureSettings(IsolatedNativeStorage settings)
     implements IsolatedNativeStorage {
   Future<String?> getOrganizationId() async {
     return settings.read('organization_id');
@@ -68,3 +50,17 @@ extension type CelestConfigValues(IsolatedNativeStorage settings)
     }
   }
 }
+
+extension type CelestSettings(NativeStorage settings) implements NativeStorage {
+  String? get pubCacheFixDigest {
+    return settings.read('pub_cache_fix_digest');
+  }
+
+  set pubCacheFixDigest(String? value) {
+    if (value == null) {
+      settings.delete('pub_cache_fix_digest');
+    } else {
+      settings.write('pub_cache_fix_digest', value);
+    }
+  }
+}
diff --git a/apps/cli/lib/src/init/project_init.dart b/apps/cli/lib/src/init/project_init.dart
@@ -1,6 +1,7 @@
 import 'dart:async';
 import 'dart:io';
 
+import 'package:async/async.dart';
 import 'package:celest_cli/src/commands/celest_command.dart';
 import 'package:celest_cli/src/commands/init_command.dart';
 import 'package:celest_cli/src/commands/start_command.dart';
@@ -11,6 +12,7 @@ import 'package:celest_cli/src/init/project_migrate.dart';
 import 'package:celest_cli/src/init/templates/project_template.dart';
 import 'package:celest_cli/src/project/celest_project.dart';
 import 'package:celest_cli/src/pub/pub_action.dart';
+import 'package:celest_cli/src/pub/pub_cache.dart';
 import 'package:celest_cli/src/sdk/dart_sdk.dart';
 import 'package:celest_cli/src/utils/error.dart';
 import 'package:celest_cli/src/utils/recase.dart';
@@ -150,6 +152,8 @@ base mixin Configure on CelestCommand {
 
     yield const Initializing();
     await init(projectRoot: projectRoot, parentProject: parentProject);
+    await _fixPubCacheIfNeeded();
+    logger.finest('Celest project initialized');
 
     var needsAnalyzerMigration = false;
     Future<void>? upgradePackages;
@@ -316,6 +320,33 @@ base mixin Configure on CelestCommand {
     );
   }
 
+  /// Fixes the pub cache if needed.
+  Future<void> _fixPubCacheIfNeeded() async {
+    final pubCacheFixDigest = PubCache.packagesToFixDigest;
+    final previousDigest = celestProject.config.settings.pubCacheFixDigest;
+    if (pubCacheFixDigest != previousDigest) {
+      logger.finest('Hydrating pub cache...');
+      final result = await pubCache.hydrate();
+      if (result case ErrorResult(:final error)) {
+        logger.finest('Failed to hydrate pub cache', error);
+        performance.captureError(error, stackTrace: StackTrace.current);
+        return;
+      }
+      logger.finest('Fixing pub cache...');
+      try {
+        await pubCache.fix(throwOnError: true);
+      } on Object catch (e, st) {
+        logger.finest('Failed to fix pub cache', e, st);
+        performance.captureError(e, stackTrace: st);
+        return;
+      }
+      logger.finest('Pub cache fixed.');
+      celestProject.config.settings.pubCacheFixDigest = pubCacheFixDigest;
+    } else {
+      logger.finest('Skipping pub cache fix, already up-to-date.');
+    }
+  }
+
   // TODO(dnys1): Improve logic here so that we don't run pub upgrade if
   // the dependencies in the lockfile are already up to date.
   Future<void> _pubUpgrade() async {
diff --git a/apps/cli/lib/src/project/celest_project.dart b/apps/cli/lib/src/project/celest_project.dart
@@ -100,13 +100,12 @@ final class CelestProject {
       clientDir: clientDir,
       outputsDir: outputsDir,
     );
-    final [
-      config as CelestConfig,
-      analysisOptions as AnalysisOptions,
-    ] = await Future.wait([
-      CelestConfig.load(configHome: configHome?.let(fileSystem.directory)),
-      AnalysisOptions.load(projectPaths.analysisOptionsYaml),
-    ]);
+    final config = CelestConfig(
+      configHome: configHome?.let(fileSystem.directory),
+    );
+    final analysisOptions = await AnalysisOptions.load(
+      projectPaths.analysisOptionsYaml,
+    );
     _logger
       ..finest('Loaded analysis options: $analysisOptions')
       ..finest('Loaded Celest config: $config');
diff --git a/apps/cli/lib/src/pub/pub_cache.dart b/apps/cli/lib/src/pub/pub_cache.dart
@@ -1,10 +1,13 @@
+import 'dart:convert';
 import 'dart:io';
 
+import 'package:async/async.dart';
 import 'package:celest_cli/src/context.dart';
 import 'package:celest_cli/src/pub/project_dependency.dart';
 import 'package:celest_cli/src/sdk/dart_sdk.dart';
 import 'package:celest_cli/src/utils/process.dart';
 import 'package:collection/collection.dart';
+import 'package:crypto/crypto.dart';
 import 'package:file/file.dart';
 import 'package:logging/logging.dart';
 import 'package:meta/meta.dart';
@@ -22,12 +25,19 @@ final class PubCache {
     // This is the only syntax that reliably works with both dart/flutter
     'native_storage': '>=0.2.2 <1.0.0',
     'native_authentication': '>=0.1.0 <1.0.0',
-    'jni': '>=0.11.0 <1.0.0',
+    'jni': '>=0.14.0 <0.15.0',
     'celest_auth': '>=$currentMinorVersion <2.0.0',
     'celest': '>=$currentMinorVersion <2.0.0',
     'celest_core': '>=$currentMinorVersion <2.0.0',
-    'objective_c': '>=2.0.0 <8.0.0',
+    'objective_c': '>=7.0.0 <8.0.0',
   };
+
+  /// MD5 hash of the [packagesToFix] map.
+  static final String packagesToFixDigest = () {
+    final pubCacheFixJson = JsonUtf8Encoder().convert(PubCache.packagesToFix);
+    return md5.convert(pubCacheFixJson).toString();
+  }();
+
   static final _logger = Logger('PubCache');
 
   String? _cachePath;
@@ -96,44 +106,66 @@ final class PubCache {
   /// Runs `pub cache add` for each package in [packagesToFix].
   ///
   /// Returns the exit codes and output for each package.
-  Future<List<(int, String)>> hydrate() async {
-    final results = <(int, String)>[];
-    for (final package in packagesToFix.entries) {
-      // Run serially to avoid flutter lock
-      final result = await processManager.start(runInShell: true, [
-        Sdk.current.sdkType.name,
+  Future<Result<void>> hydrate() async {
+    Future<void> hydratePackage(String name, String constraint) async {
+      final command = [
+        Sdk.current.dart,
         'pub',
         'cache',
         'add',
-        package.key,
+        name,
         '--version',
-        package.value,
+        constraint,
         '--all',
-      ]).then((process) async {
-        final combinedOutput = StringBuffer();
-        process.captureStdout(
-          sink: (line) {
-            _logger.finest(line);
-            combinedOutput.writeln(line);
-          },
-        );
-        process.captureStderr(
-          sink: (line) {
-            _logger.finest(line);
-            combinedOutput.writeln(line);
-          },
+      ];
+      final process = await processManager.start(command, runInShell: true);
+      final combinedOutput = StringBuffer();
+      process.captureStdout(
+        sink: (line) {
+          _logger.finest(line);
+          combinedOutput.writeln(line);
+        },
+      );
+      process.captureStderr(
+        sink: (line) {
+          _logger.finest(line);
+          combinedOutput.writeln(line);
+        },
+      );
+      if (await process.exitCode case final exitCode && != 0) {
+        throw ProcessException(
+          command.first,
+          command.sublist(1),
+          combinedOutput.toString(),
+          exitCode,
         );
-        return (await process.exitCode, combinedOutput.toString());
-      });
-      results.add(result);
+      }
+    }
+
+    final hydrations = <Future<void>>[];
+    for (final package in packagesToFix.entries) {
+      hydrations.add(hydratePackage(package.key, package.value));
+    }
+    final results = await Result.captureAll(hydrations);
+
+    var failed = false;
+    final errors = <Object>[];
+    for (final result in results) {
+      if (result.isError) {
+        failed = true;
+        errors.add(result.asError!.error);
+      }
+    }
+    if (failed) {
+      return Result.error(ParallelWaitError(<Object?>[], errors));
     }
-    return results;
+    return Result.value(null);
   }
 
   /// Fixes the pubspec for each package in [packagesToFix].
   ///
   /// Returns the number of packages fixed.
-  Future<int> fix({@visibleForTesting bool throwOnError = false}) async {
+  Future<int> fix({bool throwOnError = false}) async {
     final cachePath = _cachePath ??= findCachePath();
     if (cachePath == null) {
       if (throwOnError) {
diff --git a/apps/cli/test/commands/uninstall_test.dart b/apps/cli/test/commands/uninstall_test.dart
@@ -31,13 +31,15 @@ void main() {
       ctx.fileSystem = MemoryFileSystem.test();
       await init(projectRoot: ctx.fileSystem.systemTempDirectory.path);
 
-      await celestProject.config.settings.setOrganizationId('org-id');
+      await celestProject.config.secureSettings.setOrganizationId('org-id');
 
-      expect(await celestProject.config.settings.getOrganizationId(), 'org-id');
+      expect(await celestProject.config.secureSettings.getOrganizationId(),
+          'org-id');
 
       await const CelestUninstaller().uninstall();
 
-      expect(await celestProject.config.settings.getOrganizationId(), isNull);
+      expect(await celestProject.config.secureSettings.getOrganizationId(),
+          isNull);
     });
 
     group('uninstall AOT', () {
diff --git a/apps/cli/test/config/celest_config_test.dart b/apps/cli/test/config/celest_config_test.dart
@@ -9,21 +9,41 @@ void main() {
   group('CelestConfig', () {
     setUp(initTests);
 
-    test('migrates to local storage', () async {
+    test('delete clears all storage interfaces', () async {
       ctx.fileSystem = MemoryFileSystem.test();
-      final configHome = ctx.fileSystem.systemTempDirectory.childDirectory(
-        'config',
-      )..createSync(recursive: true);
-
-      final configJson = configHome.childFile('config.json');
-      configJson.writeAsStringSync('{"organization_id": "org-id"}');
       await init(
         projectRoot: ctx.fileSystem.systemTempDirectory.path,
-        configHome: configHome.path,
       );
 
-      expect(await celestProject.config.settings.getOrganizationId(), 'org-id');
-      expect(await configJson.exists(), isFalse);
+      expect(
+        await celestProject.config.secureSettings.getOrganizationId(),
+        isNull,
+      );
+      await celestProject.config.secureSettings.setOrganizationId('org-id');
+      expect(
+        await celestProject.config.secureSettings.getOrganizationId(),
+        'org-id',
+      );
+
+      expect(
+        celestProject.config.settings.pubCacheFixDigest,
+        isNull,
+      );
+      celestProject.config.settings.pubCacheFixDigest = 'digest';
+      expect(
+        celestProject.config.settings.pubCacheFixDigest,
+        'digest',
+      );
+
+      await celestProject.config.delete();
+      expect(
+        await celestProject.config.secureSettings.getOrganizationId(),
+        isNull,
+      );
+      expect(
+        celestProject.config.settings.pubCacheFixDigest,
+        isNull,
+      );
     });
   });
 }
diff --git a/apps/cli/test/pub/pub_cache_test.dart b/apps/cli/test/pub/pub_cache_test.dart
