chore(deps): update graphql-go-tools to fix SIGSEGV in extension filtering

[jensneuse]

Summary

Fixes ENG-8985

• Update graphql-go-tools to commit c96ac9e6f088 to fix mutation-during-iteration SIGSEGV in optionallyAllowCustomExtensionProperties

Upstream PR: wundergraph/graphql-go-tools#1402

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Expanded test coverage for extension-field filtering across wrapped and passthrough modes with many focused subtests.
  • Added scenarios ensuring only permitted extension fields are propagated and disallowed fields are omitted per configuration.
  • Introduced concurrent stress tests to verify thread-safety and consistent behavior under parallel execution with synchronization checks.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Added tests in router-tests/error_handling_test.go validating extension-field filtering for wrapped and passthrough error modes, AllowAllExtensionFields and multi-field combinations, plus a concurrent stress test; only test code changed, no production APIs modified.

Changes

Cohort / File(s)	Summary
Test Coverage Expansion router-tests/error_handling_test.go	Added many subtests to TestAllowedExtensions covering wrapped vs passthrough modes, AllowAllExtensionFields, multi-field combinations, and a concurrent thread-safety stress test. Imported sync and used sync.WaitGroup and a trigger channel. Test-only changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The PR title describes a dependency update to fix a SIGSEGV bug, but the actual changeset only adds test cases for error handling and extension field filtering with no visible dependency updates.	Update the title to reflect the primary change: 'test(router): add comprehensive error handling tests for extension field filtering' or similar to accurately describe the test additions in the changeset.
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 (1 passed)

Check name	Status	Explanation
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.}

---

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

[github-actions]

❌ Internal Query Planner CI checks failed

The Internal Query Planner CI checks failed in the celestial repository, and this is going to stop the merge of this PR.
If you are part of the WunderGraph organization, you can see the PR with more details.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.70%. Comparing base (fbed362) to head (fb90827).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2541      +/-   ##
==========================================
+ Coverage   61.00%   61.70%   +0.69%     
==========================================
  Files         239      239              
  Lines       25423    25423              
==========================================
+ Hits        15510    15687     +177     
+ Misses       8589     8417     -172     
+ Partials     1324     1319       -5     

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

[StarpTech]

Lgtm

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

router-tests/error_handling_test.go (1)

413-423: Prefer sync.WaitGroup.Go for goroutine bookkeeping (Go 1.25+).
This keeps Add/Done management consistent with repo guidance and reduces manual bookkeeping.

♻️ Proposed refactor

-			done.Add(numOfOperations)
+			done.Add(numOfOperations)
 			trigger := make(chan struct{})
 			for i := 0; i < numOfOperations; i++ {
-				go func() {
-					ready.Done()
-					defer done.Done()
-					<-trigger
-					res := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{
-						Query: `{ employees { id } }`,
-					})
-					require.Equal(t, `{"errors":[{"message":"Unauthorized","extensions":{"code":"UNAUTHORIZED","statusCode":403}}],"data":{"employees":null}}`, res.Body)
-				}()
+				done.Go(func() {
+					ready.Done()
+					<-trigger
+					res := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{
+						Query: `{ employees { id } }`,
+					})
+					require.Equal(t, `{"errors":[{"message":"Unauthorized","extensions":{"code":"UNAUTHORIZED","statusCode":403}}],"data":{"employees":null}}`, res.Body)
+				})
 			}

Based on learnings: In Go code (Go 1.25+), prefer using sync.WaitGroup.Go(func()) to run a function in a new goroutine, letting the WaitGroup manage Add/Done automatically. Avoid manual wg.Add(1) followed by go func() { defer wg.Done(); ... }() patterns. Apply this guidance across all Go files in the wundergraph/cosmo repository where concurrency is used.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@router-tests/error_handling_test.go` around lines 413 - 423, Replace the
manual goroutine launch so the done WaitGroup manages goroutine bookkeeping via
WaitGroup.Go; keep ready.Add(numOfOperations) (so each goroutine still calls
ready.Done() to signal readiness) but remove done.Add(numOfOperations) and the
plain go func() call — instead call done.Go(func() { ready.Done(); defer
done.Done(); /* original goroutine body */ }) so the done WaitGroup
auto-increments/decrements and you no longer manually call done.Add/Done around
the goroutine start.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@router-tests/error_handling_test.go`:
- Around line 421-429: The test spawns a goroutine that calls require.Equal
(which calls FailNow and may panic outside the main test goroutine); instead
have the goroutine send the response body (from xEnv.MakeGraphQLRequestOK with
testenv.GraphQLRequest) over a channel (e.g., respCh) after ready/done
coordination (ready.Done(), defer done.Done(), <-trigger), and move the
assertion into the parent goroutine where you receive from respCh and call
require.Equal(t, expected, received.Body); keep use of ready, done, trigger, and
xEnv.MakeGraphQLRequestOK/testenv.GraphQLRequest/res.Body to locate the code to
change.

---

Nitpick comments:
In `@router-tests/error_handling_test.go`:
- Around line 413-423: Replace the manual goroutine launch so the done WaitGroup
manages goroutine bookkeeping via WaitGroup.Go; keep ready.Add(numOfOperations)
(so each goroutine still calls ready.Done() to signal readiness) but remove
done.Add(numOfOperations) and the plain go func() call — instead call
done.Go(func() { ready.Done(); defer done.Done(); /* original goroutine body */
}) so the done WaitGroup auto-increments/decrements and you no longer manually
call done.Add/Done around the goroutine start.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

router-tests/error_handling_test.go (1)

494-512: Use sync.WaitGroup.Go for goroutine launch.
The repository uses Go 1.25, which provides WaitGroup.Go that automatically manages goroutine lifecycle. This simplifies the done waitgroup while keeping ready as-is.

♻️ Suggested refactor

-            ready.Add(numOfOperations)
-            done.Add(numOfOperations)
+            ready.Add(numOfOperations)
             trigger := make(chan struct{})
             for i := 0; i < numOfOperations; i++ {
-                go func() {
-                    ready.Done()
-                    defer done.Done()
-                    <-trigger
-                    res := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{
-                        Query: `{ employees { id } }`,
-                    })
-                    results <- res.Body
-                }()
+                done.Go(func() {
+                    ready.Done()
+                    <-trigger
+                    res := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{
+                        Query: `{ employees { id } }`,
+                    })
+                    results <- res.Body
+                })
             }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@router-tests/error_handling_test.go` around lines 494 - 512, Replace the
manual goroutine lifecycle management using the done WaitGroup with
WaitGroup.Go: keep numOfOperations and ready as-is (ready.Add(numOfOperations)),
remove the done variable and its Add/Done/Wait usage, introduce a new waitgroup
(e.g., wg := sync.WaitGroup{}), and launch each worker with wg.Go(func() {
ready.Done(); <-trigger; res :=
xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{ Query: `{ employees { id } }`,
}); results <- res.Body }); then call wg.Wait() after closing the trigger; this
uses WaitGroup.Go to auto-manage goroutine completion and drops the explicit
defer done.Done.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@router-tests/error_handling_test.go`:
- Around line 349-353: The HTTP handler goroutines in the test use
require.NoError (inside the http.HandlerFunc closure) which can call FailNow and
panic when invoked off the main test goroutine; replace those require.NoError
calls with non-fatal reporting (e.g., t.Error or t.Errorf) or send errors back
to the main test goroutine via a channel and assert there; update every
occurrence of require.NoError inside the handler closures (the http.HandlerFunc
in this test) to use t.Error*/channel reporting so failures are reported safely
from the main test goroutine.

---

Nitpick comments:
In `@router-tests/error_handling_test.go`:
- Around line 494-512: Replace the manual goroutine lifecycle management using
the done WaitGroup with WaitGroup.Go: keep numOfOperations and ready as-is
(ready.Add(numOfOperations)), remove the done variable and its Add/Done/Wait
usage, introduce a new waitgroup (e.g., wg := sync.WaitGroup{}), and launch each
worker with wg.Go(func() { ready.Done(); <-trigger; res :=
xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{ Query: `{ employees { id } }`,
}); results <- res.Body }); then call wg.Wait() after closing the trigger; this
uses WaitGroup.Go to auto-manage goroutine completion and drops the explicit
defer done.Done.