Skip to content

Refine typed Rust lifecycle API - #1255

Open
Gudge (MGudgin) wants to merge 1 commit into
mainfrom
user/gudge/rust-sdk-operation-api
Open

Gudge (MGudgin) wants to merge 1 commit into
mainfrom
user/gudge/rust-sdk-operation-api

Conversation

@MGudgin

@MGudgin Gudge (MGudgin) commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

This PR refines the direct typed Rust lifecycle transport into an
operation-centric public API under mxc_sdk::sandbox.

Details

  • Add sandbox::{provision,start,exec,stop,deprovision} and matching
    validate_* functions.
  • Introduce an opaque SandboxId and keep sandbox identity separate from
    policy and operation options.
  • Split provision, exec, and lifecycle request shapes so invalid combinations
    are not representable through one broad request type.
  • Return operation-specific result types instead of a shared optional-result
    structure.
  • Reject empty and embedded-NUL sandbox identities at the public boundary.

Tests

  • cargo fmt --all -- --check
  • All-feature mxc_engine and mxc-sdk tests passed.
  • Public API compile-shape and behavior tests passed.
  • mxc-sdk Rustdoc tests passed.
  • Changed-crate clippy passed with --no-deps -D warnings.
  • git diff --check
Microsoft Reviewers: Open in CodeFlow

@MGudgin
Gudge (MGudgin) requested a review from a team as a code owner September 23, 2026 22:23
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@MGudgin
Gudge (MGudgin) added this pull request to stack #1257 September 23, 2026 22:27
@MGudgin
Gudge (MGudgin) requested review from a team and a balanced review from Copilot September 24, 2026 15:08
@MGudgin
Gudge (MGudgin) force-pushed the user/gudge/rust-sdk-operation-api branch from 65268d7 to 70b5186 Compare September 24, 2026 15:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Sandbox identity validation returns the wrong established error category, and several new validation signatures lack compile-shape coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity · 1 Low severity

Open (2)
What changed in this PR

Refines the typed Rust lifecycle API into operation-specific functions and request/result types under mxc_sdk::sandbox.

Changes:

  • Adds typed lifecycle and validation entry points.
  • Introduces opaque SandboxId and operation-specific results.
  • Updates tests and documentation for the new API.
File Description
src/​core/​mxc-sdk/​tests/​state_aware.rs Tests the revised public API and ID validation.
src/​core/​mxc-sdk/​src/​sandbox.rs Adds operation-centric SDK functions.
src/​core/​mxc-sdk/​src/​lib.rs Exports the new module and types.
src/​core/​mxc-sdk/​README.md Documents the revised lifecycle API.
src/​core/​mxc_engine/​src/​state_aware.rs Implements operation-specific execution and validation.
src/​core/​mxc_engine/​src/​state_aware_sdk.rs Defines IDs, requests, options, and result types.
src/​core/​mxc_engine/​src/​lib.rs Re-exports the revised engine surface.
docs/​state-aware-lifecycle/​mxc-state-aware-sandbox-api.md Updates lifecycle architecture documentation.
docs/​state-aware-lifecycle/​mxc-state-aware-sandbox-api-overview.md Updates the high-level API overview.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/core/mxc_engine/src/state_aware_sdk.rs
Comment thread src/core/mxc-sdk/tests/state_aware.rs
Copilot AI review requested due to automatic review settings September 24, 2026 16:06
@MGudgin
Gudge (MGudgin) force-pushed the user/gudge/rust-sdk-operation-api branch from 70b5186 to 73609ab Compare September 24, 2026 16:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The documented unknown-prefix error contract conflicts with the tested Rust behavior, and telemetry-option presence lacks regression coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
Resolved since last review (2)

Comment thread src/core/mxc-sdk/tests/state_aware.rs
Comment thread src/core/mxc_engine/src/state_aware_sdk.rs
Copilot AI review requested due to automatic review settings September 24, 2026 16:27
@MGudgin
Gudge (MGudgin) force-pushed the user/gudge/rust-sdk-operation-api branch from 73609ab to 2945ff7 Compare September 24, 2026 16:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The implementation consistently applies the new typed API shape and includes aligned tests and documentation.

Review effort: Balanced
Findings: None

Resolved since last review (2)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Public crate documentation still links to the removed exec_attached_request API.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Low severity

Open (1)

Comment thread src/core/mxc-sdk/src/lib.rs
Copilot AI review requested due to automatic review settings September 25, 2026 19:28
@MGudgin
Gudge (MGudgin) force-pushed the user/gudge/rust-sdk-operation-api branch from 8f023a5 to 593a9ff Compare September 25, 2026 19:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Several engine unit tests still call removed or changed request methods, preventing the test target from compiling.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (1)

Comment thread src/core/mxc_engine/src/state_aware_sdk.rs
Copilot AI review requested due to automatic review settings September 25, 2026 19:40
@MGudgin
Gudge (MGudgin) force-pushed the user/gudge/rust-sdk-operation-api branch from 593a9ff to ae0651f Compare September 25, 2026 19:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The API refactor is internally consistent, documented, and covered by focused compile-shape and behavior tests.

Review effort: Balanced
Findings: 1 High severity

Open (1)

Base automatically changed from user/gudge/rust-sdk-direct-transport to main September 26, 2026 21:24
This PR refines the direct typed Rust lifecycle surface around explicit sandbox
operations while preserving the raw exact JSON compatibility lane.

Details

* Group typed provision, start, exec, stop, and deprovision calls under
  `mxc_sdk::sandbox` with matching validation functions.
* Add an opaque `SandboxId` and pass identity separately from typed policy.
* Move experimental authorization and telemetry preference into
  `OperationOptions`.
* Return distinct `ProvisionResult`, `LifecycleResult`, and `ValidationResult`
  types.
* Classify invalid sandbox identity syntax as `malformed_id`, while documenting
  Rust's dispatch-time handling of syntactically valid unknown prefixes.
* Cover every public validation signature and preserve both true and false
  telemetry preferences through typed request normalization.
* Adapt inherited typed-versus-exact provision tests to option-owned telemetry.
* Link typed attached exec through `sandbox::exec_attached` while retaining the
  raw JSON `exec_attached` compatibility entry point.

Tests

* `cargo fmt --all -- --check`
* `cargo check --workspace --all-targets --all-features`
* `cargo test -p mxc_engine --all-features` (151 passed)
* `cargo test -p mxc-sdk --test state_aware --all-features` (19 passed)
* `cargo clippy --workspace --all-targets --all-features -- -D warnings`
* `RUSTDOCFLAGS="-D warnings" cargo doc -p mxc-sdk --all-features --no-deps`
* `git diff --check`

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1d91874d-feea-4a33-b183-67d506ae61b5
Generated-with: gpt-5.6-sol
Copilot AI review requested due to automatic review settings September 26, 2026 21:24
@MGudgin
Gudge (MGudgin) force-pushed the user/gudge/rust-sdk-operation-api branch from ae0651f to 357ceea Compare September 26, 2026 21:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The API refactor is internally consistent, documented, and covered by compile-shape and behavior tests.

Review effort: Balanced
Findings: None

Resolved since last review (1)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants