Skip to content

Conversation

@SkArchon
Copy link
Contributor

@SkArchon SkArchon commented Jan 27, 2026

This PR allows for the use of response client expressions, currently we only support header propagation from the subgraph level. Even for cases like setting a static value like "test" for a header on responses, this would run for every subgraph. The problem with this is that the requestContext's expression context has not been updated with information such as errors, response headers, etc. As this is processed after the subgraph level round trippers. (The use case was that the user wants to access the error attribute in expressions, which is not populated at this level).

In order to mitigate this we introduce client expressions, which runs once before returning the response.

Summary by CodeRabbit

  • New Features

    • Added client header rules to set/customize response headers sent to clients, supporting static values and dynamic expressions based on incoming requests.
    • Configuration and schema updated to allow declaring per-client header rules.
  • Tests

    • Added comprehensive tests for static/dynamic expressions, multiple headers, empty/invalid cases, precedence with other header rules, error handling, and logging.

✏️ Tip: You can customize this high-level summary in your review settings.

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.
  • Documentation has been updated on https://github.com/wundergraph/cosmo-docs.
  • I have read the Contributors Guide.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 27, 2026

Walkthrough

Adds client-side header propagation: new ClientHeaderRule config and schema, compiles and applies client header expressions in HeaderPropagation, integrates propagation into router and GraphQL handler, and adds tests exercising client header evaluation, precedence, canonicalization, and error/logging cases.

Changes

Cohort / File(s) Summary
Configuration structures & schema
router/pkg/config/config.go, router/pkg/config/config.schema.json, router/pkg/config/fixtures/full.yaml, router/pkg/config/testdata/config_defaults.json, router/pkg/config/testdata/config_full.json
Added ClientHeaderRule type and Client field on HeaderRules; added Expression to ResponseHeaderRule; updated JSON schema with client_header_rule and client property; updated fixtures and testdata to include client header examples.
Header rule engine & tests
router/core/header_rule_engine.go, router/core/header_rule_engine_test.go
Split compiled expressions into compiledRequestRules and compiledClientRules; getAllRules now returns client rules; added processExpression helper; added ApplyClientResponseHeaderRules method and tests validating client-rule evaluation, empty/missing handling, and error cases.
Router & handler integration
router/core/router.go, router/core/router_config.go, router/core/graph_server.go, router/core/graphql_handler.go
Wired HeaderPropagation into router and handler: added headerPropagation to Config, assigned it in NewRouter, added HeaderPropagation to HandlerOptions and GraphQLHandler, and invoke client header application in ServeHTTP.
End-to-end tests
router-tests/header_propagation_test.go
Added extensive request-level tests ("Client Header Rules") covering static and request-sourced expressions, interaction with request/response propagation, canonicalization, empty/missing headers, error handling, and runtime logging (zapcore).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: client response expressions' clearly and concisely summarizes the main change: introducing client-side expression evaluation for response headers, which is the primary feature across all modified files.

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

✨ Finishing touches
  • 📝 Generate docstrings

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

@github-actions
Copy link

github-actions bot commented Jan 27, 2026

Router image scan passed

✅ No security vulnerabilities found in image:

ghcr.io/wundergraph/cosmo/router:sha-45c582b61ac28ff431999b602e9188a6a2583669

@SkArchon SkArchon marked this pull request as ready for review January 27, 2026 14:36
@codecov
Copy link

codecov bot commented Jan 27, 2026

Codecov Report

❌ Patch coverage is 80.76923% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.87%. Comparing base (eca6aac) to head (22586d4).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
router/core/header_rule_engine.go 78.26% 5 Missing and 5 partials ⚠️

❌ Your patch check has failed because the patch coverage (80.76%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2472       +/-   ##
===========================================
+ Coverage   36.06%   61.87%   +25.81%     
===========================================
  Files         944      229      -715     
  Lines      123477    23889    -99588     
  Branches     4951        0     -4951     
===========================================
- Hits        44530    14782    -29748     
+ Misses      77386     7851    -69535     
+ Partials     1561     1256      -305     
Files with missing lines Coverage Δ
router/core/graph_server.go 82.47% <100.00%> (+2.81%) ⬆️
router/core/graphql_handler.go 70.37% <100.00%> (+0.44%) ⬆️
router/core/router.go 69.76% <100.00%> (+0.25%) ⬆️
router/core/router_config.go 93.67% <ø> (ø)
router/pkg/config/config.go 80.51% <ø> (ø)
router/core/header_rule_engine.go 88.09% <78.26%> (-0.69%) ⬇️

... and 732 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.

Copy link
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.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@router/core/header_rule_engine.go`:
- Around line 248-277: The response rule "expression" field is never compiled or
evaluated; update compileExpressionRules to also compile response rules (call
processExpression for each response rule and store programs in a
compiledResponseRules map on HeaderPropagation using the same expr.Manager), and
update applyResponseRule to evaluate the compiled program for the response rule
(look up hf.compiledResponseRules[rule.Expression] and execute via the expr VM
to decide rule application), or alternatively validate and reject any response
rules that contain an Expression by returning an error from
compileExpressionRules; reference functions: compileExpressionRules,
processExpression, applyResponseRule and the compiledResponseRules map on
HeaderPropagation when implementing the fix.
🧹 Nitpick comments (1)
router/core/router_config.go (1)

99-101: Include client header rules in usage tracking.

Now that client rules exist, Usage() can report header_rules as disabled when only headers.client is configured. Consider counting Client as well.

✅ Suggested update
- usage["header_rules"] = c.headerRules != nil && (c.headerRules.All != nil || len(c.headerRules.Subgraphs) > 0)
+ usage["header_rules"] = c.headerRules != nil &&
+ 	(c.headerRules.All != nil || len(c.headerRules.Subgraphs) > 0 || len(c.headerRules.Client) > 0)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants