Skip to content

feat: return additional linked check info when proposals are created/updated#2258

Merged
JivusAyrus merged 3 commits intomainfrom
suvij/eng-8123-update-the-ui-of-check-on-the-hub
Oct 6, 2025
Merged

feat: return additional linked check info when proposals are created/updated#2258
JivusAyrus merged 3 commits intomainfrom
suvij/eng-8123-update-the-ui-of-check-on-the-hub

Conversation

@JivusAyrus
Copy link
Copy Markdown
Member

@JivusAyrus JivusAyrus commented Oct 2, 2025

Summary by CodeRabbit

  • New Features

    • Added a new indicator showing whether linked schema checks are present in schema check results and proposal workflows.
    • Create Proposal and Update Proposal responses now include a flag to signal if linked subgraph checks were evaluated.
    • Check summaries and schema fix responses display the presence of linked schema validations for clearer review context.
  • API

    • Response payloads for proposal and schema check operations now include a boolean flag for linked schema checks.

Checklist

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Oct 2, 2025

Walkthrough

Adds an optional boolean field hasLinkedSchemaChecks across proto definitions, generated TypeScript, repository return payloads, and proposal service responses. Repository sets it based on presence of linked subgraphs; proposal create/update propagate it. No control-flow changes; updates are additive to serialization and response structures.

Changes

Cohort / File(s) Summary of changes
Protocol and generated types
proto/wg/cosmo/platform/v1/platform.proto, connect/src/wg/cosmo/platform/v1/platform_pb.ts
Introduced optional boolean hasLinkedSchemaChecks in CreateProposalResponse and UpdateProposalResponse (proto) and added corresponding optional fields to TypeScript message types (including CheckSubgraphSchemaResponse, FixSubgraphSchemaResponse, GetCheckSummaryResponse, UpdateProposalResponse).
Repository layer
controlplane/src/core/repositories/SchemaCheckRepository.ts
checkMultipleSchemas now returns hasLinkedSchemaChecks set to true when linkedSubgraphs.length > 0; otherwise false. Signature extended to include this field.
Proposal services
controlplane/src/core/bufservices/proposal/createProposal.ts, controlplane/src/core/bufservices/proposal/updateProposal.ts
Destructure hasLinkedSchemaChecks from schemaCheckRepo.checkMultipleSchemas and include it in CreateProposalResponse and UpdateProposalResponse payloads.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Pre-merge checks and finishing touches

❌ 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%. You can run `@coderabbitai generate docstrings` to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit's high-level summary is enabled.
Title Check ✅ Passed The title clearly and concisely summarizes the primary change—adding linked check information when proposals are created or updated—using conventional commit style and accurately reflecting the diff without unnecessary detail.
✨ Finishing touches
  • 📝 Generate Docstrings

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7e9912d and 004289d.

⛔ 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 (5)
  • connect/src/wg/cosmo/platform/v1/platform_pb.ts (4 hunks)
  • controlplane/src/core/bufservices/proposal/createProposal.ts (2 hunks)
  • controlplane/src/core/bufservices/proposal/updateProposal.ts (2 hunks)
  • controlplane/src/core/repositories/SchemaCheckRepository.ts (1 hunks)
  • proto/wg/cosmo/platform/v1/platform.proto (2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-08-31T18:51:32.185Z
Learnt from: JivusAyrus
PR: wundergraph/cosmo#2156
File: controlplane/src/core/bufservices/check/getCheckSummary.ts:0-0
Timestamp: 2025-08-31T18:51:32.185Z
Learning: In the SchemaCheckRepository.getLinkedSchemaCheck method, organization-level security is implemented through post-query validation by checking `check.subgraphs[0].namespace.organizationId !== organizationId` and returning undefined if the linked check doesn't belong to the caller's organization, preventing cross-tenant data leakage.

Applied to files:

  • controlplane/src/core/bufservices/proposal/updateProposal.ts
  • controlplane/src/core/bufservices/proposal/createProposal.ts
📚 Learning: 2025-08-29T10:10:06.969Z
Learnt from: JivusAyrus
PR: wundergraph/cosmo#2156
File: studio/src/pages/[organizationSlug]/[namespace]/graph/[slug]/checks/[checkId]/index.tsx:0-0
Timestamp: 2025-08-29T10:10:06.969Z
Learning: The isCheckSuccessful function in both controlplane/src/core/util.ts and studio/src/components/check-badge-icon.tsx includes early exits for linked check validation. It returns false immediately if either isLinkedTrafficCheckFailed or isLinkedPruningCheckFailed is true, which means linked check failures automatically cause the overall check to fail.

Applied to files:

  • controlplane/src/core/bufservices/proposal/updateProposal.ts
  • controlplane/src/core/bufservices/proposal/createProposal.ts
📚 Learning: 2025-09-08T20:57:07.946Z
Learnt from: JivusAyrus
PR: wundergraph/cosmo#2156
File: controlplane/src/core/repositories/SubgraphRepository.ts:1746-1763
Timestamp: 2025-09-08T20:57:07.946Z
Learning: The checkSubgraphSchema.ts file already correctly implements linked subgraph functionality, using byName(linkedSubgraph.name, linkedSubgraph.namespace) to fetch target subgraphs and properly handles parse(newSchemaSDL) for schema building. The implementation doesn't need fixes for byId usage or schema parsing as it's already correct.

Applied to files:

  • controlplane/src/core/repositories/SchemaCheckRepository.ts
🧬 Code graph analysis (1)
controlplane/src/core/repositories/SchemaCheckRepository.ts (1)
controlplane/src/db/schema.ts (1)
  • linkedSubgraphs (249-276)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
  • GitHub Check: build-router
  • GitHub Check: build_push_image
  • GitHub Check: build_test
  • GitHub Check: build_push_image
  • GitHub Check: build_test
  • GitHub Check: image_scan
  • GitHub Check: build_push_image (nonroot)
  • GitHub Check: image_scan (nonroot)
  • GitHub Check: integration_test (./telemetry)
  • GitHub Check: build_test
  • GitHub Check: build_push_image
  • GitHub Check: integration_test (./. ./fuzzquery ./lifecycle ./modules)
  • GitHub Check: integration_test (./events)
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
proto/wg/cosmo/platform/v1/platform.proto (1)

2762-2762: LGTM! Field additions follow protobuf conventions.

The new hasLinkedSchemaChecks fields are correctly defined as optional booleans with appropriate field numbers (20 for CreateProposalResponse, 17 for UpdateProposalResponse). The placement is logical, grouped with related linked check fields.

Also applies to: 2841-2841

controlplane/src/core/repositories/SchemaCheckRepository.ts (1)

1168-1168: LGTM! Clean and correct implementation.

The hasLinkedSchemaChecks field is set based on whether any linked subgraphs were discovered during the check (linkedSubgraphs.length > 0). This is consistent with the pattern used for other linked check indicators.

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

445-445: LGTM! Proper propagation of linked check metadata.

The hasLinkedSchemaChecks field is correctly destructured from the schema check result and included in the response payload, following the same pattern as isLinkedTrafficCheckFailed and isLinkedPruningCheckFailed.

Also applies to: 497-497

controlplane/src/core/bufservices/proposal/createProposal.ts (1)

401-401: LGTM! Consistent with update proposal flow.

The hasLinkedSchemaChecks field is correctly destructured and propagated, maintaining consistency with the update proposal implementation and other linked check indicators.

Also applies to: 456-456

connect/src/wg/cosmo/platform/v1/platform_pb.ts (4)

21619-21622: Addition aligns response with linked-check flag propagation

Optional hasLinkedSchemaChecks matches the new proto definition (field 20) so clients can consume the repository flag without breaking existing payloads.


21648-21651: Descriptor update keeps serialization consistent

Including field 20 with opt: true ensures wire compatibility with the new optional bool—no issues spotted.


22219-22222: Update response mirrors create response

The optional property is added with the correct field number (17), maintaining symmetry with the proto change.


22245-22248: Field table entry is correctly appended

Serialization metadata now exposes field 17 as optional bool; ordering remains intact.


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

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Oct 2, 2025

Router-nonroot image scan passed

✅ No security vulnerabilities found in image:

ghcr.io/wundergraph/cosmo/router:sha-34d708370a7787589c1ce97259da93624a227731-nonroot

Copy link
Copy Markdown
Contributor

@wilsonrivera wilsonrivera left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Member

@Aenimus Aenimus 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 81ac616 into main Oct 6, 2025
43 of 44 checks passed
@JivusAyrus JivusAyrus deleted the suvij/eng-8123-update-the-ui-of-check-on-the-hub branch October 6, 2025 09:58
@coderabbitai coderabbitai Bot mentioned this pull request Nov 25, 2025
5 tasks
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.

4 participants