Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bindings/python/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "amplifier-core-py"
version = "1.5.1"
version = "1.5.2"
edition = "2021"
description = "PyO3 bridge for amplifier-core Rust kernel"
license = "MIT"
Expand Down
9 changes: 2 additions & 7 deletions bindings/python/src/bridges.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use amplifier_core::errors::{AmplifierError, HookError, SessionError};
use amplifier_core::models::{HookAction, HookResult};
use amplifier_core::traits::HookHandler;

use crate::helpers::{is_approval_granted, try_model_dump};
use crate::helpers::{is_approval_granted, json_dumps_safe, try_model_dump};

// ---------------------------------------------------------------------------
// PyHookHandlerBridge — wraps a Python callable as a Rust HookHandler
Expand Down Expand Up @@ -130,13 +130,8 @@ impl HookHandler for PyHookHandlerBridge {
if bound.is_none() {
return Ok("{}".to_string());
}
let json_mod = py.import("json")?;
let serializable = try_model_dump(bound);
let json_str: String = json_mod
.call_method1("dumps", (&serializable,))?
.extract()
.unwrap_or_else(|_| "{}".to_string());
Ok(json_str)
Ok(json_dumps_safe(py, &serializable).unwrap_or_else(|_| "{}".to_string()))
})
.ok_or_else(|| HookError::HandlerFailed {
message: "Failed to attach to Python runtime for result parsing".to_string(),
Expand Down
7 changes: 2 additions & 5 deletions bindings/python/src/coordinator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use pyo3::types::{PyDict, PyList};
use serde_json::Value;

use crate::cancellation::PyCancellationToken;
use crate::helpers::{try_model_dump, wrap_future_as_coroutine};
use crate::helpers::{json_dumps_safe, try_model_dump, wrap_future_as_coroutine};
use crate::hooks::PyHookRegistry;

mod capabilities;
Expand Down Expand Up @@ -114,11 +114,8 @@ impl PyCoordinator {
};
let cfg = sess.getattr("config")?;
let rc: HashMap<String, Value> = {
let json_mod = py.import("json")?;
let serializable = try_model_dump(&cfg);
let json_str: String = json_mod
.call_method1("dumps", (&serializable,))?
.extract()?;
let json_str: String = json_dumps_safe(py, &serializable)?;
serde_json::from_str(&json_str).unwrap_or_else(|e| {
log::warn!("Failed to parse session config as JSON object (using empty config): {e}");
HashMap::new()
Expand Down
18 changes: 18 additions & 0 deletions bindings/python/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// ---------------------------------------------------------------------------

use pyo3::prelude::*;
use pyo3::types::PyDict;

/// Parse an approval system's decision string into a boolean.
///
Expand Down Expand Up @@ -44,3 +45,20 @@ pub(crate) fn try_model_dump<'py>(obj: &Bound<'py, PyAny>) -> Bound<'py, PyAny>
}
}
}

/// Serialize a Python object to a JSON string with `default=str` fallback.
///
/// Like `json.dumps(obj)` but passes Python's built-in `str` as the `default=`
/// callable so non-JSON-native types (e.g. `decimal.Decimal`, `datetime`)
/// become their string representation instead of raising `TypeError`.
///
/// Use this everywhere we call `json.dumps()` at the Python/Rust FFI boundary.
pub(crate) fn json_dumps_safe<'py>(py: Python<'py>, obj: &Bound<'py, PyAny>) -> PyResult<String> {
let json_mod = py.import("json")?;
let str_fn = py.import("builtins")?.getattr("str")?;
let kwargs = PyDict::new(py);
kwargs.set_item("default", &str_fn)?;
json_mod
.call_method("dumps", (obj,), Some(&kwargs))?
.extract()
}
15 changes: 4 additions & 11 deletions bindings/python/src/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use pyo3::types::{PyDict, PyList};
use serde_json::Value;

use crate::bridges::PyHookHandlerBridge;
use crate::helpers::{try_model_dump, wrap_future_as_coroutine};
use crate::helpers::{json_dumps_safe, try_model_dump, wrap_future_as_coroutine};

// ---------------------------------------------------------------------------
// PyUnregisterFn — callable returned by PyHookRegistry.register()
Expand Down Expand Up @@ -129,11 +129,8 @@ impl PyHookRegistry {
) -> PyResult<Bound<'py, PyAny>> {
let inner = self.inner.clone();
// Convert Python data to serde_json::Value
let json_mod = py.import("json")?;
let serializable = try_model_dump(&data);
let json_str: String = json_mod
.call_method1("dumps", (&serializable,))?
.extract()?;
let json_str: String = json_dumps_safe(py, &serializable)?;
let value: Value = serde_json::from_str(&json_str)
.map_err(|e| PyErr::new::<PyRuntimeError, _>(format!("Invalid JSON: {e}")))?;

Expand Down Expand Up @@ -186,8 +183,7 @@ impl PyHookRegistry {
fn set_default_fields(&self, kwargs: Option<&Bound<'_, PyDict>>) -> PyResult<()> {
let value = match kwargs {
Some(dict) => {
let json_mod = dict.py().import("json")?;
let json_str: String = json_mod.call_method1("dumps", (dict,))?.extract()?;
let json_str = json_dumps_safe(dict.py(), dict.as_any())?;
serde_json::from_str(&json_str)
.map_err(|e| PyErr::new::<PyRuntimeError, _>(format!("Invalid JSON: {e}")))?
}
Expand Down Expand Up @@ -238,11 +234,8 @@ impl PyHookRegistry {
timeout: f64,
) -> PyResult<Bound<'py, PyAny>> {
let inner = self.inner.clone();
let json_mod = py.import("json")?;
let serializable = try_model_dump(&data);
let json_str: String = json_mod
.call_method1("dumps", (&serializable,))?
.extract()?;
let json_str: String = json_dumps_safe(py, &serializable)?;
let value: Value = serde_json::from_str(&json_str)
.map_err(|e| PyErr::new::<PyRuntimeError, _>(format!("Invalid JSON: {e}")))?;
let timeout_dur = std::time::Duration::from_secs_f64(timeout);
Expand Down
5 changes: 2 additions & 3 deletions bindings/python/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use pyo3::prelude::*;
use pyo3::types::PyDict;
use serde_json::Value;

use crate::helpers::wrap_future_as_coroutine;
use crate::helpers::{json_dumps_safe, wrap_future_as_coroutine};
use crate::hooks::PyHookRegistry;

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -109,8 +109,7 @@ impl PySession {
}

// ---- Build Rust kernel Session ----
let json_mod = py.import("json")?;
let json_str: String = json_mod.call_method1("dumps", (config,))?.extract()?;
let json_str: String = json_dumps_safe(py, config.as_any())?;
let value: Value = serde_json::from_str(&json_str)
.map_err(|e| PyErr::new::<PyRuntimeError, _>(format!("Invalid config JSON: {e}")))?;
let session_config = amplifier_core::SessionConfig::from_value(value)
Expand Down
59 changes: 59 additions & 0 deletions bindings/python/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,3 +116,62 @@ fn load_wasm_from_path_rejects_rust_transport_with_specific_message() {
"load_wasm_from_path cannot load Rust modules. Use the gRPC sidecar pattern instead."
);
}

/// Verify `json_dumps_safe` exists in helpers with the correct signature.
///
/// This test fails to compile until `json_dumps_safe` is added to helpers.rs.
/// Signature: `fn(Python<'_>, &Bound<'_, PyAny>) -> PyResult<String>`.
#[test]
fn json_dumps_safe_signature_compiles() {
let _: fn(Python<'_>, &Bound<'_, PyAny>) -> PyResult<String> = crate::helpers::json_dumps_safe;
}

/// Structural guard: no raw `json.dumps()` calls outside `helpers.rs`.
///
/// All `json.dumps()` at the Python/Rust FFI boundary must go through
/// `json_dumps_safe()` (which passes `default=str`) to prevent TypeError
/// crashes on non-JSON-native types like `Decimal` or `datetime`.
///
/// If this test fails, you added a `json.dumps()` call in a binding file.
/// Replace: `json_mod.call_method1("dumps", (&obj,))`
/// With: `json_dumps_safe(py, &obj)` (from `crate::helpers`)
#[test]
fn no_raw_json_dumps_outside_helpers() {
use std::fs;
use std::path::Path;

fn check_dir(dir: &Path, violations: &mut Vec<String>) {
if let Ok(entries) = fs::read_dir(dir) {
for entry in entries.flatten() {
let path = entry.path();
if path.is_dir() {
// Skip tests/ — no production FFI code there
if path.file_name().map_or(false, |n| n == "tests") {
continue;
}
check_dir(&path, violations);
} else if path.extension().map_or(false, |e| e == "rs")
&& path.file_name().map_or(false, |n| n != "helpers.rs")
{
if let Ok(content) = fs::read_to_string(&path) {
if content.contains(r#"call_method1("dumps""#)
|| content.contains(r#"call_method("dumps""#)
{
violations.push(path.display().to_string());
}
}
}
}
}
}

let src_dir = Path::new(env!("CARGO_MANIFEST_DIR")).join("src");
let mut violations = Vec::new();
check_dir(&src_dir, &mut violations);

assert!(
violations.is_empty(),
"Raw json.dumps() found outside helpers.rs — use json_dumps_safe() instead:\n{}",
violations.join("\n")
);
}
34 changes: 8 additions & 26 deletions bindings/python/src/wasm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use pyo3::types::PyDict;
use serde_json::Value;

use crate::coordinator::PyCoordinator;
use crate::helpers::{try_model_dump, wrap_future_as_coroutine};
use crate::helpers::{json_dumps_safe, try_model_dump, wrap_future_as_coroutine};

// ---------------------------------------------------------------------------
// PyWasmTool — thin Python wrapper around a Rust Arc<dyn Tool>
Expand Down Expand Up @@ -75,11 +75,8 @@ impl PyWasmTool {
let inner = self.inner.clone();

// Convert Python input to serde_json::Value
let json_mod = py.import("json")?;
let serializable = try_model_dump(&input);
let json_str: String = json_mod
.call_method1("dumps", (&serializable,))?
.extract()?;
let json_str: String = json_dumps_safe(py, &serializable)?;
let value: Value = serde_json::from_str(&json_str)
.map_err(|e| PyErr::new::<PyValueError, _>(format!("Invalid JSON input: {e}")))?;

Expand Down Expand Up @@ -199,11 +196,8 @@ impl PyWasmProvider {
let inner = self.inner.clone();

// Convert Python request to serde_json::Value
let json_mod = py.import("json")?;
let serializable = try_model_dump(&request);
let json_str: String = json_mod
.call_method1("dumps", (&serializable,))?
.extract()?;
let json_str: String = json_dumps_safe(py, &serializable)?;
let chat_request: amplifier_core::messages::ChatRequest = serde_json::from_str(&json_str)
.map_err(|e| {
PyErr::new::<PyValueError, _>(format!("Invalid ChatRequest JSON: {e}"))
Expand Down Expand Up @@ -242,9 +236,7 @@ impl PyWasmProvider {
fn parse_tool_calls(&self, py: Python<'_>, response: Bound<'_, PyAny>) -> PyResult<Py<PyAny>> {
let json_mod = py.import("json")?;
let serializable = try_model_dump(&response);
let json_str: String = json_mod
.call_method1("dumps", (&serializable,))?
.extract()?;
let json_str: String = json_dumps_safe(py, &serializable)?;
let chat_response: amplifier_core::messages::ChatResponse = serde_json::from_str(&json_str)
.map_err(|e| {
PyErr::new::<PyValueError, _>(format!("Invalid ChatResponse JSON: {e}"))
Expand Down Expand Up @@ -300,11 +292,8 @@ impl PyWasmHook {
) -> PyResult<Bound<'py, PyAny>> {
let inner = self.inner.clone();

let json_mod = py.import("json")?;
let serializable = try_model_dump(&data);
let json_str: String = json_mod
.call_method1("dumps", (&serializable,))?
.extract()?;
let json_str: String = json_dumps_safe(py, &serializable)?;
let value: Value = serde_json::from_str(&json_str).map_err(|e| {
PyErr::new::<PyValueError, _>(format!("Invalid JSON for hook data: {e}"))
})?;
Expand Down Expand Up @@ -372,11 +361,8 @@ impl PyWasmContext {
) -> PyResult<Bound<'py, PyAny>> {
let inner = self.inner.clone();

let json_mod = py.import("json")?;
let serializable = try_model_dump(&message);
let json_str: String = json_mod
.call_method1("dumps", (&serializable,))?
.extract()?;
let json_str: String = json_dumps_safe(py, &serializable)?;
let value: Value = serde_json::from_str(&json_str)
.map_err(|e| PyErr::new::<PyValueError, _>(format!("Invalid JSON for message: {e}")))?;

Expand Down Expand Up @@ -475,8 +461,7 @@ impl PyWasmContext {
) -> PyResult<Bound<'py, PyAny>> {
let inner = self.inner.clone();

let json_mod = py.import("json")?;
let json_str: String = json_mod.call_method1("dumps", (&messages,))?.extract()?;
let json_str: String = json_dumps_safe(py, messages.as_any())?;
let values: Vec<Value> = serde_json::from_str(&json_str).map_err(|e| {
PyErr::new::<PyValueError, _>(format!("Invalid JSON for messages: {e}"))
})?;
Expand Down Expand Up @@ -707,11 +692,8 @@ impl PyWasmApproval {
) -> PyResult<Bound<'py, PyAny>> {
let inner = self.inner.clone();

let json_mod = py.import("json")?;
let serializable = try_model_dump(&request);
let json_str: String = json_mod
.call_method1("dumps", (&serializable,))?
.extract()?;
let json_str: String = json_dumps_safe(py, &serializable)?;
let approval_request: amplifier_core::models::ApprovalRequest =
serde_json::from_str(&json_str).map_err(|e| {
PyErr::new::<PyValueError, _>(format!("Invalid ApprovalRequest JSON: {e}"))
Expand Down
Loading
Loading