From d42b1d9c3f462d5eadca61e50231aeb30bbd2bb2 Mon Sep 17 00:00:00 2001 From: John McDole Date: Tue, 30 Jun 2026 09:50:53 -0700 Subject: [PATCH 1/3] feat(app_dart): fall back to ciStaging in GetPresubmitGuard when no guards found In the Unified Check Run flow, if no modern `UnifiedCheckRun` presubmit guards are found for a commit, retrieve the legacy `ciStaging` documents as a fallback. This will make waiting for check-runs to pass universal. - Implement `CiStaging.getCiStagingForCommitSha` to query staging documents by repository and commit SHA. - Refactor `GetPresubmitGuard` to invoke `_getCiStagingFallback` when no guards are present. - Extract `check_run_id` from the staging `checkRunGuard` JSON, and look up pull request and author metadata via `PrCheckRuns`. - Add unit tests verifying `ciStaging` fallback execution and JSON response structure. - Clean up related linter warnings (redundant imports, type annotations, and curly braces). --- .../lib/src/model/firestore/ci_staging.dart | 16 +++ .../request_handlers/get_presubmit_guard.dart | 97 ++++++++++++++++++- .../get_presubmit_guard_test.dart | 50 ++++++++++ dashboard/test/views/presubmit_view_test.dart | 6 +- 4 files changed, 164 insertions(+), 5 deletions(-) diff --git a/app_dart/lib/src/model/firestore/ci_staging.dart b/app_dart/lib/src/model/firestore/ci_staging.dart index d0f57caf41..cc93406187 100644 --- a/app_dart/lib/src/model/firestore/ci_staging.dart +++ b/app_dart/lib/src/model/firestore/ci_staging.dart @@ -112,6 +112,22 @@ final class CiStaging extends AppDocument { return CiStaging.fromDocument(document); } + /// Queries for [CiStaging] records by [slug] and [sha]. + /// + /// This is used to get both `_engine` and `_fusion` documents. + static Future> getCiStagingForCommitSha({ + required FirestoreService firestoreService, + required RepositorySlug slug, + required String sha, + }) async { + final filterMap = { + '$fieldRepoFullPath =': slug.fullName, + '$fieldCommitSha =': sha, + }; + final documents = await firestoreService.query(_collectionId, filterMap); + return documents.map(CiStaging.fromDocument).toList(); + } + /// Create [CiStaging] from a Commit Document. CiStaging.fromDocument(Document other) { this diff --git a/app_dart/lib/src/request_handlers/get_presubmit_guard.dart b/app_dart/lib/src/request_handlers/get_presubmit_guard.dart index c0817c853f..5bec07b87d 100644 --- a/app_dart/lib/src/request_handlers/get_presubmit_guard.dart +++ b/app_dart/lib/src/request_handlers/get_presubmit_guard.dart @@ -3,13 +3,16 @@ // found in the LICENSE file. import 'dart:async'; +import 'dart:convert'; import 'package:cocoon_common/guard_status.dart'; import 'package:cocoon_common/rpc_model.dart' as rpc_model; +import 'package:cocoon_common/task_status.dart'; import 'package:github/github.dart'; import 'package:meta/meta.dart'; import '../../cocoon_service.dart'; +import '../model/firestore/ci_staging.dart'; import '../request_handling/public_api_request_handler.dart'; import '../service/firestore/unified_check_run.dart'; @@ -95,9 +98,7 @@ final class GetPresubmitGuard extends PublicApiRequestHandler { ); if (guards.isEmpty) { - return Response.json({ - 'error': 'No guard found for slug $slug and sha $sha', - }, statusCode: HttpStatus.notFound); + return _getCiStagingFallback(slug, sha); } // Consolidate metadata from the first record. @@ -134,4 +135,94 @@ final class GetPresubmitGuard extends PublicApiRequestHandler { return Response.json(response); } + + Future _getCiStagingFallback( + RepositorySlug slug, + String sha, + ) async { + final ciStagings = await CiStaging.getCiStagingForCommitSha( + firestoreService: _firestore, + slug: slug, + sha: sha, + ); + + if (ciStagings.isEmpty) { + return Response.json({ + 'error': 'No guard found for slug $slug and sha $sha', + }, statusCode: HttpStatus.notFound); + } + + final totalFailed = ciStagings.fold(0, (sum, g) => sum + g.failed); + final totalRemaining = ciStagings.fold( + 0, + (sum, g) => sum + g.remaining, + ); + final totalBuilds = ciStagings.fold(0, (sum, g) => sum + g.total); + + final guardStatus = GuardStatus.calculate( + failedBuilds: totalFailed, + remainingBuilds: totalRemaining, + totalBuilds: totalBuilds, + ); + + var checkRunId = -1; + if (ciStagings.isNotEmpty) { + final guardJsonStr = ciStagings.first.checkRunGuard; + if (guardJsonStr.isNotEmpty) { + try { + // Try to extract the check-run id from the json string. + final guardJson = jsonDecode(guardJsonStr) as Map; + checkRunId = guardJson['id'] as int? ?? 0; + } catch (_) { + // ignore + } + } + } + + var prNum = 0; + var author = ''; + + final prInfo = await PrCheckRuns.findPullRequestForSha(_firestore, sha); + if (prInfo != null) { + prNum = prInfo.number ?? 0; + author = prInfo.user?.login ?? ''; + } + + final response = rpc_model.PresubmitGuardResponse( + prNum: prNum, + checkRunId: checkRunId, + author: author, + guardStatus: guardStatus, + enableGeminiLogAnalysis: config.flags.enableGeminiLogAnalysis, + stages: [ + for (final g in ciStagings) + rpc_model.PresubmitGuardStage( + name: g.stage?.name ?? 'unknown', + createdAt: g.createTime != null + ? DateTime.parse(g.createTime!).millisecondsSinceEpoch + : 0, + jobs: { + for (final MapEntry(:key, :value) in g.checkRuns.entries) + key: _taskStatusFromConclusion(value), + }, + ), + ], + ); + + return Response.json(response); + } + + TaskStatus _taskStatusFromConclusion(TaskConclusion conclusion) { + switch (conclusion) { + case TaskConclusion.unknown: + case TaskConclusion.failure: + return TaskStatus.failed; + case TaskConclusion.scheduled: + return TaskStatus.waitingForBackfill; + case TaskConclusion.success: + return TaskStatus.succeeded; + case TaskConclusion.neutral: + return TaskStatus.neutral; + } + } } diff --git a/app_dart/test/request_handlers/get_presubmit_guard_test.dart b/app_dart/test/request_handlers/get_presubmit_guard_test.dart index e0754bb51f..2430d163fd 100644 --- a/app_dart/test/request_handlers/get_presubmit_guard_test.dart +++ b/app_dart/test/request_handlers/get_presubmit_guard_test.dart @@ -11,9 +11,12 @@ import 'package:cocoon_common/task_status.dart'; import 'package:cocoon_integration_test/testing.dart'; import 'package:cocoon_server_test/test_logging.dart'; import 'package:cocoon_service/src/model/firestore/base.dart'; +import 'package:cocoon_service/src/model/firestore/ci_staging.dart'; import 'package:cocoon_service/src/request_handlers/get_presubmit_guard.dart'; import 'package:cocoon_service/src/request_handling/exceptions.dart'; +import 'package:cocoon_service/src/service/firestore.dart'; import 'package:github/github.dart'; +import 'package:googleapis/firestore/v1.dart' hide Status; import 'package:test/test.dart'; import '../src/request_handling/request_handler_tester.dart'; @@ -213,4 +216,51 @@ void main() { final response = await tester.get(handler); expect(response.statusCode, HttpStatus.ok); }); + + test('falls back to ciStaging if no guards found', () async { + final slug = RepositorySlug('flutter', 'flutter'); + const sha = 'abc'; + + final staging = CiStaging.fromDocument( + Document( + name: CiStaging.documentNameFor( + slug: slug, + sha: sha, + stage: CiStage.fusionEngineBuild, + ), + fields: { + CiStaging.kTotalField: 2.toValue(), + CiStaging.kRemainingField: 1.toValue(), + CiStaging.kFailedField: 0.toValue(), + CiStaging.kCheckRunGuardField: '{"id": 0}'.toValue(), + CiStaging.fieldRepoFullPath: slug.fullName.toValue(), + CiStaging.fieldCommitSha: sha.toValue(), + CiStaging.fieldStage: CiStage.fusionEngineBuild.name.toValue(), + 'job1': TaskConclusion.success.name.toValue(), + 'job2': TaskConclusion.scheduled.name.toValue(), + }, + ), + ); + + firestore.putDocuments([staging]); + + tester.request = FakeHttpRequest( + queryParametersValue: { + GetPresubmitGuard.kOwnerParam: 'flutter', + GetPresubmitGuard.kRepoParam: 'flutter', + GetPresubmitGuard.kShaParam: sha, + }, + ); + + final result = (await getResponse())!; + expect(result.prNum, 0); + expect(result.checkRunId, 0); + expect(result.guardStatus, GuardStatus.inProgress); + expect(result.stages.length, 1); + expect(result.stages.first.name, CiStage.fusionEngineBuild.name); + expect(result.stages.first.jobs, { + 'job1': TaskStatus.succeeded, + 'job2': TaskStatus.waitingForBackfill, + }); + }); } diff --git a/dashboard/test/views/presubmit_view_test.dart b/dashboard/test/views/presubmit_view_test.dart index e3d137c17f..d2aa13404f 100644 --- a/dashboard/test/views/presubmit_view_test.dart +++ b/dashboard/test/views/presubmit_view_test.dart @@ -1183,8 +1183,9 @@ void main() { for (var i = 0; i < 20; i++) { await tester.pump(); await Future.delayed(const Duration(milliseconds: 50)); - if (find.textContaining('linux_analyze').evaluate().isNotEmpty) + if (find.textContaining('linux_analyze').evaluate().isNotEmpty) { break; + } } }); await tester.pump(); @@ -1243,8 +1244,9 @@ void main() { for (var i = 0; i < 20; i++) { await tester.pump(); await Future.delayed(const Duration(milliseconds: 50)); - if (find.textContaining('linux_analyze').evaluate().isNotEmpty) + if (find.textContaining('linux_analyze').evaluate().isNotEmpty) { break; + } } }); await tester.pump(); From 945cfa43df897e7565327a5d95dfd343513d5f97 Mon Sep 17 00:00:00 2001 From: John McDole Date: Tue, 30 Jun 2026 10:00:42 -0700 Subject: [PATCH 2/3] toaster --- .../request_handlers/get_presubmit_guard.dart | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/app_dart/lib/src/request_handlers/get_presubmit_guard.dart b/app_dart/lib/src/request_handlers/get_presubmit_guard.dart index 5bec07b87d..74a93c1beb 100644 --- a/app_dart/lib/src/request_handlers/get_presubmit_guard.dart +++ b/app_dart/lib/src/request_handlers/get_presubmit_guard.dart @@ -166,16 +166,14 @@ final class GetPresubmitGuard extends PublicApiRequestHandler { ); var checkRunId = -1; - if (ciStagings.isNotEmpty) { - final guardJsonStr = ciStagings.first.checkRunGuard; - if (guardJsonStr.isNotEmpty) { - try { - // Try to extract the check-run id from the json string. - final guardJson = jsonDecode(guardJsonStr) as Map; - checkRunId = guardJson['id'] as int? ?? 0; - } catch (_) { - // ignore - } + final guardJsonStr = ciStagings.first.checkRunGuard; + if (guardJsonStr.isNotEmpty) { + try { + // Try to extract the check-run id from the json string. + final guardJson = jsonDecode(guardJsonStr) as Map; + checkRunId = guardJson['id'] as int? ?? 0; + } catch (_) { + // ignore } } @@ -198,9 +196,9 @@ final class GetPresubmitGuard extends PublicApiRequestHandler { for (final g in ciStagings) rpc_model.PresubmitGuardStage( name: g.stage?.name ?? 'unknown', - createdAt: g.createTime != null - ? DateTime.parse(g.createTime!).millisecondsSinceEpoch - : 0, + createdAt: + DateTime.tryParse(g.createTime ?? '')?.millisecondsSinceEpoch ?? + 0, jobs: { for (final MapEntry(:key, :value) in g.checkRuns.entries) key: _taskStatusFromConclusion(value), From 8d79a1353f129b814918bef1127214618477a3ed Mon Sep 17 00:00:00 2001 From: John McDole Date: Tue, 30 Jun 2026 11:31:38 -0700 Subject: [PATCH 3/3] reviewer --- .../src/model/common/checks_extension.dart | 10 ++++++++ .../request_handlers/get_presubmit_guard.dart | 18 ++------------- .../model/common/checks_extension_test.dart | 23 +++++++++++++++++++ 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/app_dart/lib/src/model/common/checks_extension.dart b/app_dart/lib/src/model/common/checks_extension.dart index f8b9fa568f..63b2137ade 100644 --- a/app_dart/lib/src/model/common/checks_extension.dart +++ b/app_dart/lib/src/model/common/checks_extension.dart @@ -69,4 +69,14 @@ extension ChecksExtension on TaskStatus { _ => .failed, }; } + + /// Converts a [TaskConclusion] to a [TaskStatus]. + static TaskStatus fromTaskConclusion(TaskConclusion conclusion) { + return switch (conclusion) { + TaskConclusion.unknown || TaskConclusion.failure => TaskStatus.failed, + TaskConclusion.scheduled => TaskStatus.waitingForBackfill, + TaskConclusion.success => TaskStatus.succeeded, + TaskConclusion.neutral => TaskStatus.neutral, + }; + } } diff --git a/app_dart/lib/src/request_handlers/get_presubmit_guard.dart b/app_dart/lib/src/request_handlers/get_presubmit_guard.dart index 74a93c1beb..eb906fb661 100644 --- a/app_dart/lib/src/request_handlers/get_presubmit_guard.dart +++ b/app_dart/lib/src/request_handlers/get_presubmit_guard.dart @@ -7,11 +7,11 @@ import 'dart:convert'; import 'package:cocoon_common/guard_status.dart'; import 'package:cocoon_common/rpc_model.dart' as rpc_model; -import 'package:cocoon_common/task_status.dart'; import 'package:github/github.dart'; import 'package:meta/meta.dart'; import '../../cocoon_service.dart'; +import '../model/common/checks_extension.dart'; import '../model/firestore/ci_staging.dart'; import '../request_handling/public_api_request_handler.dart'; import '../service/firestore/unified_check_run.dart'; @@ -201,7 +201,7 @@ final class GetPresubmitGuard extends PublicApiRequestHandler { 0, jobs: { for (final MapEntry(:key, :value) in g.checkRuns.entries) - key: _taskStatusFromConclusion(value), + key: ChecksExtension.fromTaskConclusion(value), }, ), ], @@ -209,18 +209,4 @@ final class GetPresubmitGuard extends PublicApiRequestHandler { return Response.json(response); } - - TaskStatus _taskStatusFromConclusion(TaskConclusion conclusion) { - switch (conclusion) { - case TaskConclusion.unknown: - case TaskConclusion.failure: - return TaskStatus.failed; - case TaskConclusion.scheduled: - return TaskStatus.waitingForBackfill; - case TaskConclusion.success: - return TaskStatus.succeeded; - case TaskConclusion.neutral: - return TaskStatus.neutral; - } - } } diff --git a/app_dart/test/model/common/checks_extension_test.dart b/app_dart/test/model/common/checks_extension_test.dart index c5065a1981..1f812c38ae 100644 --- a/app_dart/test/model/common/checks_extension_test.dart +++ b/app_dart/test/model/common/checks_extension_test.dart @@ -38,6 +38,29 @@ void main() { TaskStatus.neutral, ); }); + + test('fromTaskConclusion mapping', () { + expect( + ChecksExtension.fromTaskConclusion(TaskConclusion.success), + TaskStatus.succeeded, + ); + expect( + ChecksExtension.fromTaskConclusion(TaskConclusion.neutral), + TaskStatus.neutral, + ); + expect( + ChecksExtension.fromTaskConclusion(TaskConclusion.failure), + TaskStatus.failed, + ); + expect( + ChecksExtension.fromTaskConclusion(TaskConclusion.scheduled), + TaskStatus.waitingForBackfill, + ); + expect( + ChecksExtension.fromTaskConclusion(TaskConclusion.unknown), + TaskStatus.failed, + ); + }); }); group('TaskConclusion', () {