Skip to content

feat(router): support dot-path slicingArguments - #2831

Closed
ysmolski wants to merge 3 commits into
mainfrom
yury/eng-9338-router-only-support-dot-path-in-slicingarguments
Closed

feat(router): support dot-path slicingArguments#2831
ysmolski wants to merge 3 commits into
mainfrom
yury/eng-9338-router-only-support-dot-path-in-slicingarguments

Conversation

@ysmolski

@ysmolski ysmolski commented May 6, 2026

Copy link
Copy Markdown
Contributor

See #2801 for details.

This PR merely adds a verifying test and uses the engine with support of this feature.

This PR should released before the composition.

Summary by CodeRabbit

  • New Features

    • Added product search with query parameters and pagination.
    • Search endpoint enforces a maximum result limit (1000 items).
  • Tests

    • Added test coverage for cost estimation using nested input paths.
  • Chores

    • Updated a dependency version for stability and reliability.

See #2801 for details.

This PR merely adds a verifying test and uses the engine with support of
this feature.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@coderabbitai

coderabbitai Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

Adds a product search API (schema, generated models, resolver) with pagination to the products subgraph, updates graphql-go-tools/v2 dependency in two modules, and adds a router test validating cost estimation for nested input literals.

Changes

Product Search Feature

Layer / File(s) Summary
Data Shape
demo/pkg/subgraphs/products/subgraph/model/models_gen.go
Added ProductSearchInput (Pagination, Query) and ProductSearchPagination (First, After).
Schema Definition
demo/pkg/subgraphs/products/subgraph/schema.graphqls
Added searchThings(input: ProductSearchInput!): [Thing!]! and input types ProductSearchPagination and ProductSearchInput.
Resolver Implementation
demo/pkg/subgraphs/products/subgraph/schema.resolvers.go
Added SearchThings(ctx, input model.ProductSearchInput) ([]*model.Thing, error) on queriesResolver; enforces max size 1000 and constructs the requested list of Thing items.

Dependency & Cost Estimation Update

Layer / File(s) Summary
Dependency Update
router/go.mod, router-tests/go.mod
Updated github.com/wundergraph/graphql-go-tools/v2 from v2.1.0 to v2.1.1-0.20260506124225-15b6928c79f6.
Tests
router-tests/security/costs_test.go
Added test "dot-path slicingArgument resolves nested input literal" asserting cost headers (estimated and actual) equal 7 for a nested input literal case.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 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 (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(router): support dot-path slicingArguments' accurately describes the main change - adding support for dot-path slicing arguments in the router, which is demonstrated by the new test case and dependency updates.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.1)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


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

@github-actions github-actions Bot added the router label May 6, 2026
@ysmolski ysmolski changed the title feat(router): support dot-path slicingArguments in router feat(router): support dot-path slicingArguments May 6, 2026
@github-actions

github-actions Bot commented May 6, 2026

Copy link
Copy Markdown

Router-nonroot image scan passed

✅ No security vulnerabilities found in image:

ghcr.io/wundergraph/cosmo/router:sha-28ab5e96ee96527006a9ddb11302cb2af20a7ed1-nonroot

@codecov

codecov Bot commented May 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.57%. Comparing base (42b21fc) to head (82a4444).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2831       +/-   ##
===========================================
+ Coverage   46.23%   55.57%    +9.34%     
===========================================
  Files        1080      238      -842     
  Lines      144704    25946   -118758     
  Branches     9247        0     -9247     
===========================================
- Hits        66897    14419    -52478     
+ Misses      76077    10057    -66020     
+ Partials     1730     1470      -260     

see 968 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.

@ysmolski

ysmolski commented May 6, 2026

Copy link
Copy Markdown
Contributor Author

i will have to make it together with the compsition, it looks like.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
router/go.mod (1)

192-192: 💤 Low value

Potential security concern: go.opentelemetry.io/otel/sdk pinned below recommended version.

The replace directive pins otel/sdk to v1.28.0, which is below the recommended v1.40.0+ threshold for CVE-2026-24051 (PATH hijacking on macOS/Darwin). While the comment indicates this is intentional to preserve attribute naming, verify this risk is acceptable for your deployment environment.

Based on learnings: "In wundergraph/cosmo, google.golang.org/grpc v1.79.3 can transitively pull a vulnerable go.opentelemetry.io/otel/sdk version (GHSA-9h8m-3fm2-qjrq / CVE-2026-24051, PATH hijacking on macOS/Darwin). In every module go.mod, explicitly pin go.opentelemetry.io/otel/sdk to v1.40.0 or later."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@router/go.mod` at line 192, The replace directive currently pins the module
go.opentelemetry.io/otel/sdk to v1.28.0 which is below the recommended safe
threshold; update the replace to pin go.opentelemetry.io/otel/sdk to v1.40.0 or
later (or remove the replace if not required) and then run go mod tidy to
refresh transitive deps; if you must keep v1.28.0 to preserve attribute naming,
add a short justification comment and document the risk acceptance for
CVE-2026-24051 and validate on macOS/Darwin that no vulnerable code paths exist
(and consider adding a TODO to revisit the pin once attribute compatibility is
resolved).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@router/go.mod`:
- Line 192: The replace directive currently pins the module
go.opentelemetry.io/otel/sdk to v1.28.0 which is below the recommended safe
threshold; update the replace to pin go.opentelemetry.io/otel/sdk to v1.40.0 or
later (or remove the replace if not required) and then run go mod tidy to
refresh transitive deps; if you must keep v1.28.0 to preserve attribute naming,
add a short justification comment and document the risk acceptance for
CVE-2026-24051 and validate on macOS/Darwin that no vulnerable code paths exist
(and consider adding a TODO to revisit the pin once attribute compatibility is
resolved).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 95398890-0f43-4e4f-97c1-9f207d6f7684

📥 Commits

Reviewing files that changed from the base of the PR and between 8331e81 and 60f5d86.

⛔ Files ignored due to path filters (3)
  • demo/pkg/subgraphs/products/subgraph/generated/generated.go is excluded by !**/generated/**
  • router-tests/go.sum is excluded by !**/*.sum
  • router/go.sum is excluded by !**/*.sum
📒 Files selected for processing (7)
  • demo/pkg/subgraphs/products/subgraph/model/models_gen.go
  • demo/pkg/subgraphs/products/subgraph/schema.graphqls
  • demo/pkg/subgraphs/products/subgraph/schema.resolvers.go
  • router-tests/go.mod
  • router-tests/security/costs_test.go
  • router-tests/testenv/testdata/config.json
  • router/go.mod

@ysmolski
ysmolski marked this pull request as draft May 7, 2026 11:25
@ysmolski ysmolski closed this May 12, 2026
@ysmolski
ysmolski deleted the yury/eng-9338-router-only-support-dot-path-in-slicingarguments branch May 12, 2026 14:01
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.

1 participant