fix: incorrect alias behavior for required fields

[Noroth]

Aliases are not correctly taken into accoutn when using required fields. This PR fixes the behavior.

@coderabbitai summary

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.

Open Source AI Manifesto

This project follows the principles of the Open Source AI Manifesto. Please ensure your contribution aligns with its principles.

[coderabbitai]

📝 Walkthrough

Walkthrough

Federation @requires planning now tracks required fields by response key, clones entries for repeated selections under aliases, and builds RPC response paths from GraphQL aliases or paths. Planner and integration tests cover aliased-only and plain-plus-aliased selections.

Changes

Federation Requires Aliases

Layer / File(s)	Summary
Per-response-key required-field binding v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go	The federation visitor reuses matching required-field entries or clones definitions when the same required field is selected under different response keys.
Aliased required-call generation and validation v2/pkg/engine/datasource/grpc_datasource/execution_plan.go, v2/pkg/engine/datasource/grpc_datasource/*_test.go	Required-field RPC response paths use AliasOrPath(), with tests covering aliased-only and combined plain-and-aliased selections and returned entity values.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• wundergraph/graphql-go-tools#1452: Both changes modify federation @requires execution planning and required-field RPC construction.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: fixing alias handling for required fields.
Description check	✅ Passed	The description is directly related to the PR and explains the alias-related required-field fix.
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.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ludwig/router-581-engine-alias-on-required-fields-is-misbehaving

---

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

[claude]

Claude Code Review

Claude Code Review is paused for this repository. To reconnect it, an admin of this repository's GitHub organization (or the account owner, for personal repositories) who can also manage your Claude organization's Code Review settings needs to re-link GitHub in Code Review settings. This is a one-time step.

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go (1)

440-467: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Cloned entry's fields get overwritten by stale data — requiredField local var never repointed to the clone.

In the else branch (append-clone path), requiredField.clone() is appended into config.requiredFields, but the local requiredField variable is never reassigned to that clone — it still holds the previously-bound entry. The subsequent lines (requiredField.ref = ref, .fieldDefRef = fieldDefRef, .resultField = field) mutate this stale value, and fieldArguments is only overwritten when the current occurrence has arguments (len(fieldArgs) > 0). If the current (aliased) occurrence has no arguments while the previously-bound entry did, the stale fieldArguments leaks into the newly appended entry at line 467, silently overwriting the intentionally-cleared clone.

Example: nullableFilteredTagSummary(prefix: $prefix) plus an aliased selection of the same field without the argument would send the wrong (stale) prefix argument for the aliased required-fields call.

🐛 Proposed fix

 		} else {
 			index = len(config.requiredFields)
-			config.requiredFields = append(config.requiredFields, requiredField.clone())
+			requiredField = requiredField.clone()
+			config.requiredFields = append(config.requiredFields, requiredField)
 			r.entityConfig.setEntity(r.entityInfo.typeName, config)
 		}

Consider adding a regression test (e.g. in execution_plan_requires_test.go) combining an aliased selection with a field that has optional arguments, to catch this class of bug.

🤖 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
`@v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go`
around lines 440 - 467, In the clone branch of the required-field handling
logic, repoint the requiredField local variable to the newly appended clone and
update index accordingly before mutating ref, fieldDefRef, resultField, and
fieldArguments. Ensure each response-key entry starts with the clone’s cleared
argument state when the current occurrence has no arguments, preventing stale
arguments from the previously bound entry from being copied.

🤖 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.

Outside diff comments:
In
`@v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go`:
- Around line 440-467: In the clone branch of the required-field handling logic,
repoint the requiredField local variable to the newly appended clone and update
index accordingly before mutating ref, fieldDefRef, resultField, and
fieldArguments. Ensure each response-key entry starts with the clone’s cleared
argument state when the current occurrence has no arguments, preventing stale
arguments from the previously bound entry from being copied.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b6cac6bd-a5ec-476f-ba15-d06bf8261d65

📥 Commits

Reviewing files that changed from the base of the PR and between dd73327 and 0cf6a47.

📒 Files selected for processing (4)

• v2/pkg/engine/datasource/grpc_datasource/execution_plan.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan_requires_test.go
• v2/pkg/engine/datasource/grpc_datasource/execution_plan_visitor_federation.go
• v2/pkg/engine/datasource/grpc_datasource/grpc_datasource_federation_test.go