feat: WASM bindings for cedar-policy-mcp-schema-generator#64
feat: WASM bindings for cedar-policy-mcp-schema-generator#64tomjwxf wants to merge 1 commit intocedar-policy:mainfrom
Conversation
0380632 to
6a6ab42
Compare
|
Rebased onto current main and updated
Ready for review when you have a chance! Happy to address any feedback. |
victornicolet
left a comment
There was a problem hiding this comment.
Thank you for the contribution!
I think the main improvements would be integration tests on the JS side calling the bindings, and removing the locked version on cedar-policy-core by adding some functionality in cedar-policy-mcp-schema-generator.
| serde = { version = "1.0", features = ["derive"] } | ||
| serde_json = "1.0" | ||
| uuid = { version = "1.19.0", features = ["v4", "js"] } | ||
| getrandom = { version = "0.3", features = ["wasm_js"] } |
There was a problem hiding this comment.
Is this dependency used? I can't see where it is, I think it could be removed.
You might want to also bump the dependencies (the same way you bumped the cedar-policy-core dependency) to match the cedar-policy-mcp-schema-generator.
|
|
||
| // Parse schema stub | ||
| let extensions = Extensions::all_available(); | ||
| let fragment = match Fragment::<RawName>::from_cedarschema_str(schema_stub, extensions) { |
There was a problem hiding this comment.
This parsing functionality (which depends on cedar-policy-core) could go in the cedar_policy_mcp_schema_generator crate, so that the bindings crate doesn't have a dependency on cedar-policy-core.
For example, adding SchemaGenerator::from_cedarschema_str and SchemaGenerator::get_schema_as_` if the schemas only need to be represented/parsed as Strings in the bindings.
| [dependencies] | ||
| cedar-policy-mcp-schema-generator = { path = "../cedar-policy-mcp-schema-generator" } | ||
| mcp-tools-sdk = { path = "../mcp-tools-sdk" } | ||
| cedar-policy-core = "=4.9.1" |
There was a problem hiding this comment.
Ideally we would not have to depend on a pinned version of cedar-policy-core in this crate. I think it would be possible to add limited functionality to the cedar-policy-mcp-schema-generator crate to avoid that.
There was a problem hiding this comment.
Thank you for updating cedar-policy-core to 4.9.1 after the upstream changes. You might want to also bump other upstream dependency changes for CI to pass.
There was a problem hiding this comment.
@tomjwxf I think this is the last outstanding item; the diff still shows many dependencies downgraded relative to mainline in Cargo.toml.
|
@victornicolet Thank you for the thorough review. Addressed all five points in the latest push: 1. Removed 2. Moved schema parsing into the generator crate -- added 3. Removed 4. Cargo.lock / dependency bumps -- the lock file updated naturally with the dependency removals. If CI still needs further bumps for other workspace deps, happy to align. 5. Integration tests on the JS side -- acknowledged, I didn't include JS-side integration tests in this push. Would you prefer those as All 32 tests passing (29 generator + 3 WASM), clippy clean, |
|
bcfed1a to
e2fe475
Compare
|
@victornicolet Added both. Squashed into a single commit with DCO sign-off. wasm-bindgen-test integration tests (8 tests in
Generator API tests (6 tests in
Total: 38 tests passing (35 generator + 3 WASM unit), plus the 8 wasm-bindgen-test integration tests (which run under |
e2fe475 to
b0d6875
Compare
|
@victornicolet thanks, the Cargo.lock downgrades were the root issue. I rebased onto the cedar-policy-core 4.9.1 bump, the lock file got regenerated with older transitive dependencies instead of preserving the versions from #65. This pulled in time v0.3.44 (CVE) and yanked keccak v0.1.5, which caused the cargo audit failure. Fixed in the latest push! Rebased cleanly onto current main and restored the upstream lock file before adding the WASM crate's dependencies on top. The diff now only contains legitimate additions (wasm-bindgen, wasm-bindgen-test, etc.). All 237 tests pass locally, clippy clean, fmt clean. The new CI run will need a workflow approval since it's from a fork. Appreciate the patience with the iteration! |
Coverage ReportHead Commit: Base Commit: Download the full coverage report. Coverage of Added or Modified Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 75.12% Status: FAILED ❌ Details
Coverage of All Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 89.79% Status: PASSED ✅ Details
|
b0d6875 to
d9d67b1
Compare
Add `cedar-policy-mcp-schema-generator-wasm`, a thin wasm-bindgen wrapper around the existing Rust SchemaGenerator. Enables JavaScript and TypeScript environments (Node.js, browsers) to generate Cedar schemas from MCP tool descriptions with identical behavior to the Rust implementation. Motivated by @lianah's recommendation in cedar-policy#63 to use WASM bindings instead of a TypeScript reimplementation. Changes to cedar-policy-mcp-schema-generator: - Add SchemaGenerator::from_cedarschema_str() and from_cedarschema_str_with_config() convenience constructors - Add SchemaGenerator::get_schema_as_str() for human-readable output - Add SchemaParseError variant to SchemaGeneratorError - 6 new unit tests for the above APIs WASM bindings crate: - Single generateSchema() function exposed via wasm-bindgen - All SchemaGeneratorConfig options exposed (camelCase JS naming) - Returns JSON with schema, schemaJson, error, and isOk fields - Zero direct dependency on cedar-policy-core (all parsing delegated to generator crate's from_cedarschema_str) - 3 Rust unit tests - 8 wasm-bindgen-test integration tests (basic generation, multi-tool, config options, error handling, config defaults) Signed-off-by: tommylauren <tfarley@utexas.edu>
d9d67b1 to
df3bf4f
Compare
|
@victornicolet @john-h-kastner-aws Updated with additional test coverage for the uncovered code paths (nested objects, array types, all config options, error field completeness, generate_schema_inner match arms, SchemaParseError Display). 56 tests total, clippy clean, fmt clean - coverage now ✅ ✅ |
Coverage ReportHead Commit: Base Commit: Download the full coverage report. Coverage of Added or Modified Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 92.68% Status: PASSED ✅ Details
Coverage of All Lines of Rust CodeRequired coverage: 80.00% Actual coverage: 91.11% Status: PASSED ✅ Details
|
Summary
Adds
rust/cedar-policy-mcp-schema-generator-wasm/, a thinwasm-bindgenwrapper around the existing RustSchemaGenerator. This enables JavaScript and TypeScript environments (Node.js, browsers) to generate Cedar schemas from MCP tool descriptions with the exact same behavior as the Rust implementation.Motivated by @lianah's recommendation in #63 to use WASM bindings instead of a TypeScript reimplementation, to avoid implementation divergence.
What it exposes
A single function:
All
SchemaGeneratorConfigoptions are exposed:includeOutputs,objectsAsRecords,eraseAnnotations,flattenNamespaces,numbersAsDecimal.What it delegates
All schema generation logic, type mapping, and edge case handling is delegated to the Rust
SchemaGenerator. The WASM layer adds no independent logic. This ensures:numberhandling (Long vs Decimal) is identicaladditionalPropertiesas tagged entities is identicalTesting
cargo test)cargo clippyclean with workspace lints (including strictunwrap_used,expect_used,panicdenials)Example output:
Build
Output: 2.5MB WASM binary (wasm-opt applied).
Relationship to #63
This PR replaces the TypeScript reimplementation in #63 with WASM bindings that wrap the existing Rust code. I will close #63 since the TypeScript implementation is no longer needed.
Open questions
@cedar-policy/mcp-schema-generator-wasm? Happy to adjust naming.SchemaGeneratoronly. ShouldRequestGeneratoralso be exposed via WASM in this PR, or in a follow-up?