fix: infer JSON serialization for agent RPCs - #379
Conversation
◈ PR Lens
Architecture 
5 components touched across 3 lanes. Inside the changed components — 2 viewsComponent view — Agent serialization & collection RPC registration and dump collection automatically validate and infer JSON serialization for agent-exposed functions. 
Component view — RPC type contracts Dedicated schema and schema-less interfaces simplify generated declaration snapshots. 
Data flow 
The other flows — 1 sequence
View
Tip Click the link under each diagram to open it on a canvas you can zoom, pan and step through. 🪧 More tips
Thanks for using PR Lens! It's built by Coldtea, free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
🟡 Changes recommended
packages/devframe/src/rpc/types.ts now references incorrect/nonexistent diagnostic codes in the updated jsonSerializable documentation, which should be corrected before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR makes agent-exposed RPC functions implicitly opt into strict JSON serialization (jsonSerializable: true), so MCP-exposed tools always use JSON-safe payloads without requiring an explicit flag, while continuing to reject an explicit jsonSerializable: false.
Changes:
- Normalize RPC registrations/updates so
agentimpliesjsonSerializable: true, and throwDF0019only whenagentis combined withjsonSerializable: false. - Update
DF0019diagnostics text and docs to reflect the new “implicit JSON” behavior. - Adjust collector tests and RPC guide examples to validate/teach the inferred behavior.
File summaries
| File | Description |
|---|---|
| packages/devframe/src/rpc/types.ts | Updates API docs for jsonSerializable to reflect inferred behavior for agent-exposed functions. |
| packages/devframe/src/rpc/diagnostics.ts | Refines DF0019 message/fix text for the new gating rule (jsonSerializable: false). |
| packages/devframe/src/rpc/collector.ts | Implements inference/normalization of jsonSerializable when agent is present. |
| packages/devframe/src/rpc/collector.test.ts | Updates tests to assert inference on register/update and rejection only on explicit false. |
| docs/content/6.errors/DF0019.md | Updates the error reference page to match new semantics and recommended fixes. |
| docs/content/1.guide/3.rpc.md | Updates the guide to show agent implying strict JSON serialization (removes explicit flag in example). |
Review details
Suppressed comments (2)
packages/devframe/src/rpc/types.ts:254
- This docblock mentions
DF0019/DF0018, but strict JSON serialization failures throwDF0020, and theagent+jsonSerializable: falseconflict throwsDF0019(noDF0018exists).
* - `true`: wire and dump use strict `JSON.stringify`; misshapen
* values throw `DF0019` at the call site. Inferred for `agent`.
* - `false` (default): `structured-clone-es` round-trips fancy
* types. Cannot be `agent`-exposed (registration throws `DF0018`).
packages/devframe/src/rpc/types.ts:300
- Same diagnostic-code mismatch here: strict JSON serialization failures are
DF0020, and theagent+jsonSerializable: falseconflict isDF0019(there is noDF0018).
* - `true`: wire and dump use strict `JSON.stringify`; misshapen
* values throw `DF0019` at the call site. Inferred for `agent`.
* - `false` (default): `structured-clone-es` round-trips fancy
* types. Cannot be `agent`-exposed (registration throws `DF0018`).
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
The updated jsonSerializable docs in packages/devframe/src/rpc/types.ts still reference incorrect diagnostic codes/behavior, which would leave the public API documentation inconsistent with the runtime.
Review details
Suppressed comments (3)
packages/devframe/src/rpc/types.ts:181
- The
jsonSerializabledocs referenceDF0019for strict-JSON serialization failures andDF0018for agent exposure, but the strict JSON serializer throwsDF0020(seestrictJsonStringify), and agent exposure only errors whenjsonSerializable: falseis explicitly set (DF0019). Updating these codes/behavior here will keep the docs accurate.
* - `true`: args and return are encoded with strict `JSON.stringify`
* on the wire and on disk. Misshapen values throw `DF0019` at the
* sender, surfacing the bug *during the offending call* rather than
* silently coercing to `{}` later. Inferred for `agent` exposure.
packages/devframe/src/rpc/types.ts:254
- This JSDoc block still says strict JSON failures throw
DF0019and that agent exposure fails withDF0018, but strict JSON validation throwsDF0020and the agent conflict now throwsDF0019only whenjsonSerializable: falseis explicitly set.
* - `true`: wire and dump use strict `JSON.stringify`; misshapen
* values throw `DF0019` at the call site. Inferred for `agent`.
* - `false` (default): `structured-clone-es` round-trips fancy
* types. Cannot be `agent`-exposed (registration throws `DF0018`).
packages/devframe/src/rpc/types.ts:300
- Same as above: this block references
DF0019/DF0018for serialization/agent gating, but strict JSON validation throwsDF0020and the agent +jsonSerializable: falseconflict throwsDF0019.
* - `true`: wire and dump use strict `JSON.stringify`; misshapen
* values throw `DF0019` at the call site. Inferred for `agent`.
* - `false` (default): `structured-clone-es` round-trips fancy
* types. Cannot be `agent`-exposed (registration throws `DF0018`).
- Files reviewed: 6/6 changed files
- Comments generated: 0 new
- Review effort level: Lite
| * silently coercing to `{}` later. Required for `agent` exposure. | ||
| * - `false` (default): payloads use `structured-clone-es`, which | ||
| * round-trips Maps/Sets/cycles. Functions in this mode cannot be | ||
| * exposed via the `agent` field; registration throws `DF0018`. |
There was a problem hiding this comment.
I simplified the docs, they mention error codes, but that's an implementation detail and it shouldn't appear
There was a problem hiding this comment.
🟡 Changes recommended
The new agent serialization inference currently mutates caller-provided RPC definitions and relies on truthiness checks, which can cause surprising runtime behavior and should be tightened for robustness.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new “agent implies JSON” contract is only enforced during collector registration/update, which can leave other definition consumers (e.g. dump/static paths) behaving inconsistently with the updated docs/types.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 7/9 changed files
- Comments generated: 2
- Review effort level: Lite
| function ensureAgentJsonSerializable( | ||
| fnDef: RpcFunctionDefinition<string, any, any, any, any, any, any>, | ||
| ): void { | ||
| if (fn.agent && fn.jsonSerializable !== true) | ||
| throw diagnostics.DF0019({ name: fn.name }) | ||
| if (fnDef.agent && fnDef.jsonSerializable === false) | ||
| throw diagnostics.DF0019({ name: fnDef.name }) | ||
| if (fnDef.agent && !fnDef.jsonSerializable) | ||
| fnDef.jsonSerializable = true | ||
| } |
| /** | ||
| * Declares whether this function's args/return are JSON-serializable, | ||
| * i.e. no `Map`, `Set`, `Date`, `BigInt`, class instances, circular | ||
| * references, `undefined` leaves, `Symbol`, or `Function` values. | ||
| * Selects the serialization format for arguments and return values. | ||
| * | ||
| * - `true`: args and return are encoded with strict `JSON.stringify` | ||
| * on the wire and on disk. Misshapen values throw `DF0019` at the | ||
| * sender, surfacing the bug *during the offending call* rather than | ||
| * silently coercing to `{}` later. Required for `agent` exposure. | ||
| * - `false` (default): payloads use `structured-clone-es`, which | ||
| * round-trips Maps/Sets/cycles. Functions in this mode cannot be | ||
| * exposed via the `agent` field; registration throws `DF0018`. | ||
| * - `true`: uses strict JSON encoding (default when `agent` is set). | ||
| * - `false` (default otherwise): uses structured-clone encoding and supports values | ||
| * such as `Map`, `Set`, `Date`, and cycles. Functions using this mode | ||
| * cannot be agent-exposed. |
There was a problem hiding this comment.
🟢 Approval recommended
The behavioral change is covered by updated unit tests and the docs/diagnostics/snapshots are consistent with the new implicit JSON-serialization contract for agent-exposed RPCs.
Review details
- Files reviewed: 7/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
There are small but important follow-ups needed (notably a missing DF0019-on-update negative test) to fully lock in the new agent/jsonSerializable gating behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 7/16 changed files
- Comments generated: 2
- Review effort level: Lite
| it('also infers jsonSerializable: true on update()', () => { | ||
| const collector = new RpcFunctionsCollectorBase({}) | ||
| collector.register({ name: 'plugin:fn', handler: () => 0 } as any) | ||
| expect(() => collector.update({ | ||
| collector.update({ | ||
| name: 'plugin:fn', |
| /** | ||
| * Prevents registering an agent function that is explicitly marked as | ||
| * non-serializable, and ensures that agent functions are marked as | ||
| * serializable by default. | ||
| * | ||
| * @internal | ||
| */ |
There was a problem hiding this comment.
🟡 Changes recommended
ensureAgentJsonSerializable() doesn’t fully normalize agent-exposed definitions to jsonSerializable: true in the presence of non-boolean truthy values, which can contradict the intended MCP JSON contract.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 10/19 changed files
- Comments generated: 1
- Review effort level: Lite
| export function ensureAgentJsonSerializable(fnDef: RpcFunctionDefinitionAny): void { | ||
| if (fnDef.agent && fnDef.jsonSerializable === false) | ||
| throw diagnostics.DF0019({ name: fnDef.name }) | ||
| if (fnDef.agent && !fnDef.jsonSerializable) | ||
| fnDef.jsonSerializable = true | ||
| } |
I found this to be weird: having to always manually set jsonSerializable when defining agent info. I switched the error to throw when it's set to false.
I also renamed locally a var to something more explicit