Skip to content

refactor(jsonrpc): carry typed inspection errors#2244

Merged
johntmyers merged 1 commit into
NVIDIA:mainfrom
shiju-nv:refactor/jsonrpc-typed-inspection-errors
Jul 13, 2026
Merged

refactor(jsonrpc): carry typed inspection errors#2244
johntmyers merged 1 commit into
NVIDIA:mainfrom
shiju-nv:refactor/jsonrpc-typed-inspection-errors

Conversation

@shiju-nv

Copy link
Copy Markdown
Contributor

Summary

Replace raw JSON-RPC inspection-error strings with a typed value that distinguishes invalid JSON from valid JSON that fails JSON-RPC or MCP message inspection. Relay, OPA, and forward-proxy boundaries preserve their existing diagnostics and behavior, including current MCP batch acceptance.

Related Issue

Part of #2174; does not close it. Addresses typed-error review feedback on #2083. This representation-only prerequisite does not fix #2082.

Changes

  • Change JsonRpcRequestInfo.error from Option<String> to Option<JsonRpcInspectionError>.
  • Add the public, #[non_exhaustive] JsonRpcInspectionErrorKind with InvalidJson and InvalidMessage; keep constructors and fields private so the parser remains the classification authority.
  • Expose explicit kind() and detail() accessors without Deref<str>, AsRef<str>, string comparison support, or Serde on the Rust error value.
  • Preserve OPA's existing string-or-null shape by projecting detail() only at the policy boundary.
  • Preserve the existing JSON-RPC request rejected: {detail} denial reason in route-selected relay, direct relay, and forward-proxy paths.
  • Add parser classification, OPA projection, forward-proxy reason, and current MCP batch-acceptance regression tests.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (not applicable: this is a representation-only change with no wire or policy behavior change)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

Signed-off-by: Shiju <shiju@nvidia.com>
@johntmyers johntmyers added the test:e2e Requires end-to-end coverage label Jul 13, 2026
@github-actions

Copy link
Copy Markdown

Label test:e2e applied for ef38559. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

PR Review Status

Validation: This concentrated JSON-RPC/MCP refactor is project-valid because it directly supports the typed inspection-error prerequisite described in #2174 and addresses the implementation feedback referenced from #2083 without expanding into the version-specific behavior tracked separately.
Head SHA: ef3855933a8b4f44912a3957a8694745fe385d83

Review findings:

  • No blocking findings remain. The independent review confirmed that error classification stays typed, fail-closed behavior and denial diagnostics are preserved, the OPA boundary remains string-or-null, and current MCP batch acceptance is unchanged.

Docs: Not needed because this is a representation-only refactor with no user-facing configuration, policy shape, wire behavior, or workflow change.

E2E: Required because the direct JSON-RPC/MCP relay and forward-proxy enforcement paths changed. I applied test:e2e.

Next state: gator:watch-pipeline

@johntmyers johntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status gator:approval-needed Gator completed review; maintainer approval needed and removed gator:watch-pipeline Gator is monitoring PR CI/CD status labels Jul 13, 2026
@johntmyers
johntmyers merged commit fcc9db3 into NVIDIA:main Jul 13, 2026
68 of 69 checks passed
@johntmyers

Copy link
Copy Markdown
Collaborator

gator-agent

Monitoring Complete

Monitoring is complete because this PR has merged.

Final status: Gator had completed its code review and pipeline monitoring, the required E2E label was present, maintainer approval was recorded, and the PR merged at head ef3855933a8b4f44912a3957a8694745fe385d83.

I removed the active gator:* label because there is nothing left for gator to monitor on this PR.

@johntmyers johntmyers removed the gator:approval-needed Gator completed review; maintainer approval needed label Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:e2e Requires end-to-end coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP endpoints accept JSON-RPC batch payloads

2 participants