Skip to content

fix: preserve JSON-looking strings in body mutations - #2846

Open
git-jxj wants to merge 2 commits into
theagentrouter:mainfrom
git-jxj:fix/body-mutation-json-values-20261013
Open

git-jxj wants to merge 2 commits into
theagentrouter:mainfrom
git-jxj:fix/body-mutation-json-values-20261013

Conversation

@git-jxj

@git-jxj git-jxj commented Oct 13, 2026

Copy link
Copy Markdown

Description

A bodyMutation.set value such as 1.0.0, +1, or [abc] passes the JSON-looking heuristic and is inserted unquoted, corrupting an otherwise valid request body. Use the existing gjson.Valid validator so non-JSON values follow the existing string fallback. Valid JSON literals, objects, and arrays retain their types.

Regression tests exercise BodyMutator.Mutate with malformed numbers, objects, arrays, escape sequences, empty strings, and valid JSON values.

Related Issues/PRs (if applicable)

None.

Special notes for reviewers (if applicable)

  • make precommit: passed, including generated API files, lint, format, and Helm checks.
  • make test: passes with TestStartConfigBundleWatcher and TestNewRunOpts_Errors skipped. Both failures reproduce on unchanged main; the permission tests also pass in a subprocess without DAC bypass capabilities.
  • A local HTTP harness returns 400 before the fix and 204 after it for the same version-string mutation.

Codex assisted with the implementation and regression tests.

Signed-off-by: xinjun.jiang <xinjun.jiang@daocloud.io>
@git-jxj
git-jxj requested a review from a team as a code owner October 13, 2026 10:44
@netlify

netlify Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for theagentrouter canceled.

Name Link
🔨 Latest commit 75f12ff
🔍 Latest deploy log https://app.netlify.com/projects/theagentrouter/deploys/6ad19dafcd4d51417743c2e9

@git-jxj

git-jxj commented Oct 15, 2026

Copy link
Copy Markdown
Author

The workflows for current head b1b18aa656a2 are still awaiting maintainer approval: Precommit Check, CodeQL, Build and Test. Could a maintainer approve these current-head runs so CI validation can start?

@codecov

codecov Bot commented Oct 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Signed-off-by: xinjun.jiang <xinjun.jiang@daocloud.io>
@git-jxj

git-jxj commented Oct 16, 2026

Copy link
Copy Markdown
Author

Merged main at ca3e5166 in 75f12ff9 to pick up the existing MCP benchmark fix in #2856.

On Linux, the full make test-data-plane-mcp (including all four benchmarks) and make precommit pass. The complete unit run has one failure, TestStartConfigBundleWatcher; the same assertion reproduces on unchanged main ca3e5166.

The previous inference job timed out waiting for the secondary Gateway parent; its final InferencePool status contained only the primary parent. That controller/conformance source is identical on main, so this still needs separate CI validation.

Could a maintainer approve Build and Test, Precommit, and CodeQL for 75f12ff9? All three new runs are action_required. macOS MCP and inference conformance have not yet run on this head.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant