feat: issue subgraph check extensions for proposals#2615
Merged
wilsonrivera merged 8 commits intomainfrom Mar 11, 2026
Merged
Conversation
Contributor
WalkthroughSchema check extension flow changed from single-subgraph to multi-subgraph: repositories now track per-subgraph Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
controlplane/src/core/webhooks/OrganizationWebhookService.ts (1)
754-761: Silent error suppression may hide webhook response parsing issues.The
try-catchblock silently swallows any error when converting the lintIssues record to a Map. If the webhook returns malformed data that passes Zod validation but fails during Map conversion, this could silently drop lint issues without any logging or indication to the caller.💡 Consider logging the error for debugging
if (typeof parsedResponse.data.lintIssues === 'object') { try { const lintIssuesRecord = parsedResponse.data.lintIssues as Record<string, LintIssueResult[]>; lintIssuesBySubgraph = new Map(Object.entries(lintIssuesRecord)); } catch { - // ignore + this.logger.debug('Failed to parse lintIssues from check extension response'); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controlplane/src/core/webhooks/OrganizationWebhookService.ts` around lines 754 - 761, The try-catch that converts parsedResponse.data.lintIssues to a Map in OrganizationWebhookService (the block that creates lintIssuesRecord and sets lintIssuesBySubgraph) currently swallows errors; modify the catch to log the caught error and relevant context (e.g., the malformed parsedResponse.data.lintIssues and identifying info like the webhook id or response metadata) using the existing logger so failures converting to Map aren't silent, while still preserving the fallback behavior; ensure the log message references lintIssuesBySubgraph and LintIssueResult so it’s easy to trace.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controlplane/src/core/repositories/SchemaCheckRepository.ts`:
- Around line 1327-1341: Replace the hardcoded isDeleted: false in the subgraph
mapping with the actual deletion flag from the check/subgraph object; when
constructing subgraphs from checkSubgraphs (the destructure [subgraphName, {
subgraph, ...check }]), set isDeleted to the real value (e.g.,
subgraph?.isDeleted ?? check?.isDeleted ?? false) so deleted subgraphs that
produced an empty newSchemaSDL are reported correctly in the webhook payload.
---
Nitpick comments:
In `@controlplane/src/core/webhooks/OrganizationWebhookService.ts`:
- Around line 754-761: The try-catch that converts
parsedResponse.data.lintIssues to a Map in OrganizationWebhookService (the block
that creates lintIssuesRecord and sets lintIssuesBySubgraph) currently swallows
errors; modify the catch to log the caught error and relevant context (e.g., the
malformed parsedResponse.data.lintIssues and identifying info like the webhook
id or response metadata) using the existing logger so failures converting to Map
aren't silent, while still preserving the fallback behavior; ensure the log
message references lintIssuesBySubgraph and LintIssueResult so it’s easy to
trace.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 02d4d278-01da-49bb-be37-81b7fc1f38ed
📒 Files selected for processing (4)
controlplane/src/core/repositories/SchemaCheckRepository.tscontrolplane/src/core/repositories/SubgraphRepository.tscontrolplane/src/core/webhooks/OrganizationWebhookService.tscontrolplane/test/subgraph-check-extensions.test.ts
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2615 +/- ##
==========================================
+ Coverage 62.79% 64.58% +1.78%
==========================================
Files 244 301 +57
Lines 25818 42876 +17058
Branches 0 4600 +4600
==========================================
+ Hits 16213 27690 +11477
- Misses 8223 15165 +6942
+ Partials 1382 21 -1361
🚀 New features to boost your workflow:
|
wilsonrivera
requested review from
JivusAyrus,
StarpTech and
thisisnithin
as code owners
…extensions-on-proposal
JivusAyrus
requested changes
Mar 10, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@controlplane/src/core/repositories/SchemaCheckRepository.ts`:
- Around line 1344-1348: The mapping that builds webhook payloads in
SchemaCheckRepository currently uses subgraph?.labels ?? [] which drops labels
for new proposal subgraphs where subgraph is undefined; update the mapping over
checkSubgraphs (the map that creates objects with id, name, labels, schemaSDL)
to use check.labels as the fallback (e.g., labels: subgraph?.labels ??
check.labels ?? []) so new subgraphs preserve their labels in the webhook
payload.
- Around line 1360-1385: The loop that processes sceResult.lintIssuesBySubgraph
only appends to per-subgraph check.lintIssues but doesn't add those issues into
the aggregate lintWarnings/lintErrors or flip hasLintErrors; update the loop
inside SchemaCheckRepository (the block iterating checkSubgraphs) to also push
sceLintIssues into the top-level lintWarnings and lintErrors arrays (filter by
LintSeverity.warn/error) and/or set a local flag so that when calling
this.update(...) the hasLintErrors value reflects any extension-reported
error-severity issues; keep the existing call to
schemaLintRepo.addSchemaCheckLintIssues(checkSubgraphId) intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e4daf383-6b5f-42e5-a221-a5db5c6ca574
📒 Files selected for processing (2)
controlplane/src/core/repositories/SchemaCheckRepository.tscontrolplane/src/core/repositories/SubgraphRepository.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
controlplane/src/core/repositories/SchemaCheckRepository.ts (1)
1360-1381: Redundant filtering detected.Lines 1364-1365 create
sceLintWarningsandsceLintErrorsby filteringsceLintIssues, but lines 1367-1368 re-filter the same array instead of using the already-filtered variables. Use the pre-filtered variables for consistency and efficiency.♻️ Proposed fix
const sceLintWarnings = sceLintIssues.filter((issue) => issue.severity === LintSeverity.warn); const sceLintErrors = sceLintIssues.filter((issue) => issue.severity === LintSeverity.error); - check.lintIssues.warnings.push(...sceLintIssues.filter((issue) => issue.severity === LintSeverity.warn)); - check.lintIssues.errors.push(...sceLintIssues.filter((issue) => issue.severity === LintSeverity.error)); + check.lintIssues.warnings.push(...sceLintWarnings); + check.lintIssues.errors.push(...sceLintErrors); lintWarnings.push(...sceLintWarnings.map((issue) => new LintIssue({ ...issue, subgraphName }))); lintErrors.push(...sceLintErrors.map((issue) => new LintIssue({ ...issue, subgraphName })));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@controlplane/src/core/repositories/SchemaCheckRepository.ts` around lines 1360 - 1381, In SchemaCheckRepository inside the loop over checkSubgraphs, avoid re-filtering sceLintIssues — use the precomputed arrays sceLintWarnings and sceLintErrors instead: replace the push calls that currently do check.lintIssues.warnings.push(...sceLintIssues.filter(...)) and check.lintIssues.errors.push(...sceLintIssues.filter(...)) to push the already-filtered sceLintWarnings and sceLintErrors variables; keep the mappings that create new LintIssue instances from sceLintWarnings/sceLintErrors and leave schemaLintRepo.addSchemaCheckLintIssues as-is.
🤖 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/repositories/SchemaCheckRepository.ts`:
- Around line 1360-1381: In SchemaCheckRepository inside the loop over
checkSubgraphs, avoid re-filtering sceLintIssues — use the precomputed arrays
sceLintWarnings and sceLintErrors instead: replace the push calls that currently
do check.lintIssues.warnings.push(...sceLintIssues.filter(...)) and
check.lintIssues.errors.push(...sceLintIssues.filter(...)) to push the
already-filtered sceLintWarnings and sceLintErrors variables; keep the mappings
that create new LintIssue instances from sceLintWarnings/sceLintErrors and leave
schemaLintRepo.addSchemaCheckLintIssues as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0105869d-5c6c-4958-b6ca-4b5c6965f627
📒 Files selected for processing (1)
controlplane/src/core/repositories/SchemaCheckRepository.ts
JivusAyrus
approved these changes
Mar 11, 2026
…extensions-on-proposal
wilsonrivera
deleted the
wilson/eng-9071-investigation-missing-check-extensions-on-proposal
branch
5 tasks
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Checklist