Skip to content

feat: add composedSchemaBreakingChanges to UpdateProposalResponse#2627

Merged
JivusAyrus merged 1 commit intomainfrom
suvij/eng-9154-add-the-new-check-details-to-the-proposal-view
Mar 11, 2026
Merged

feat: add composedSchemaBreakingChanges to UpdateProposalResponse#2627
JivusAyrus merged 1 commit intomainfrom
suvij/eng-9154-add-the-new-check-details-to-the-proposal-view

Conversation

@JivusAyrus
Copy link
Copy Markdown
Member

@JivusAyrus JivusAyrus commented Mar 11, 2026

This PR returns the composedSchema breaking changes on updating a proposal. This would help hub with evaluating the check on its ui.

Summary by CodeRabbit

Release Notes

  • New Features
    • Update proposal responses now include detected breaking schema changes when composing federated graphs.

Checklist

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 11, 2026

Walkthrough

A new repeated field composedSchemaBreakingChanges of type FederatedGraphSchemaChange[] is added to the UpdateProposalResponse message (field number 18) and propagated through the proposal update service to expose federated graph schema changes detected during schema checks.

Changes

Cohort / File(s) Summary
Protocol Buffer Definition
proto/wg/cosmo/platform/v1/platform.proto
Added new field composedSchemaBreakingChanges as a repeated FederatedGraphSchemaChange with field number 18 to UpdateProposalResponse message.
Generated TypeScript Code
connect/src/wg/cosmo/platform/v1/platform_pb.ts
Added public field composedSchemaBreakingChanges: FederatedGraphSchemaChange[] = [] to the UpdateProposalResponse class definition.
Service Implementation
controlplane/src/core/bufservices/proposal/updateProposal.ts
Added composedSchemaBreakingChanges field initialization as empty array across all response branches (early returns and success path), and propagates the field from SchemaCheckRepository.checkMultipleSchemas result into final UpdateProposalResponse payload.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding a new field 'composedSchemaBreakingChanges' to the 'UpdateProposalResponse' message, which is the primary modification across all affected files.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 11, 2026

Codecov Report

❌ Patch coverage is 60.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.38%. Comparing base (5131bba) to head (4e51fdb).
⚠️ Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
...ne/src/core/bufservices/proposal/updateProposal.ts 60.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2627      +/-   ##
==========================================
- Coverage   41.42%   38.38%   -3.04%     
==========================================
  Files         799      969     +170     
  Lines      118919   125253    +6334     
  Branches     9618     5419    -4199     
==========================================
- Hits        49259    48077    -1182     
- Misses      69298    75490    +6192     
- Partials      362     1686    +1324     
Files with missing lines Coverage Δ
...ne/src/core/bufservices/proposal/updateProposal.ts 74.59% <60.00%> (-0.46%) ⬇️

... and 318 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Mar 11, 2026

Router image scan passed

✅ No security vulnerabilities found in image:

ghcr.io/wundergraph/cosmo/router:sha-453b5829746bc0c1068782481161222f8f8f2b87

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
controlplane/src/core/bufservices/proposal/updateProposal.ts (1)

61-81: Consider extracting a helper for error responses to reduce duplication.

There are ~15 similar error response objects in this file. A helper function like createErrorResponse(code, details) would reduce duplication and make future field additions easier. This is a nice-to-have for a future cleanup.

♻️ Example helper function
function createErrorResponse(
  code: EnumStatusCode,
  details: string,
): PlainMessage<UpdateProposalResponse> {
  return {
    response: { code, details },
    breakingChanges: [],
    nonBreakingChanges: [],
    compositionErrors: [],
    checkId: '',
    lintWarnings: [],
    lintErrors: [],
    graphPruneWarnings: [],
    graphPruneErrors: [],
    compositionWarnings: [],
    lintingSkipped: false,
    graphPruningSkipped: false,
    checkUrl: '',
    composedSchemaBreakingChanges: [],
  };
}

Usage:

-      return {
-        response: {
-          code: EnumStatusCode.ERR_NOT_FOUND,
-          details: `Namespace ${req.namespace} not found`,
-        },
-        breakingChanges: [],
-        ...
-        composedSchemaBreakingChanges: [],
-      };
+      return createErrorResponse(
+        EnumStatusCode.ERR_NOT_FOUND,
+        `Namespace ${req.namespace} not found`,
+      );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@controlplane/src/core/bufservices/proposal/updateProposal.ts` around lines 61
- 81, There are many duplicated error response objects in updateProposal.ts
(e.g., the block returning a PlainMessage with response: { code:
EnumStatusCode.ERR_NOT_FOUND, details: `Namespace ${req.namespace} not found` })
— add a helper function (e.g., createErrorResponse(code: EnumStatusCode,
details: string): PlainMessage<UpdateProposalResponse>) and replace each
duplicated return with a call to createErrorResponse(...) so all fields
(breakingChanges, nonBreakingChanges, compositionErrors, etc.) are populated
consistently from one place; update uses in the updateProposal handler where
req.namespace and other checks occur.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@controlplane/src/core/bufservices/proposal/updateProposal.ts`:
- Around line 61-81: There are many duplicated error response objects in
updateProposal.ts (e.g., the block returning a PlainMessage with response: {
code: EnumStatusCode.ERR_NOT_FOUND, details: `Namespace ${req.namespace} not
found` }) — add a helper function (e.g., createErrorResponse(code:
EnumStatusCode, details: string): PlainMessage<UpdateProposalResponse>) and
replace each duplicated return with a call to createErrorResponse(...) so all
fields (breakingChanges, nonBreakingChanges, compositionErrors, etc.) are
populated consistently from one place; update uses in the updateProposal handler
where req.namespace and other checks occur.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6da51a4b-14e0-424e-8e10-f834fa239307

📥 Commits

Reviewing files that changed from the base of the PR and between e3e9803 and 4e51fdb.

⛔ Files ignored due to path filters (1)
  • connect-go/gen/proto/wg/cosmo/platform/v1/platform.pb.go is excluded by !**/*.pb.go, !**/gen/**
📒 Files selected for processing (3)
  • connect/src/wg/cosmo/platform/v1/platform_pb.ts
  • controlplane/src/core/bufservices/proposal/updateProposal.ts
  • proto/wg/cosmo/platform/v1/platform.proto

Copy link
Copy Markdown
Contributor

@StarpTech StarpTech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@JivusAyrus JivusAyrus merged commit 2375cd8 into main Mar 11, 2026
58 of 59 checks passed
@JivusAyrus JivusAyrus deleted the suvij/eng-9154-add-the-new-check-details-to-the-proposal-view branch March 11, 2026 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants