From 08c831673f6738713b32a8db383e1ae6e224a1a9 Mon Sep 17 00:00:00 2001 From: Dmitry Grand Date: Mon, 9 Mar 2026 11:10:36 -0700 Subject: [PATCH 1/2] fixed LUCI BUILD log url for presubmit --- app_dart/lib/server.dart | 2 +- .../get_presubmit_checks.dart | 2 +- .../get_presubmit_guard_summaries.dart | 2 +- .../get_presubmit_check_20260205/spec.md | 2 +- .../get_presubmit_guard_20260204/plan.md | 2 +- .../get_presubmit_guard_20260204/spec.md | 2 +- .../plan.md | 2 +- .../spec.md | 4 +- .../archive/presubmit_view_20260209/plan.md | 2 +- .../archive/presubmit_view_20260209/spec.md | 8 +-- .../public_presubmit_apis_20260226/plan.md | 2 +- dashboard/lib/views/presubmit_view.dart | 2 +- .../widgets/luci_task_attempt_summary.dart | 2 +- packages/cocoon_common/lib/build_log_url.dart | 60 +++++++++++++++++-- .../test/build_log_url_test.dart | 8 +-- 15 files changed, 77 insertions(+), 25 deletions(-) diff --git a/app_dart/lib/server.dart b/app_dart/lib/server.dart index a696eb6f13..dd3525fe49 100644 --- a/app_dart/lib/server.dart +++ b/app_dart/lib/server.dart @@ -180,7 +180,7 @@ Server createServer({ /// /// Consolidates multiple [PresubmitGuard] records (one per stage) into a single response. /// - /// GET: /api/get-presubmit-guard + /// GET: /api/public/get-presubmit-guard /// /// Parameters: /// slug: (string in query) required. The repository owner/name (e.g., 'flutter/flutter'). diff --git a/app_dart/lib/src/request_handlers/get_presubmit_checks.dart b/app_dart/lib/src/request_handlers/get_presubmit_checks.dart index 2cd9fa1961..3ea1a88931 100644 --- a/app_dart/lib/src/request_handlers/get_presubmit_checks.dart +++ b/app_dart/lib/src/request_handlers/get_presubmit_checks.dart @@ -13,7 +13,7 @@ import '../service/firestore/unified_check_run.dart'; /// Returns all checks for a specific presubmit check run. /// -/// GET: /api/get-presubmit-checks +/// GET: /api/public/get-presubmit-checks /// /// Parameters: /// check_run_id: (int in query) mandatory. The GitHub Check Run ID. diff --git a/app_dart/lib/src/request_handlers/get_presubmit_guard_summaries.dart b/app_dart/lib/src/request_handlers/get_presubmit_guard_summaries.dart index f2ff2e0ffc..897e53f51e 100644 --- a/app_dart/lib/src/request_handlers/get_presubmit_guard_summaries.dart +++ b/app_dart/lib/src/request_handlers/get_presubmit_guard_summaries.dart @@ -16,7 +16,7 @@ import '../service/firestore/unified_check_run.dart'; /// Request handler for retrieving all presubmit guards for a specific pull request. /// -/// GET: /api/get-presubmit-guard-summaries +/// GET: /api/public/get-presubmit-guard-summaries /// /// Parameters: /// repo: (string in query) required. The repository name (e.g., 'flutter'). diff --git a/conductor/archive/get_presubmit_check_20260205/spec.md b/conductor/archive/get_presubmit_check_20260205/spec.md index be564716e9..6c422a4090 100644 --- a/conductor/archive/get_presubmit_check_20260205/spec.md +++ b/conductor/archive/get_presubmit_check_20260205/spec.md @@ -4,7 +4,7 @@ This track involves implementing a new API endpoint in the `app_dart` backend service to provide detailed information about a specific presubmit check. The dashboard will use this API to display the history and status of all checks for a given check run. ## Functional Requirements -- **Endpoint:** `/api/get-presubmit-checks` +- **Endpoint:** `/api/public/get-presubmit-checks` - **Method:** GET - **Parameters (Mandatory):** - `check_run_id`: The unique identifier for the GitHub Check Run. diff --git a/conductor/archive/get_presubmit_guard_20260204/plan.md b/conductor/archive/get_presubmit_guard_20260204/plan.md index aef4c8d862..53bc8c271f 100644 --- a/conductor/archive/get_presubmit_guard_20260204/plan.md +++ b/conductor/archive/get_presubmit_guard_20260204/plan.md @@ -21,7 +21,7 @@ Implement the request handler and wire it into the `app_dart` server. - Consolidate records into the `GetPresubmitGuardResponse` format. - Handle cases with no matching records (e.g., return 404 or empty response). - [x] Task: Register Endpoint in Server [checkpoint: 8f78382] - - [x] Add the new `/api/get-presubmit-guard` route to the `app_dart` server configuration (e.g., `app_dart/lib/src/server.dart` or equivalent). + - [x] Add the new `/api/public/get-presubmit-guard` route to the `app_dart` server configuration (e.g., `app_dart/lib/src/server.dart` or equivalent). - [x] Task: Conductor - User Manual Verification 'Phase 2: API Endpoint Implementation' (Protocol in workflow.md) ## Phase 3: Verification & Documentation diff --git a/conductor/archive/get_presubmit_guard_20260204/spec.md b/conductor/archive/get_presubmit_guard_20260204/spec.md index ee4acf2cda..b87273838d 100644 --- a/conductor/archive/get_presubmit_guard_20260204/spec.md +++ b/conductor/archive/get_presubmit_guard_20260204/spec.md @@ -7,7 +7,7 @@ This track involves implementing a new backend API endpoint in the `app_dart` se As a Flutter developer, I want to see the real-time status of my PR's presubmit checks on the Cocoon dashboard so that I can quickly identify and address failures without navigating through multiple GitHub or LUCI pages. ## Functional Requirements -1. **New API Endpoint:** Create an authenticated GET endpoint in `app_dart` (e.g., `/api/get-presubmit-guard`). +1. **New API Endpoint:** Create an authenticated GET endpoint in `app_dart` (e.g., `/api/public/get-presubmit-guard`). 2. **Input Parameters:** The endpoint must accept a `slug` (e.g., `flutter/flutter`) and a `commit_sha` as query parameters. 3. **Data Retrieval:** - Query Cloud Firestore for all `PresubmitGuard` records matching the provided slug and commit SHA. diff --git a/conductor/archive/get_presubmit_guard_summaries_20260211/plan.md b/conductor/archive/get_presubmit_guard_summaries_20260211/plan.md index bc447f41e5..009f2f528a 100644 --- a/conductor/archive/get_presubmit_guard_summaries_20260211/plan.md +++ b/conductor/archive/get_presubmit_guard_summaries_20260211/plan.md @@ -40,7 +40,7 @@ This plan outlines the steps to refactor `GuardStatus` logic and implement the ` - [x] Task: Conductor - User Manual Verification 'Rename Handler to GetPresubmitGuardSummaries' (Protocol in workflow.md) ## Phase 6: Update API URL -- [x] Task: Update API URL in `app_dart/lib/server.dart` to `/api/get-presubmit-guard-summaries`. +- [x] Task: Update API URL in `app_dart/lib/server.dart` to `/api/public/get-presubmit-guard-summaries`. - [x] Task: Update documentation in `GetPresubmitGuardSummaries` handler. - [x] Task: Verify all tests pass. - [x] Task: Conductor - User Manual Verification 'Update API URL' (Protocol in workflow.md) diff --git a/conductor/archive/get_presubmit_guard_summaries_20260211/spec.md b/conductor/archive/get_presubmit_guard_summaries_20260211/spec.md index 07ce7a9032..d872c89d85 100644 --- a/conductor/archive/get_presubmit_guard_summaries_20260211/spec.md +++ b/conductor/archive/get_presubmit_guard_summaries_20260211/spec.md @@ -9,7 +9,7 @@ This track involves implementing a new request handler in the `app_dart` service - Updated existing `GetPresubmitGuard` handler to use the centralized logic. - **RPC Model:** - Created `PresubmitGuardSummary` model in `packages/cocoon_common` to represent the aggregated status of guards for a single commit SHA. -- **Endpoint:** Created a new GET endpoint: `/api/get-presubmit-guard-summaries`. +- **Endpoint:** Created a new GET endpoint: `/api/public/get-presubmit-guard-summaries`. - **Input Parameters:** - `repo`: The GitHub repository name (required). - `pr`: The pull request number (required). @@ -29,6 +29,6 @@ This track involves implementing a new request handler in the `app_dart` service - [x] `GetPresubmitGuard` refactored to use `GuardStatus.calculate`. - [x] `PresubmitGuardSummary` RPC model created and exported. - [x] `GetPresubmitGuardSummaries` handler implemented with grouping and aggregation logic. -- [x] Endpoint registered at `/api/get-presubmit-guard-summaries`. +- [x] Endpoint registered at `/api/public/get-presubmit-guard-summaries`. - [x] Unit tests cover all requirements and grouping logic. - [x] Code coverage > 95%. diff --git a/conductor/archive/presubmit_view_20260209/plan.md b/conductor/archive/presubmit_view_20260209/plan.md index d5bab6f695..1470681fc1 100644 --- a/conductor/archive/presubmit_view_20260209/plan.md +++ b/conductor/archive/presubmit_view_20260209/plan.md @@ -5,7 +5,7 @@ - [x] Write Tests: Create unit tests for the new models, ensuring correct JSON deserialization based on the Cocoon API structure. (3c968d9) - [x] Implement: Reuse model classes from `cocoon_common`. (3c968d9) - [x] Task: Integrate the new endpoints into `CocoonService`. (3c968d9) - - [x] Write Tests: Mock the `/dashboard/api/get-presubmit-guard` and `/dashboard/api/get-presubmit-checks` endpoints and verify the service correctly fetches and parses the data. (3c968d9) + - [x] Write Tests: Mock the `/dashboard/api/public/get-presubmit-guard` and `/dashboard/api/public/get-presubmit-checks` endpoints and verify the service correctly fetches and parses the data. (3c968d9) - [x] Implement: Add `fetchPresubmitGuard` and `fetchPresubmitCheckDetails` methods to `CocoonService` and its implementations. (3c968d9) - [x] Task: Conductor - User Manual Verification 'Phase 1: Infrastructure & Data Model' (Protocol in workflow.md) (085b744) diff --git a/conductor/archive/presubmit_view_20260209/spec.md b/conductor/archive/presubmit_view_20260209/spec.md index 4f9b36afc5..0a9cb6181e 100644 --- a/conductor/archive/presubmit_view_20260209/spec.md +++ b/conductor/archive/presubmit_view_20260209/spec.md @@ -9,7 +9,7 @@ Implement a detailed monitoring view for a specific Pull Request (PR) in the Flu - `?repo=&pr=`: **Mocked Route**. Displays placeholder data based on the provided layout. - **Data Integration (Checks Sidebar):** - For the functional `sha` route, the sidebar MUST be populated by calling the Cocoon API: - `GET /api/get-presubmit-guard?slug=flutter/&sha=` + `GET /api/public/get-presubmit-guard?slug=flutter/&sha=` - **API Response Handling:** The UI must parse the `PresubmitGuardResponse` and map it to the sidebar: - `stages`: Map each stage to a sidebar section (e.g., Engine, Framework). - `builds` (within stages): Map each build entry to an individual check item with its name and status. @@ -23,7 +23,7 @@ Implement a detailed monitoring view for a specific Pull Request (PR) in the Flu - **Log Viewer Pane:** - Display the "Execution Log" for the selected check in the sidebar. - For the functional `sha` route, fetch the check details using the Cocoon API: - `GET /api/get-presubmit-checks?check_run_id=&build_name=` + `GET /api/public/get-presubmit-checks?check_run_id=&build_name=` - **Handling Multiple Attempts (Tabs):** - If the API returns multiple `PresubmitCheckResponse` objects for a build, display them as tabs in the log viewer (as shown in the layout). - **Tab Naming:** Use the `attemptNumber` prefixed with a hash as the tab label (e.g., `#1`, `#2`). @@ -38,9 +38,9 @@ Implement a detailed monitoring view for a specific Pull Request (PR) in the Flu - **Accessibility:** Adhere to WCAG 2.1 Level AA standards for new UI components. ## Acceptance Criteria -- [ ] Navigating to `?repo=flutter&sha=` correctly calls the `/api/get-presubmit-guard` endpoint and renders the sidebar. +- [ ] Navigating to `?repo=flutter&sha=` correctly calls the `/api/public/get-presubmit-guard` endpoint and renders the sidebar. - [ ] Navigating to `?repo=flutter&pr=` displays the mocked dashboard layout. -- [ ] Selecting a check in the sidebar correctly calls the `/api/get-presubmit-checks` endpoint (for functional SHA routes). +- [ ] Selecting a check in the sidebar correctly calls the `/api/public/get-presubmit-checks` endpoint (for functional SHA routes). - [ ] Multiple attempts for a check are displayed as clickable tabs labeled by their attempt number. - [ ] The UI supports both Light and Dark modes. diff --git a/conductor/archive/public_presubmit_apis_20260226/plan.md b/conductor/archive/public_presubmit_apis_20260226/plan.md index 5d4124ff8d..fe507395bf 100644 --- a/conductor/archive/public_presubmit_apis_20260226/plan.md +++ b/conductor/archive/public_presubmit_apis_20260226/plan.md @@ -35,7 +35,7 @@ This phase transitions the specified handlers to `PublicApiRequestHandler`. - [x] Task: Verify Public Access - [x] Add/Update tests for each handler to verify they return successful responses even when no authentication is provided. - [x] Task: Update API Paths - - [x] Change `/api/get-presubmit-*` to `/api/public/get-presubmit-*` in `app_dart/lib/server.dart`. + - [x] Change `/api/public/get-presubmit-*` to `/api/public/get-presubmit-*` in `app_dart/lib/server.dart`. - [x] Task: Refactor all Public Handlers - [x] Identify all handlers with `/api/public/` path. - [x] Update `GetBuildStatus`, `GetSuppressedTests`, `GetRepos`, `GetEngineArtifactsReady`, `GetReleaseBranches`, `GetStatus`, `GetGreenCommits`, and `GithubRateLimitStatus` to extend `PublicApiRequestHandler`. diff --git a/dashboard/lib/views/presubmit_view.dart b/dashboard/lib/views/presubmit_view.dart index 0c6d869aa9..cfd02b78d3 100644 --- a/dashboard/lib/views/presubmit_view.dart +++ b/dashboard/lib/views/presubmit_view.dart @@ -428,7 +428,7 @@ class _LogViewerPaneState extends State<_LogViewerPane> { ? null : () async => await launchUrl( Uri.parse( - generateBuildLogUrl( + generatePreSubmitBuildLogUrl( buildName: selectedCheck.buildName, buildNumber: selectedCheck.buildNumber!, ), diff --git a/dashboard/lib/widgets/luci_task_attempt_summary.dart b/dashboard/lib/widgets/luci_task_attempt_summary.dart index d0374432a4..7a305b9106 100644 --- a/dashboard/lib/widgets/luci_task_attempt_summary.dart +++ b/dashboard/lib/widgets/luci_task_attempt_summary.dart @@ -25,7 +25,7 @@ class LuciTaskAttemptSummary extends StatelessWidget { return ElevatedButton( child: Text('OPEN LOG FOR BUILD #$buildNumber'), onPressed: () async { - final url = generateBuildLogUrl( + final url = generatePostSubmitBuildLogUrl( buildName: task.builderName, buildNumber: buildNumber, isBringup: task.isBringup, diff --git a/packages/cocoon_common/lib/build_log_url.dart b/packages/cocoon_common/lib/build_log_url.dart index 18164ddf25..52260c29c6 100644 --- a/packages/cocoon_common/lib/build_log_url.dart +++ b/packages/cocoon_common/lib/build_log_url.dart @@ -11,11 +11,44 @@ const String luciProdLogBase = 'https://ci.chromium.org/p/flutter/builders'; const String dartInternalLogBase = 'https://ci.chromium.org/p/dart-internal/builders'; -/// Generates a LUCI UI URL for a build log. -String generateBuildLogUrl({ +/// Generates a LUCI UI URL for a postsubmit build log. +/// +/// The [isBringup] flag configures the URL to target the staging builder pool +/// (if true) rather than the default production pool. [buildName] and +/// [buildNumber] are required to identify the specific build. +String generatePostSubmitBuildLogUrl({ required String buildName, required int buildNumber, bool isBringup = false, +}) { + return _generateBuildLogUrl( + buildName: buildName, + buildNumber: buildNumber, + buildersGroup: isBringup + ? BuildersGroup.flutterStaging + : BuildersGroup.flutter, + ); +} + +/// Generates a LUCI UI URL for a presubmit build log. +/// +/// Targets the try builder pool. [buildName] and [buildNumber] are required +/// to identify the specific build. +String generatePreSubmitBuildLogUrl({ + required String buildName, + required int buildNumber, +}) { + return _generateBuildLogUrl( + buildName: buildName, + buildNumber: buildNumber, + buildersGroup: BuildersGroup.flutterTryBuilders, + ); +} + +String _generateBuildLogUrl({ + required String buildName, + required int buildNumber, + required BuildersGroup buildersGroup, }) { if (isTaskFromDartInternalBuilder(builderName: buildName)) { return Uri.https( @@ -23,10 +56,29 @@ String generateBuildLogUrl({ '/p/dart-internal/builders/flutter/$buildName/$buildNumber', ).toString(); } else { - final pool = isBringup ? 'staging' : 'prod'; return Uri.https( 'ci.chromium.org', - '/p/flutter/builders/$pool/$buildName/$buildNumber', + '/p/flutter/builders/${buildersGroup.value}/$buildName/$buildNumber', ).toString(); } } + +/// Represents the group or pool of builders running the task. +/// +/// Corresponds to the segment in the LUCI URL path to target +/// specific builder groups (like 'prod', 'staging', or 'try'). +enum BuildersGroup { + /// The production builders pool. + flutter('prod'), + + /// The staging builders pool for bringup tasks. + flutterStaging('staging'), + + /// The try builders pool for presubmit tasks. + flutterTryBuilders('try'); + + const BuildersGroup(this.value); + + /// The string value representing this group in a LUCI URL. + final String value; +} diff --git a/packages/cocoon_common/test/build_log_url_test.dart b/packages/cocoon_common/test/build_log_url_test.dart index cf4830f687..5236a2784e 100644 --- a/packages/cocoon_common/test/build_log_url_test.dart +++ b/packages/cocoon_common/test/build_log_url_test.dart @@ -6,17 +6,17 @@ import 'package:cocoon_common/build_log_url.dart'; import 'package:test/test.dart'; void main() { - group('generateBuildLogUrl', () { + group('generatePostSubmitBuildLogUrl', () { test('generates luci prod url', () { expect( - generateBuildLogUrl(buildName: 'Linux', buildNumber: 123), + generatePostSubmitBuildLogUrl(buildName: 'Linux', buildNumber: 123), '$luciProdLogBase/prod/Linux/123', ); }); test('generates luci staging url', () { expect( - generateBuildLogUrl( + generatePostSubmitBuildLogUrl( buildName: 'Linux', buildNumber: 123, isBringup: true, @@ -27,7 +27,7 @@ void main() { test('generates dart-internal url', () { expect( - generateBuildLogUrl( + generatePostSubmitBuildLogUrl( buildName: 'Linux flutter_release_builder', buildNumber: 123, ), From 051dbe43e0b4f91abbc033f549663c165bd4285a Mon Sep 17 00:00:00 2001 From: Dmitry Grand Date: Mon, 9 Mar 2026 11:21:59 -0700 Subject: [PATCH 2/2] fix gemini review requests --- .../public_presubmit_apis_20260226/plan.md | 2 +- .../test/build_log_url_test.dart | 29 +++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/conductor/archive/public_presubmit_apis_20260226/plan.md b/conductor/archive/public_presubmit_apis_20260226/plan.md index fe507395bf..5d4124ff8d 100644 --- a/conductor/archive/public_presubmit_apis_20260226/plan.md +++ b/conductor/archive/public_presubmit_apis_20260226/plan.md @@ -35,7 +35,7 @@ This phase transitions the specified handlers to `PublicApiRequestHandler`. - [x] Task: Verify Public Access - [x] Add/Update tests for each handler to verify they return successful responses even when no authentication is provided. - [x] Task: Update API Paths - - [x] Change `/api/public/get-presubmit-*` to `/api/public/get-presubmit-*` in `app_dart/lib/server.dart`. + - [x] Change `/api/get-presubmit-*` to `/api/public/get-presubmit-*` in `app_dart/lib/server.dart`. - [x] Task: Refactor all Public Handlers - [x] Identify all handlers with `/api/public/` path. - [x] Update `GetBuildStatus`, `GetSuppressedTests`, `GetRepos`, `GetEngineArtifactsReady`, `GetReleaseBranches`, `GetStatus`, `GetGreenCommits`, and `GithubRateLimitStatus` to extend `PublicApiRequestHandler`. diff --git a/packages/cocoon_common/test/build_log_url_test.dart b/packages/cocoon_common/test/build_log_url_test.dart index 5236a2784e..50092d57b7 100644 --- a/packages/cocoon_common/test/build_log_url_test.dart +++ b/packages/cocoon_common/test/build_log_url_test.dart @@ -34,5 +34,34 @@ void main() { '$dartInternalLogBase/flutter/Linux%20flutter_release_builder/123', ); }); + test('generates dart-internal url with isBringup', () { + expect( + generatePostSubmitBuildLogUrl( + buildName: 'Linux flutter_release_builder', + buildNumber: 123, + isBringup: true, + ), + '$dartInternalLogBase/flutter/Linux%20flutter_release_builder/123', + ); + }); + }); + + group('generatePreSubmitBuildLogUrl', () { + test('generates luci try url', () { + expect( + generatePreSubmitBuildLogUrl(buildName: 'Linux', buildNumber: 123), + '$luciProdLogBase/try/Linux/123', + ); + }); + + test('generates dart-internal url', () { + expect( + generatePreSubmitBuildLogUrl( + buildName: 'Linux flutter_release_builder', + buildNumber: 123, + ), + '$dartInternalLogBase/flutter/Linux%20flutter_release_builder/123', + ); + }); }); }