test

actions/bin/deputy_scan.dart

@@ -46,21 +46,46 @@ Future<void> _deputyScan() async {
   if (updates == null) {
     return core.info('No updates needed');
   }
-  final git = NodeGitDir(deputy.repo.git);
-  final existingPrs = await _listExistingPrs();
-  final tmpDir = nodeFileSystem.systemTempDirectory.createTempSync('deputy');
 
   await allocateSwapSpace();
+  await createPrs(deputy.repo, updates);
+}
+
+/// Lists all Deputy PRs which currently exist in the repo with the PR number
+/// and the constraint
+Future<Map<String, int>> _listExistingPrs() async {
+  final octokit = github.getOctokit(process.getEnv('GITHUB_TOKEN')!);
+  return core.withGroup('Check for existing PRs', () async {
+    final existingPrs = <String, int>{};
+    final pulls = await octokit.rest.pulls.list();
+    for (final pull in pulls) {
+      final commitMessage =
+          CommitMessage.parse('', pull.title, body: pull.body);
+      final trailers = commitMessage.trailers;
+      final groupName = trailers['Updated-Group'];
+      if (groupName == null) {
+        continue;
+      }
+      existingPrs[groupName] = pull.number;
+    }
+    core.info('Found existing PRs: $existingPrs');
+    return existingPrs;
+  });
+}
 
-  // Create a PR for each dependency group which does not already have a PR.
-  for (final MapEntry(key: groupName, value: group)
-      in updates.entries) {
+/// Creates a PR for each dependency group, closing existing PRs which are superceded.
+Future<void> createPrs(
+  Repo repo,
+  Map<String, DependencyGroupUpdate> updates,
+) async {
+  final git = NodeGitDir(repo.git);
+  final tmpDir = nodeFileSystem.systemTempDirectory.createTempSync('deputy');
+  final existingPrs = await _listExistingPrs();
+  core.info('Creating PRs for update groups: $updates');
+  for (final MapEntry(key: groupName, value: group) in updates.entries) {
     // If the group updates all deps to a unique constraint, use that in messages.
-    final uniqueConstraint = group
-        .updatedConstraints
-        .values
-        .toSet()
-        .singleOrNull;
+    final uniqueConstraint =
+        group.updatedConstraints.values.toSet().singleOrNull;
     await core.withGroup('Create PR for group "$groupName"', () async {
       if (group.dependencies.any(_doNotUpdate.contains)) {
         core.info(
@@ -103,7 +128,7 @@ Future<void> _deputyScan() async {
         processManager: nodeProcessManager,
         fileSystem: nodeFileSystem,
         platform: nodePlatform,
-        logger: logger,
+        logger: repo.logger,
       );
       final worktree = NodeGitDir(worktreeRepo.git);
 
@@ -145,16 +170,16 @@ Updated-Group: $groupName
       final tmpFile = tmpDir.childFile('pr_body_$groupName.txt')
         ..createSync()
         ..writeAsStringSync(prBody);
-      final prResult = nodeProcessManager.runSync(
+      final prResult = await nodeProcessManager.run(
         <String>[
           'gh',
           'pr',
           'create',
           '--base=main',
           '--body-file=${tmpFile.path}',
           '--title=$commitTitle',
-          '--draft',
         ],
+        echoOutput: true,
         workingDirectory: worktreeDir,
       );
       if (prResult.exitCode != 0) {
@@ -165,20 +190,24 @@ Updated-Group: $groupName
         return;
       }
 
-      // Close existing PR with comment pointing to new PR.
+      // Close existing PR if this supercedes.
       final existingPr = existingPrs[groupName];
       if (existingPr == null) {
         return;
       }
 
       core.info('Closing existing PR...');
-      final closeResult = nodeProcessManager.runSync(<String>[
-        'gh',
-        'pr',
-        'close',
-        '$existingPr',
-        '--comment="Dependency has been updated. Closing in favor of new PR."',
-      ]);
+      final closeResult = await nodeProcessManager.run(
+        <String>[
+          'gh',
+          'pr',
+          'close',
+          '$existingPr',
+          '--comment="Closing in favor of new PR."',
+        ],
+        echoOutput: true,
+        workingDirectory: worktreeDir,
+      );
       if (closeResult.exitCode != 0) {
         core.error(
           'Failed to close existing PR. Will need to be closed manually: '
@@ -191,28 +220,6 @@ Updated-Group: $groupName
   }
 }
 
-/// Lists all Deputy PRs which currently exist in the repo with the PR number
-/// and the constraint
-Future<Map<String, int>> _listExistingPrs() async {
-  final octokit = github.getOctokit(process.getEnv('GITHUB_TOKEN')!);
-  return core.withGroup('Check for existing PRs', () async {
-    final existingPrs = <String, int>{};
-    final pulls = await octokit.rest.pulls.list();
-    for (final pull in pulls) {
-      final commitMessage =
-          CommitMessage.parse('', pull.title, body: pull.body);
-      final trailers = commitMessage.trailers;
-      final groupName = trailers['Updated-Group'];
-      if (groupName == null) {
-        continue;
-      }
-      existingPrs[groupName] = pull.number;
-    }
-    core.info('Found existing PRs: $existingPrs');
-    return existingPrs;
-  });
-}
-
 /// Special characters which appear in stringified [VersionConstraint]s.
 final _specialChars = RegExp(r'[\^<>=]');
 

actions/lib/src/deputy/post_update_task.dart

@@ -22,35 +22,50 @@ enum DependencyUpdateGroup {
       'code_builder',
     ],
   ),
-  drift(needsBuildRunner: true, dependencies: ['drift', 'drift_dev']);
+  drift(
+    needsBuildRunner: true,
+    dependencies: ['drift', 'drift_dev'],
+  );
 
   const DependencyUpdateGroup({
     required this.dependencies,
     this.needsBuildRunner = false,
     this.needsSmithy = false,
   });
 
+  /// The list of dependencies in this group.
+  final List<String> dependencies;
+
+  /// Whether `build_runner` should be run in dependent packages after an update.
   final bool needsBuildRunner;
+
+  /// Whether Smithy outputs need to be re-generated after an update.
   final bool needsSmithy;
-  final List<String> dependencies;
 
+  /// All groups mapped to their dependencies.
   static Map<String, List<String>> get all => {
         for (final value in values) value.name.snakeCase: value.dependencies,
       };
 
-  static DependencyUpdateGroup? of(String dependency) =>
-      values.firstWhereOrNull((el) => el.name.snakeCase == dependency);
+  /// Finds the group for the given [groupName].
+  static DependencyUpdateGroup? of(String groupName) =>
+      values.firstWhereOrNull((el) => el.name.snakeCase == groupName);
 
-  List<PostUpdateTask> buildTasks(Repo repo, Iterable<String> updatedPackages) {
+  /// Builds the list of post-update tasks for the group given the active [repo]
+  /// and the set of [updatedPackages].
+  List<PostUpdateTask> postUpdateTasks(
+    Repo repo,
+    Iterable<String> updatedPackages,
+  ) {
     return [
       if (needsSmithy) ...[
         const PostUpdateTask.aft(['generate', 'goldens']),
         // FIXME: Could run SDK but it would also pull latest models currently
         // so, updates may be unrelated to dep update.
         //
-        // Probably should have this run on a schedule before uncommenting this
-        // or find a way to pin the SDK ref so running `generate sdk` does not
-        // change the ref.
+        // Probably should have SDK gen run on a schedule before uncommenting this
+        // or find a way to track the SDK ref so running `generate sdk` does not
+        // necessarily pull the latest models (similar to goldens).
         // const PostUpdateTask.aft(['generate', 'sdk']),
       ],
       if (needsBuildRunner)
@@ -78,10 +93,10 @@ extension GroupPostUpdateTasks on DependencyGroupUpdate {
       return;
     }
     core.info('Running post-update tasks for "$groupName"');
-    final updatedPackages = updates.values
-        .expand((update) => update.dependentPackages.keys)
+    final updatedPackages = updatedConstraints.keys
+        .expand((updatedDep) => updates[updatedDep]!.dependentPackages.keys)
         .toSet();
-    final tasks = tasksBuilder.buildTasks(repo, updatedPackages);
+    final tasks = tasksBuilder.postUpdateTasks(repo, updatedPackages);
     for (final task in tasks) {
       await task.run(repo);
     }

actions/lib/src/util.dart

@@ -10,7 +10,7 @@ Never get unreachable => throw _UnreachableError();
 
 final class _UnreachableError extends Error {}
 
-/// Allocate swap space to help relieve memory pressure when running `build_runner`.
+/// Allocates swap space to help relieve memory pressure when running `build_runner`.
 /// 
 /// By default, the large runners do not allocate any swap space. Without swap space,
 /// `build_runner` actions will often fail with an "The operation was canceled" exception.
@@ -61,7 +61,7 @@ $script
       if (exitCode != 0) {
         throw ProcessException(
           '/bin/bash',
-          [script],
+          [scriptPath],
           'Script failed with exit code', 
           exitCode,
         );

packages/aft/common/lib/src/deputy/deputy.dart

@@ -127,13 +127,17 @@ final class Deputy {
     for (final MapEntry(key: groupName, value: groupUpdates)
         in dependencyGroups.toMap().entries) {
       for (final update in groupUpdates) {
-        final proposedUpdate = (proposedUpdates[groupName] ??=
-            DependencyGroupUpdateBuilder())
-          ..groupName = groupName
-          ..deputy = this
-          ..dependencies.addAll(this.dependencyGroups[groupName] ?? const {})
-          ..dependencies.add(update.dependencyName)
-          ..updates[update.dependencyName] = update;
+        // Lazily add a proposed update when one of the below conditions are true:
+        // - There is a change to the global constraint
+        // - There is a change to one of the packages' constraints.
+        DependencyGroupUpdateBuilder proposedUpdate() {
+          return (proposedUpdates[groupName] ??= DependencyGroupUpdateBuilder())
+            ..groupName = groupName
+            ..deputy = this
+            ..dependencies.addAll(this.dependencyGroups[groupName] ?? const {})
+            ..dependencies.add(update.dependencyName);
+        }
+
         if (update.globalConstraint case final globalConstraint?) {
           final updatedGlobalConstraint = versionResolver.updateFor(
             update.dependencyName,
@@ -144,7 +148,8 @@ final class Deputy {
             logger
               ?..info('Proposing global update to ${update.dependencyName}:')
               ..info('  $globalConstraint -> $updatedGlobalConstraint');
-            proposedUpdate
+            proposedUpdate()
+              ..updates[update.dependencyName] = update
               ..updatedConstraints[update.dependencyName] ??=
                   updatedGlobalConstraint
               ..pubspecUpdates.add(
@@ -177,7 +182,8 @@ final class Deputy {
               ? DependencyType.dependency
               : DependencyType.devDependency;
 
-          proposedUpdate
+          proposedUpdate()
+            ..updates[update.dependencyName] = update
             ..updatedConstraints[update.dependencyName] = updatedConstraint
             ..pubspecUpdates.add(
               (repo) => repo

packages/aft/common/test/deputy/deputy_test.dart

@@ -48,9 +48,15 @@ void main() {
         d.package(
           'local_a',
           dependencies: {
+            // ab group
             'third_party_a': '1.0.0',
             'third_party_b': '1.0.0',
+
+            // c group
             'third_party_c': '1.0.0',
+
+            // not updated
+            'third_party_d': '1.0.0',
           },
         ),
       ]).create();
@@ -67,6 +73,7 @@ void main() {
           'third_party_a': '1.1.0',
           'third_party_b': '1.1.0',
           'third_party_c': '1.1.0',
+          'third_party_d': '1.0.0',
         }),
       );
       final updates = await deputy.scanForUpdates();
@@ -76,7 +83,8 @@ void main() {
         updates!.keys,
         unorderedEquals([abGroupName, cGroupName]),
         reason: 'The third party group should be bundled together. '
-            'third_party_c should be in its own group.',
+            'third_party_c should be in its own group. '
+            'third_party_d should not be included.',
       );
       expect(
         updates.values,
@@ -138,6 +146,7 @@ void main() {
             'third_party_a': '1.1.0',
             'third_party_b': '1.1.0',
             'third_party_c': '1.1.0',
+            'third_party_d': '1.0.0',
           },
         ),
       ]).validate();