fix: send auth and x-user when validating change reason

[Datron]

Problem

Change reason function is being redirected to login page because of missing x-user and auth token

Solution

Add x-user and auth token

Summary by CodeRabbit

• Refactor
  • Updated change-reason validation across experiment and experiment group operations to include authenticated user context. This affects create, update, add/remove members operations for groups and create, conclude, discard, ramp, update overrides, pause, and resume operations for experiments.

[semanticdiff-com]

Review changes with  

Changed Files

File	Status
crates/experimentation_platform/src/api/experiment_groups/handlers.rs	0% smaller
crates/experimentation_platform/src/api/experiments/handlers.rs	0% smaller
crates/experimentation_platform/src/api/experiments/helpers.rs	0% smaller

[coderabbitai]

Walkthrough

The PR updates change-reason validation to include authenticated user context. A helper function is renamed and refactored to accept a user parameter, serialize it into an x-user header, and pass it to the CAC validation endpoint. All experiment and experiment-group handlers are updated to pass the authenticated user when calling this validation function.

Changes

User Context in Change-Reason Validation

Layer / File(s)	Summary
Helper Function Refactor crates/experimentation_platform/src/api/experiments/helpers.rs	fetch_and_validate_change_reason_with_function renamed to validate_change_reason_with_function; function now requires user: &User parameter and serializes user into x-user header for CAC validation requests.
Experiment Handlers Update crates/experimentation_platform/src/api/experiments/handlers.rs	Seven handlers (create, conclude, discard, ramp, update, pause, resume) now pass authenticated user to validate_change_reason_with_function.
Experiment Group Handlers Update crates/experimentation_platform/src/api/experiment_groups/handlers.rs	Import updated; four handlers (create, update, add_members, remove_members) now pass authenticated user to validate_change_reason_with_function.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested reviewers

• sauraww
• ayushjain17

Poem

🐰 A user hops into the validation flow,
Their context now travels where validations go,
Headers are serialized, endpoints now see,
Who made the change—transparency! 🌟

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly summarizes the main change: adding auth and x-user header to change reason validation function calls across multiple handlers.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-change-reason-fn

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

crates/experimentation_platform/src/api/experiments/helpers.rs (1)

806-820: ⚡ Quick win

Check HTTP status before deserializing the response body.

res.status() is never inspected. If CAC returns a 5xx (e.g., the function endpoint is temporarily unavailable), the code tries to parse the error body as FunctionExecutionResponse, fails, and surfaces bad_argument!("Change reason validation failed.") to the caller — misleading them into thinking their change reason text is invalid when the real issue is a server-side problem. The correct behavior for a non-2xx response is unexpected_error!.

♻️ Proposed fix

     match response {
-        Ok(res) => match res.json::<FunctionExecutionResponse>().await {
-            Ok(response) => {
-                log::info!("Change reason validation function response: {:?}", response);
-                Ok(())
-            }
-            Err(err) => {
-                log::error!(
-                    "Change reason validation function returned false for change reason: {:?} with error: {:?}",
-                    change_reason,
-                    err
-                );
-                Err(bad_argument!("Change reason validation failed."))
-            }
-        },
+        Ok(res) => {
+            let status = res.status();
+            if !status.is_success() {
+                let body = res.text().await.unwrap_or_default();
+                log::error!(
+                    "Change reason validation endpoint returned {} for change reason: {:?}, body: {}",
+                    status,
+                    change_reason,
+                    body
+                );
+                return Err(unexpected_error!(
+                    "Change reason validation endpoint returned unexpected status: {}",
+                    status
+                ));
+            }
+            match res.json::<FunctionExecutionResponse>().await {
+                Ok(response) => {
+                    log::info!("Change reason validation function response: {:?}", response);
+                    Ok(())
+                }
+                Err(err) => {
+                    log::error!(
+                        "Change reason validation function returned false for change reason: {:?} with error: {:?}",
+                        change_reason,
+                        err
+                    );
+                    Err(bad_argument!("Change reason validation failed."))
+                }
+            }
+        },
         Err(error) => {

🤖 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 `@crates/experimentation_platform/src/api/experiments/helpers.rs` around lines
806 - 820, The current match arm deserializes
res.json::<FunctionExecutionResponse>().await without checking res.status(),
which makes server errors (5xx/4xx) get misreported as bad_argument! for the
change_reason; update the logic in the match handling of response to first
inspect res.status(). If res.status().is_success() then parse
res.json::<FunctionExecutionResponse>().await and proceed as now; otherwise log
the status and body and return unexpected_error! (instead of bad_argument!) to
surface server-side failures related to the FunctionExecutionResponse call for
change_reason validation.

🤖 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 `@crates/experimentation_platform/src/api/experiments/helpers.rs`:
- Around line 806-820: The current match arm deserializes
res.json::<FunctionExecutionResponse>().await without checking res.status(),
which makes server errors (5xx/4xx) get misreported as bad_argument! for the
change_reason; update the logic in the match handling of response to first
inspect res.status(). If res.status().is_success() then parse
res.json::<FunctionExecutionResponse>().await and proceed as now; otherwise log
the status and body and return unexpected_error! (instead of bad_argument!) to
surface server-side failures related to the FunctionExecutionResponse call for
change_reason validation.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bf6d8c1b-c1ce-493d-a017-dbbc88457e81

📥 Commits

Reviewing files that changed from the base of the PR and between 4f57a97 and 864e784.

📒 Files selected for processing (3)

• crates/experimentation_platform/src/api/experiment_groups/handlers.rs
• crates/experimentation_platform/src/api/experiments/handlers.rs
• crates/experimentation_platform/src/api/experiments/helpers.rs

[Copilot]

Pull request overview

This PR fixes change-reason validation requests getting redirected to the login flow by ensuring the internal function test call includes both the x-user header and an internal Authorization token.

Changes:

• Renamed the change-reason validation helper and extended its signature to accept &User.
• Added x-user and Authorization: Internal <token> headers to the change-reason validation function HTTP request.
• Updated experiment and experiment-group handlers to pass the authenticated user through to the validation helper.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
crates/experimentation_platform/src/api/experiments/helpers.rs	Adds x-user + internal auth header to the change-reason validation function call and updates helper signature/name.
crates/experimentation_platform/src/api/experiments/handlers.rs	Updates experiment endpoints to call the renamed helper and pass &user.
crates/experimentation_platform/src/api/experiment_groups/handlers.rs	Updates experiment-group endpoints to call the renamed helper and pass &user.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.