Phase 2B: deserialize the parser into the wire model#566
Open
MGudgin wants to merge 3 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR is Phase 2B of the versioning remediation: it rewires wxc_common’s config parsing to deserialize directly into the typed wire model (wire::MxcConfig) and removes the separate permissive Raw* serde structs, reducing drift risk between schema and parser.
Changes:
- Switch config parsing (one-shot and state-aware) to
serde_json→wire::MxcConfig, then map wire → domain (ExecutionRequest/ParsedStateAwareRequest). - Tighten the stable surface to strict enums +
deny_unknown_fieldsat every stable level; keep theexperimentalsurface permissive/forward-compatible. - Remove
processContainer.nameemission from both the Rustmxc-sdkcrate and the TypeScript SDK to match the strict wire model.
Show a summary per file
| File | Description |
|---|---|
| src/core/wxc_common/src/wire.rs | Makes the wire types always-compiled, gates schema generation derives, and adds/adjusts aliases + permissiveness boundaries. |
| src/core/wxc_common/src/state_aware_request.rs | Removes Phase::from_wire now that phase parsing is handled via the wire enum. |
| src/core/wxc_common/src/lib.rs | Exposes wire unconditionally as the parser’s deserialization target. |
| src/core/wxc_common/src/config_parser.rs | Replaces Raw* deserialization + conversions with wire-model deserialization and wire→domain mapping for one-shot and state-aware. |
| src/core/mxc-sdk/src/policy.rs | Stops emitting processContainer.name in the Rust library’s config builder. |
| sdk/src/sandbox.ts | Stops emitting processContainer.name in the TS SDK policy→config builder. |
| schemas/dev/mxc-config.schema.0.8.0-dev.json | Updates generated schema to reflect the new permissive experimental nested structs. |
| docs/versioning.md | Updates documentation to reflect wire-model parsing and strict-enum behavior (including alias observability changes). |
| docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md | Updates state-aware spec text/examples to reflect wire::MxcConfig as the Rust mirror/parse target. |
| docs/state-aware-lifecycle/mxc-state-aware-sandbox-api-overview.md | Same: aligns overview with the new wire-model parse flow. |
| docs/schema-codegen.md | Updates roadmap/status now that the parser is wired to the wire model and Raw structs are gone. |
| docs/nanvix-microvm/nanvix-integration-plan.md | Updates the plan references to reflect the wire-model-based parser flow. |
| docs/bwrap-support/bubblewrap-backend-plan.md | Marks older Raw-struct steps as superseded; points to wire-model approach. |
| docs/authoring-a-new-feature.md | Updates the feature-authoring checklist to edit wire model + regenerate schema, then map wire→domain. |
| .github/copilot-instructions.md | Updates repository conventions/docs to reflect the new config parsing flow and feature-authoring steps. |
Copilot's findings
- Files reviewed: 15/15 changed files
- Comments generated: 0
This PR changes the config parser to deserialize directly into the typed wire model (`wxc_common::wire::MxcConfig`) instead of an intermediate set of permissive structs, so the JSON schema source of truth and the parser's trust boundary share a single definition of the wire shape and cannot drift. Details: - The parser deserializes both one-shot and state-aware requests straight into `wire::MxcConfig`, discriminating state-aware requests by the `phase` key and keeping the `experimental` block as a raw value for per-backend dispatch typing. - The wire structs are the deserialization target; the previous intermediate structs and their conversion helpers are removed (a large net reduction in `config_parser.rs`). - The boundary is strict by construction: invalid containment, network, clipboard, and configurationId values are rejected, and unknown nested fields are rejected (`deny_unknown_fields`). Deprecated spellings (`appcontainer`, `macos_sandbox`, `appContainer`) are accepted via serde aliases. - The `wire` module is always compiled as the parser target; its `JsonSchema` derive stays behind the `schema-gen` feature. - The TypeScript SDK and the in-process `mxc-sdk` crate emit only fields the wire objects define: the container id is carried at top-level `containerId` and the teardown / preserve flags at `lifecycle.destroyOnExit` / `lifecycle.preservePolicy`, so `processContainer.name`, `lxc.containerName`, `lxc.destroyOnExit`, and `filesystem.clearPolicyOnExit` are no longer sent. - Docs (`versioning.md`, `authoring-a-new-feature.md`, `schema-codegen.md`, `copilot-instructions.md`, state-aware lifecycle) describe the wire-model parser. Tests: - cargo test --workspace; cargo fmt --all -- --check; cargo clippy --workspace --all-targets -- -D warnings. - Schema codegen, config corpus (169/169), and schema-version gates. - SDK unit tests (cd sdk && npm test). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Generated-with: claude-opus-4.8
c837fc4 to
6d3538b
Compare
bbonaby
reviewed
Jun 25, 2026
…doc cleanup) This change applies code-review feedback on the wire-model parser without altering behavior: it makes the trivial wire->domain enum/struct conversions idiomatic, groups the schema-generation code behind one feature gate, and removes stale references to the deleted Raw* structs. Details: - Replace the free `map_wire_*` enum/struct converters with `From` impls beside the domain types: `From<wire::LaunchMethod>`, `From<wire::IsolationConfigurationId>`, `From<wire::IsolationUser>` in models.rs and `From<wire::Phase>` in state_aware_request.rs; call sites now use `.into()`. `map_wire_containment` stays a function (it resolves `Option<&_>` per target_os, not a pure conversion). - Group the schema-gen functions (`generate_config_schema_json` and its helpers) into a single `#[cfg(feature = "schema-gen")] mod schema_gen`, re-exporting the one public entry point, instead of repeating the attribute per item. The wire structs stay un-gated (they are the parser's deserialization target); only the schemars-dependent generation code is gated. - Rename the discriminator `value` to `parsed_json` in the parse entry point. - Drop the now-redundant SDK comments explaining omitted wire fields, and remove stale `Raw*` references from docs (authoring-a-new-feature, bubblewrap plan, state-aware API, copilot-instructions). Tests: - cargo build -p wxc_common (default + schema-gen); cargo clippy --workspace --all-targets -- -D warnings; cargo fmt --all -- --check. - cargo test --workspace; schema codegen byte-identical; corpus 169/169. - cd sdk && npm test -> 183 pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Generated-with: claude-opus-4.8
This change adds a "Wire Model vs Runtime Model" section to docs/versioning.md explaining why the parser keeps two Rust representations (the JSON-mirroring wire model and the validated runtime model) with a mapping at the parse boundary, rather than a single shared type. Details: - Describe the wire model (schema/SDK codegen source, faithful JSON shape) and the runtime/domain model (validated, invariant-rich, what backends consume), and the parser as the single validate/normalize boundary between them. - Lay out the pros (one boundary, parse-don't-validate, pure schema source, decoupled evolution) and cons (boilerplate, internal drift, indirection), and note the costs are addressable without merging the layers. Tests: - Documentation-only change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Generated-with: claude-opus-4.8
bbonaby
approved these changes
Jun 25, 2026
| Some("read") => ClipboardPolicy::Read, | ||
| Some("write") => ClipboardPolicy::Write, | ||
| Some("all") => ClipboardPolicy::All, | ||
| if let Some(raw_ui) = cfg.ui { |
Collaborator
There was a problem hiding this comment.
some of these ones can also have the from<T> added for them as well.
wire::ClipboardPolicy -> ClipboardPolicy,wire::NetworkEnforcement-> NetworkEnforcementModewire::NetworkPolicy-> NetworkPolicywire::Containment -> ContainmentBackend
Then for wire_ui_isolation_str we'd probably want a to_string() function in the enum itself kind of like what we do here:
mxc/src/core/wxc_common/src/models.rs
Line 50 in 22f8a46
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR changes the config parser to deserialize directly into the typed wire model (
wxc_common::wire::MxcConfig) instead of an intermediate set of permissive structs, so the JSON schema source of truth and the parser's trust boundary share a single definition of the wire shape and cannot drift.What changed
wire::MxcConfig, discriminating state-aware requests by thephasekey and keeping theexperimentalblock as a raw value for per-backend dispatch typing.config_parser.rs).containment/network/clipboard/configurationIdvalues are rejected, and unknown nested fields are rejected (deny_unknown_fields). Deprecated spellings (appcontainer,macos_sandbox,appContainer) are accepted via serde aliases.wiremodule is always compiled as the parser target; itsJsonSchemaderive stays behind theschema-genfeature.mxc-sdkcrate emit only fields the wire objects define: the container id is carried at top-levelcontainerIdand the teardown / preserve flags atlifecycle.destroyOnExit/lifecycle.preservePolicy, soprocessContainer.name,lxc.containerName,lxc.destroyOnExit, andfilesystem.clearPolicyOnExitare no longer sent.versioning.md,authoring-a-new-feature.md,schema-codegen.md,copilot-instructions.md, and the state-aware lifecycle docs) describes the wire-model parser.Verification
cargo test --workspace;cargo fmt --all -- --check;cargo clippy --workspace --all-targets -- -D warningscd sdk && npm testMicrosoft Reviewers: Open in CodeFlow