refactor(rust/core): unify job serialization across bindings via serde (Closes #1113)

[dhyansraj]

Summary

The binding-duplication cluster of #1113 — the second and final part (hygiene quick-wins landed in #1119). Unifies job serialization across the three native bindings (Python/pyo3, Node/napi, FFI) by routing them all through serde instead of three hand-built per-binding mappers. Wire-format-preserving — proven byte-identical end-to-end.

• Job wire-shape: add Serialize to Job and JobEventReceipt (JobStatus/JobEvent already had it); delete the three copied job_status_to_str (serde's rename_all="snake_case" is the single source of truth — no as_str() added). Route each binding through serde: FFI → serde_json::to_string (Result handled, no unwrap), napi → serde_json::to_value (still returns Value; signatures unchanged), py → serde_json::to_value + the shared json_value_to_pyobject.
  • No skip_serializing_if on Job/JobEvent/JobEventReceipt: their Option fields keep emitting None → null (all keys present), which the Python SDK's strict snapshot["progress"] access depends on. (#[serde(default)] is deserialize-only — no serialize effect.)
• Dedup json_value_to_py: keep the lib.rs &Value version (now pub(crate)), delete the owned-Value copy in jobs_py.rs.
• Dedup timeout validators: three near-identical parse_*timeout_secs → one validate_secs_to_duration(secs, negative_is_none) (py/napi reject-negative; FFI negative-as-none). validate_deadline_secs and the wait / run-as-job deadline blocks left untouched (different shapes).

Net ~245 lines of duplication removed.

Review Notes

Independent review: 0 blockers, 0 warnings, 3 INFO (all non-defects, left as-is):

• napi serializers use .unwrap_or(Value::Null) vs FFI/py error-propagation — cosmetic; these plain structs never fail to serialize.
• Negative-timeout error wording split into clearer per-cause messages (improvement; no test asserts the old string; the out of range overflow substring is preserved for the napi test).
• progress_f32_roundtrips uses 0.5 (exact) — deterministic by design.
  Verified: keep-nulls intact, exact field-set/"type"-key match, FFI errors routed (no panic across C ABI), timeout policy preserved per call site (incl. NEG_INFINITY ordering), validate_deadline_secs not folded in.

Test plan

• cargo test green incl. 6 new shape/helper tests (full + minimal/keep-nulls Job, JobEvent "type" key, JobEventReceipt 3-key, progress f32 roundtrip, timeout edge cases)
• cargo check clean across python/typescript/ffi feature variants; cargo clippy no new warnings (one removed)
• src-tests rebuild 12/12 — incl. tc02_build_rust_core, tc02a_build_rust_core_nodejs (napi), Python SDK (pyo3 link), Java SDK (FFI)
• MeshJob suites 72/72 across all three wire paths — uc21_meshjob Python 21/21, uc22_meshjob_ts 24/24, uc23_meshjob_java 27/27 — status snapshots (keep-nulls), events (recv/send/list/subscribe), and long-running lifecycle parse identically in every SDK. Zero serialization/parse mismatches.

Completes #1113 alongside the already-merged #1119 quick-wins.

Closes #1113

Summary by CodeRabbit

• Bug Fixes

  • Improved error handling and reporting for job data serialization across all language bindings
  • Enhanced timeout validation with consistent error checking and clearer error messages
  • Better error detection for invalid timeout specifications

• Improvements

  • Consolidated serialization logic for improved system stability and consistency

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR consolidates duplicate job serialization and timeout validation logic across the three binding layers (Python/PyO3, JavaScript/N-API, C/FFI) by introducing Serialize derives on core types, a shared timeout validation helper, and refactoring all binding entry points to use canonical serde-based conversion instead of hand-written field assembly.

Changes

Binding layer consolidation

Layer / File(s)	Summary
Core Serialize derives and shared timeout validation src/runtime/core/src/task_backend.rs, src/runtime/core/src/lib.rs	Job and JobEventReceipt now derive Serialize to enable canonical wire-format serialization; new validate_secs_to_duration(secs, negative_is_none) centralizes timeout validation (NaN/Inf/overflow checks, policy-driven negative handling) replacing per-binding duplicates; json_value_to_pyobject becomes pub(crate) for reuse across bindings; comprehensive wire-shape and edge-case tests added.
Python binding consolidation src/runtime/core/src/jobs_py.rs	Timeout parsing delegates to validate_secs_to_duration; job_to_pydict, job_event_to_pydict, and job_event_receipt_to_pydict now serialize via serde_json::to_value followed by crate::json_value_to_pyobject instead of manual PyDict field assembly; JobController.recv_event, JobProxy.status, JobProxy.wait, and JobProxy.send_event updated to use unified helpers; JobStatus import removed.
JavaScript/N-API binding consolidation src/runtime/core/src/jobs_napi.rs	Timeout parsing delegates to validate_secs_to_duration; job_event_to_json, job_event_receipt_to_json, and job_to_json now use serde_json::to_value with Null fallback instead of manual serde_json::json!({ ... }) field-by-field construction; JobStatus import removed.
C FFI binding consolidation and error handling src/runtime/core/src/jobs_ffi.rs	New write_out_json helper centralizes JSON serialization error handling (sets thread-local last-error on failure); timeout parsing delegates to validate_secs_to_duration; job/event JSON helpers return serde_json::Result<String> (error-capable) instead of infallible String; job_status_to_str removed; mesh_job_controller_recv_event, mesh_job_proxy_status, mesh_job_proxy_send_event, and mesh_job_proxy_list_events updated to use write_out_json and propagate serialization failures through FFI error codes; JobStatus import removed.

Sequence Diagram(s)

sequenceDiagram
  participant FFI_Caller as FFI Caller
  participant FFI_Function as FFI Function (mesh_job_proxy_status)
  participant json_helper as JSON Helper (job_to_string)
  participant write_out_json
  participant thread_local as Thread-local Error
  participant c_output as C Output (out_json)
  
  FFI_Caller->>FFI_Function: mesh_job_proxy_status(..., out_json, ...)
  FFI_Function->>json_helper: job_to_string(&job)
  alt Serialization Success
    json_helper-->>FFI_Function: Ok(json_str)
    FFI_Function->>write_out_json: write_out_json(json_str.as_ptr(), out_json)
    write_out_json->>c_output: copy json_str into *out_json
    write_out_json-->>FFI_Function: 0 (success)
    FFI_Function-->>FFI_Caller: 0 (success)
  else Serialization Failure
    json_helper-->>FFI_Function: Err(serde_json::Error)
    FFI_Function->>write_out_json: write_out_json(err_json_str.as_ptr(), out_json)
    write_out_json->>thread_local: set_last_error(message)
    write_out_json-->>FFI_Function: -1 (error)
    FFI_Function-->>FFI_Caller: -1 (error)
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• dhyansraj/mcp-mesh#1119: Both PRs modify parse_ffi_timeout_secs in jobs_ffi.rs and share the same validation refactoring strategy via validate_secs_to_duration.
• dhyansraj/mcp-mesh#1079: Changes to job serialization and JobStatus wire shape in this PR affect the N-API bindings and their TypeScript facades that depend on the underlying JSON/timeout behavior.

Poem

🐰 Three bindings once cast spells apart,
Duplicating job-shape, field by art—
Now serde's one truth spreads its decree,
Timeout and JSON: one, shared, free. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The PR addresses the binding-layer duplication fixes from #1113 (job serialization, json_value_to_py deduplication, timeout validation consolidation) but does not implement the hygiene quick-wins (backoff jitter, heartbeat log level demotion) or minor fixes (HashMap lookup, clippy warnings).	Implement the remaining objectives from #1113: add decorrelated jitter to calculate_backoff, demote heartbeat JSON logging to debug!/trace!, fix HashMap double lookup, and address clippy useless_format warnings.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main refactoring objective: unifying job serialization across bindings via serde, with issue reference.
Out of Scope Changes check	✅ Passed	All changes are in-scope: unifying job serialization (jobs_*.rs files), making helpers crate-visible (lib.rs), and adding validation/serialization support (task_backend.rs).
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ 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 refactor/1113-job-serde-unification

---

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)

src/runtime/core/src/jobs_napi.rs (1)

108-124: 💤 Low value

Consider propagating serialization errors for consistency.

The unwrap_or(serde_json::Value::Null) fallback differs from Python (propagates PyValueError) and FFI (sets last-error and returns -1). While current types are guaranteed serializable, this could silently mask issues if a non-serializable field is added in the future.

If signature stability is the constraint, this is acceptable as-is.

🤖 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 `@src/runtime/core/src/jobs_napi.rs` around lines 108 - 124, The three helpers
(job_event_to_json, job_event_receipt_to_json, job_to_json) currently swallow
serde errors by returning Value::Null; change their signatures to return
Result<serde_json::Value, E> so serialization failures are propagated (e.g.
Result<serde_json::Value, serde_json::Error> or mapped to napi::Error) and
replace unwrap_or(...) with serde_json::to_value(&...)? to return the error;
ensure callers are updated to handle or convert the Result into the appropriate
FFI/napi error path so failures are reported instead of silently producing Null.

🤖 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 `@src/runtime/core/src/jobs_napi.rs`:
- Around line 108-124: The three helpers (job_event_to_json,
job_event_receipt_to_json, job_to_json) currently swallow serde errors by
returning Value::Null; change their signatures to return
Result<serde_json::Value, E> so serialization failures are propagated (e.g.
Result<serde_json::Value, serde_json::Error> or mapped to napi::Error) and
replace unwrap_or(...) with serde_json::to_value(&...)? to return the error;
ensure callers are updated to handle or convert the Result into the appropriate
FFI/napi error path so failures are reported instead of silently producing Null.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 09e865a2-668e-4723-8de4-85854ddf7f23

📥 Commits

Reviewing files that changed from the base of the PR and between d531d1b and 562901b.

📒 Files selected for processing (5)

• src/runtime/core/src/jobs_ffi.rs
• src/runtime/core/src/jobs_napi.rs
• src/runtime/core/src/jobs_py.rs
• src/runtime/core/src/lib.rs
• src/runtime/core/src/task_backend.rs