Skip to content

Implement W365 Agents Phase 2 checkout and release lifecycle - #1723

Open
tbyfield wants to merge 8 commits into
mainfrom
users/tbyfield/w365a-phase2-lifecycle
Open

tbyfield wants to merge 8 commits into
mainfrom
users/tbyfield/w365a-phase2-lifecycle

Conversation

@tbyfield

@tbyfield tbyfield commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

📖 Description

Implement the experimental W365 Agents Phase 2 vertical slice on the merged Phase 1 contracts: checkout through provision and terminal release through stop or deprovision.

  • Add an internal environment-configured API 2.0 service client, session-based opaque IDs, checked binding and normal engine dispatch. W365 is included in normal builds and requires runtime experimental authorization.
  • Preserve known cleanup identities, request IDs and uncertain outcomes in native structured errors. Never automatically replay checkout or perform hidden cleanup. Release acceptance does not claim completed guest reset or sanitization.
  • Use the existing Windows executor lifecycle interface. Linux/macOS executors are unchanged; their general lifecycle CLI support is deferred. Engine-level trusted-TLS fixtures run on all three platforms, with fresh-process executor recovery coverage on Windows.
  • Keep readiness/start, exec, PTY, create-and-run execution and placement sessions explicitly unavailable. Typed W365 SDK APIs are not added; Dev SDK JSON access remains gated pending cross-SDK error metadata and SDK validation.
  • Preserve Phase 1 request types and generated artifacts. Service settings remain environment-only, with no per-call endpoint/token overrides or caller-selected API version.

The branch contains one implementation commit, d9985fc7, above the merged Phase 1 commit 37a3e931.

🔗 References

Refs #1615 (open, maintainer-applied feature-accepted).

Approved and merged design/API proposal: #1619. Phase 1 contracts: #1697.

Implementation scope and validation limits: docs/backends/w365agents/w365agents-backend.md.

🔍 Validation

Local Windows validation of the final tree, from src/ unless stated otherwise:

  • cargo check -p mxc-sdk --quiet — passed after removing the unused ureq/json feature and refreshing the lockfile.
  • cargo build --locked -p wxc --features test-support --quiet — passed.
  • With MXC_W365_TEST_EXECUTOR pointing to the built target/debug/wxc-exec.exe, cargo test --locked -p mxc-sdk --lib w365 -- --include-ignored — passed, 6 tests, including trusted-TLS engine and fresh-process Windows executor fixtures. Only loopback services and synthetic credentials were used.
  • cargo test --locked -p mxc-sdk --lib --quiet -- config_parser state_aware backend_registry policy_identity — passed, 497 tests.
  • cargo test --locked -p mxc-sdk --test mxc_contract_v1_1_0_alpha --test state_aware --quiet — passed, 388 contract tests and 16 lifecycle tests.
  • cargo test --locked -p mxc_ffi --lib state_aware --quiet — passed, 21 tests.
  • cargo fmt --all -- --check — passed.
  • cargo clippy --locked -p mxc-sdk -p mxc_ffi -p wxc --all-targets -- -D warnings — passed; build script reported the existing Python Store alias prerequisite warning.
  • Repository root: node scripts/versioning/check-contract-codegen.js — passed; three artifact sets match and published schemas match origin/main.
  • Repository root: git diff --check origin/main..HEAD — passed.

Native Linux/macOS runs and dependency-feed validation have not been run locally; PR CI remains required. Existing CI matrices are preserved. Windows CI requires both fixtures; Linux/macOS CI requires the engine fixture.

Separately supplied live evidence at earlier commit 32ac2531 established exactly one authorized API 2.0 Local checkout and fresh-process deprovision with renewed credentials and the same retained ID. This proves release acceptance only, not completed reset, HumanUser, readiness/exec or SDK behavior. An earlier HumanUser attempt returned HTTP 500/provision_timeout with uncertainty and no session ID and remains unresolved. No additional live calls were made for this review or publication. The backend guide's original statement that live validation has not run predates that supplied evidence.

✅ Checklist

  • Signed the Contributor License Agreement
  • Linked to an issue
  • For features or changes requiring a spec: linked the issue with maintainer-applied feature-accepted
  • For implementation: linked the required approved and merged design/API spec, or explained why no spec is required
  • Feature issue remains open unless this PR completes all agreed implementation, validation, and documentation
  • Updated documentation (if applicable)
  • Updated Copilot instructions (if build, architecture, or conventions changed)
  • If this PR changes Cargo.lock, the dependency-feed-check check passes (see pull request builds)

No new repository-wide architecture or contributor convention requires an instruction update. CLA status and dependency-feed CI are left unconfirmed.

📋 Issue Type

  • Bug fix
  • Feature
  • Task
Microsoft Reviewers: Open in CodeFlow

Add environment-configured checkout and terminal release through the normal engine, preserving session cleanup identities and uncertain outcomes. Expose lifecycle operations through the existing Windows executor only; leave Linux and macOS CLI lifecycle support unchanged. Keep readiness, execution and public SDK access explicitly deferred. Validate parser and engine behavior with cross-platform trusted-TLS fixtures and fresh-process recovery through the Windows executor.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 10, 2026 18:43
@azure-pipelines

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

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.

🟡 Changes recommended

Checkout error responses discard valid session IDs needed for cleanup, and required Azure builds omit lifecycle and recovery fixtures, allowing artifacts to be published without those checks.

2 open issues
What changed in this PR

Implements the experimental W365 Agents checkout and terminal-release lifecycle on the merged Phase 1 contracts, using MXC’s existing engine and Windows executor interfaces.

Changes:

  • Adds an environment-configured API 2.0 client, session-based IDs, checked dispatch and structured recovery errors.
  • Keeps readiness, execution and Dev SDK access unavailable pending later phases.
  • Adds trusted-TLS fixtures, GitHub Actions validation and documentation of configuration, cleanup and current limits.
File Description
src/​mxc-sdk/​tests/​mxc_contract/​v1_1_0_alpha/​w365_agents.rs Updates routing, authorization and SDK-gating assertions.
src/​mxc-sdk/​src/​core/​mxc_engine/​state_aware.rs Adds W365 dispatch and blocks SDK lifecycle access.
src/​mxc-sdk/​src/​core/​mxc_engine/​run.rs Rejects W365 create-and-run execution.
src/​mxc-sdk/​src/​core/​mxc_engine/​backend_registry.rs Registers W365 as experimental and not locally detectable.
src/​mxc-sdk/​src/​core/​mxc_common/​state_aware_request.rs Adds lifecycle phase parsing from strings.
src/​mxc-sdk/​src/​core/​mxc_common/​state_aware_operation.rs Adds passive W365 provision inputs.
src/​mxc-sdk/​src/​core/​mxc_common/​state_aware_dispatch.rs Routes the w365 ID prefix.
src/​mxc-sdk/​src/​core/​mxc_common/​state_aware_binding.rs Adds checked W365 request binding.
src/​mxc-sdk/​src/​core/​mxc_common/​policy_identity.rs Excludes W365 account coordinates from containment policy identity.
src/​mxc-sdk/​src/​core/​mxc_common/​models.rs Adds the W365 backend variant and names.
src/​mxc-sdk/​src/​core/​mxc_common/​config_parser.rs Normalizes W365 lifecycle requests and ID routing.
src/​mxc-sdk/​src/​core/​mxc_common/​config_contract_adapters/​dev/​w365_agents.rs Adapts contract settings into provision inputs.
src/​mxc-sdk/​src/​core/​mxc_common/​config_contract_adapters/​dev/​state_aware.rs Produces W365 lifecycle operations.
src/​mxc-sdk/​src/​core/​mxc_common/​cmdline.rs Selects Windows command syntax for W365.
src/​mxc-sdk/​src/​backends/​w365agents/​tests.rs Adds TLS lifecycle and fresh-executor recovery fixtures.
src/​mxc-sdk/​src/​backends/​w365agents/​service.rs Implements checkout, release, identity validation and service errors.
src/​mxc-sdk/​src/​backends/​w365agents/​mod.rs Implements backend validation and lifecycle methods.
src/​mxc-sdk/​src/​backends/​mod.rs Includes the internal W365 backend module.
src/​mxc-sdk/​Cargo.toml Adds HTTP/TLS dependencies and fixture support.
src/​ffi/​mxc_ffi/​src/​state_aware.rs Tests that W365 SDK access remains unavailable.
src/​Cargo.lock Records updated dependency resolution.
docs/​schema.md Explains W365 development-contract support and limits.
docs/​development/​build-and-test/​ci-validation-infrastructure.md Documents platform fixture coverage.
docs/​development/​architecture/​designs/​1615-w365a-backend/​README.md Updates implementation scope and environment-only configuration.
docs/​development/​architecture/​designs/​1615-w365a-backend/​api.md Clarifies the Phase 2 executor and SDK boundaries.
docs/​containment-policy/​dev/​policy.md Documents experimental checkout and release support.
docs/​backends/​w365agents/​w365agents-backend.md Adds configuration, cleanup and validation guidance.
.github/​workflows/​Build.Windows.Job.yml Requires engine and fresh-executor fixtures.
.github/​workflows/​Build.MacOS.Job.yml Requires the engine TLS fixture.
.github/​workflows/​Build.Linux.Job.yml Requires the engine TLS fixture.

Review effort: Balanced

Comment on lines +175 to +176
cargo test --locked -p mxc-sdk --lib --target '${{ matrix.target }}' w365 -- --include-ignored
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in d2d77aa. Azure now explicitly discovers and runs the engine fixture on Linux x64 and both fixtures on Windows x64. Windows builds its test-support executor in a separate target directory outside shipping artifacts. macOS checks fixture discovery before the existing package suite runs it; cross-compiled Windows/Linux arm64 jobs remain build-only. Template structure and embedded PowerShell/Bash syntax checks passed locally; the updated Azure jobs have not run yet.

Retain valid session identities from bounded checkout error responses without changing HTTP failure classification or replaying allocation. Cover recovery and unreadable bodies in TLS and fresh-executor fixtures, and require fixture discovery and execution on native Azure build hosts.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 10, 2026 20:08

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.

🟡 Changes recommended

The dependency changes introduce unconfigured native TLS/OpenSSL requirements into the ARM64 cross-build, while missing feed dependencies leave the dependency gate failing and block platform builds and W365 fixture verification.

2 open issues
1 previously missed issue

In code that hasn't changed since last review

Medium severity Add locked dependencies to the required dependency feed

src/​mxc-sdk/​Cargo.toml:84

[P2] Add the new locked dependencies to the required dependency feed before merging. The current dependency-feed-check job fails with HTTP 401 and “No local versions of package 'rcgen'” (job 130096170380), which also skips the Windows, Linux and macOS build jobs and their new W365 fixtures. As documented in docs/development/build-and-test/pull-requests.md:228–240, a feed Contributor needs to run MXC-Update-Feed-Dependencies with prNumber=1723, then rerun the dependency gate and affected builds.

1 resolved since last review

Review effort: Balanced

Comment thread src/mxc-sdk/Cargo.toml Resolved
Preserve W365 rustls on all targets while restoring the architecture-specific optional native TLS and gzip features for Hyperlight. Add an Azure ARM64 dependency graph regression check.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 10, 2026 20:38

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.

🔵 Needs a closer look

The dependency feed cannot resolve rcgen, causing the dependency gate to fail and Windows, Linux, and macOS builds—including the new lifecycle fixtures—to be skipped.

1 open issue
1 previously missed issue

In code that hasn't changed since last review

High severity Seed rcgen in MxcDependencies before merging

src/​mxc-sdk/​Cargo.toml:83

[P2] Seed the new dependencies in MxcDependencies before merging. The current dependency-feed-check job (130113697446) fails resolving rcgen with HTTP 401 and the feed explicitly reports “No local versions of package 'rcgen'.” Consequently, the Windows, Linux and macOS build jobs—including the new lifecycle fixtures—are skipped. Have a contributor run MXC-Update-Feed-Dependencies with prNumber=1723, as documented in docs/development/build-and-test/pull-requests.md:228–240, then rerun the gate and platform jobs; successful crates.io builds do not validate this dependency path.

1 resolved since last review

Review effort: Balanced

@tbyfield

Copy link
Copy Markdown
Contributor Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Use platform trust for production HTTPS and replace the rustls test server with native TLS. Generate test certificates at runtime, package Windows identities with OpenSSL, and explicitly clean up imported fixture keys. Add Linux OpenSSL build prerequisites including ARM64 target libraries and update CI dependency checks and documentation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 10, 2026 22:33

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.

🔵 Needs a closer look

Maintainer and service-owner review is needed to confirm outstanding cross-platform TLS/OpenSSL validation, accept the API 2.0 contract and live-validation limits, and reconcile the uncertain HumanUser checkout outcome.

1 open issue

Review effort: Balanced

Package generated EC fixture identities as PKCS12 for macOS import compatibility. Select the Windows OpenSSL executable directly instead of prepending Git Unix tools to PATH, preserving Windows tar for packaging validation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 10, 2026 23:18

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.

🔵 Needs a closer look

Unresolved API 2.0 HumanUser checkout behavior and allocation cleanup risks need service-owner confirmation, while Linux/macOS TLS fixtures and Azure ARM64 OpenSSL builds still lack execution results.

1 open issue

Review effort: Balanced

@tbyfield
tbyfield marked this pull request as ready for review November 11, 2026 00:10
@tbyfield
tbyfield requested a review from a team November 11, 2026 00:10
@tbyfield
tbyfield requested a review from a team as a code owner November 11, 2026 00:10
Comment on lines +7 to +12
included in normal Windows, Linux and macOS builds. Phase 2 exposes checkout
and release through the Windows executor's existing lifecycle interface,
with `--experimental` authorization. The Linux and macOS executors do not
expose lifecycle operations; adding that CLI support is outside this slice.
Local probes do not report W365
availability and never authenticate, discover a pool or check out a Cloud PC.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

question: through the use of the sdk's v1.dev namespace users should be able to actually invoke the w356A backend. They don't have to only go through the executor. Under the hood all the code converges to the same place regardless of whether it comes through the sdk or the executors. So, if we have code in this PR that adds the backend, then we should be able to easily get to it through the sdk as well.

Comment on lines +17 to +18
JSON lifecycle access is also deliberately unavailable until cross-SDK
structured error metadata and public SDK validation are complete.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

question: what do we mean by "structured error metadata and public SDK validation "? My thinking would be that all of this could be done in this PR. If these something I might be missing here?


| Variable | Requirement |
| --- | --- |
| `W365A_CONTROLPLANE_URL` | Bare ASCII DNS hostname, without scheme, port, path, query, credentials or whitespace. HTTPS uses port 443. |

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

note: we probably should have an example of this, just in case someones agent reads this doc to try to figure out how to use the w365A backend.

environment again. MXC does not acquire, cache or renew tokens. The caller
renews credentials before a later cleanup call. Checkout uses the design's
45-second client budget; release uses its separate 30-second budget. These
are transport budgets, not promises about service reset or resource lifetime.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

question: about the budget stuff. By budget do you just mean like networking timeouts?

Comment on lines +60 to +61
## Service contract and identity

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thought: something for the future, I wonder if you'll be able to correlate that the request came from MXC via your apis.

use crate::mxc_common::mxc_error::MxcError;
use crate::mxc_common::network_parser::{parse_network_policy, NetworkSections};
use crate::mxc_common::state_aware_input::StateAwareInput;
#[cfg(test)]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

question: why would we need to move these only for tests?

}

fn state_aware_containment_from_id(sandbox_id: &str) -> Option<wire::Containment> {
fn state_aware_containment_from_id(sandbox_id: &str) -> Option<ContainmentBackend> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

issue: please don't change return types or parameters in this file. The W365A backend shouldn't need to do much in here. The goal of this file is just to parse the configuration, then forward it to the appropriate backend, who is then responsible for doing any extra validations it needs. The expectation is that all the backends implement spawn and run and have them return an error if it's not supported.

cfg: crate::mxc_common::common_request_ir::CommonRequestIR,
logger: &mut Logger,
require_process: bool,
lifecycle_containment: Option<ContainmentBackend>,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

issue: not sure I follow why this was needed here.

Comment on lines -1764 to -1784
/// Temporary routing view; production derives it exclusively from the operation.
struct NormalizationContext<'a> {
phase: Phase,
containment: Option<wire::Containment>,
sandbox_id: Option<&'a str>,
}

fn normalize_state_aware_common(
mut common: crate::mxc_common::common_request_ir::CommonRequestIR,
context: NormalizationContext<'_>,
common: crate::mxc_common::common_request_ir::CommonRequestIR,
phase: Phase,
containment: Option<ContainmentBackend>,
logger: &mut Logger,
) -> Result<ExecutionRequest, WxcError> {
let network_supplied = common.network.is_some();
common.containment = if context.phase.routes_by_containment() {
context.containment
} else {
context.sandbox_id.and_then(state_aware_containment_from_id)
};
let require_process = context.phase == Phase::Exec;
let mut request = normalize_common_request_ir(common, logger, require_process)?;
if context.phase != Phase::Provision && !network_supplied {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

issue: not sure why we have to remove this code. I assumed w365 would follow the same code flow as the other state aware backends.

exec_state_aware_pty_parsed(parsed, &mut logger, size)
}

fn require_sdk_lifecycle_ready(parsed: &ParsedStateAwareRequest) -> Result<(), Error> {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

issue: see my comments about the json dev schema. That is the experimental interface folks can use through the SDK. We can hash out the sdk error details in this PR if you like. I believe we return a stand error object across SDKs.

@microsoft-github-policy-service microsoft-github-policy-service Bot added the Needs-Author-Feedback Waiting for additional information or action from the issue or pull-request author. label Nov 11, 2026
Restore the original parser signatures and normalization context, extending only the internal containment mappings for W365. Preserve backend-owned validation and assert normalized routing and telemetry attribution across the development lifecycle contract.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Use normal run and streaming dispatch while keeping unsupported execution refusal in the W365 backend. Cover dry runs, experimental authorization and zero service requests in the lifecycle fixture.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

@MGudgin Gudge (MGudgin) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Four additional, non-blocking Medium findings, checked against the merge-base 37a3e93 and head 9f4d1ec. Existing duplicate or overlapping feedback is excluded.

Validation evidence: Build run 42922219010 succeeded on this exact head. Its Windows logs show both W365 lifecycle fixtures passing (6 W365 tests passed); Linux x64 and macOS arm64 logs also show the engine fixture passing. The policy and deadline comments identify regression-coverage gaps, not evidence that unsupported policy currently executes or that the configured deadlines currently fail. Local fixture execution was limited by sandbox permissions, so this review relies on those verified CI results rather than claiming an entirely green local run. No live service calls were made; final-head live compatibility is not established.

All four findings are anchored to added lines. No out-of-diff findings are included.

Verified pre-existing - not attributed to this PR

None included; previously raised or resolved issues are not reopened.

shell: bash
run: |
set -euo pipefail
names="$(cargo test --locked -p mxc-sdk --lib --target "${{ matrix.target }}" -- --list)"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Medium (performance) - Reuse the release-profile SDK test artifacts.

Attribution: introduced_by_change - these new fixture commands select a different profile from the adjacent existing tests.

The preceding SDK tests use --release, but both fixture discovery and execution omit it. On a cold runner, even -- --list therefore compiles and links a second, debug-profile SDK test binary and its dependencies. The execution command reuses that debug build; the unnecessary work is the extra profile, not a separate compilation for each command. The new macOS step has the same issue at lines 69-71.

Please add --release to both discovery and execution on Linux and macOS, retaining the target, discovery assertion and --include-ignored. This avoids redundant CI build work; no specific duration increase has been measured.


## Optional live validation: explicit invocation only

**Live Phase 2 validation has not been run.** Do not use another caller's

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Medium (documentation/correctness) - Reconcile the live-validation record with the supplied evidence.

Attribution: claim_mismatch - this newly added guide says live Phase 2 validation has never run, while the PR description explicitly records earlier-commit API 2.0 evidence and acknowledges that this statement predates it.

The PR reports one authorized Local checkout and fresh-process deprovision with renewed credentials and the same retained ID at commit 32ac2531. It separately records an unresolved HumanUser HTTP 500/provision_timeout result, with uncertainty and no session ID. This guide instead describes only incompatible API 3.0 evidence and says API 2.0 checkout/release remains entirely unvalidated. The CI runbook also calls the live validation explicitly unrun.

Please update both records to distinguish the earlier-commit Local checkout/release acceptance from the unresolved HumanUser result and the absence of final-head live validation. That does not establish completed reset/sanitization, other identities or SDK support, and does not require making another live allocation for this review. This is a non-blocking evidence-record correction.

request(phase, A)
};
value[key] = json!({});
assert!(invoke(&value, true).is_err(), "{key}/{phase}");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Medium (correctness/test coverage) - Assert the backend's policy refusal, not merely any error.

Attribution: introduced_by_change - the new unsupported-policy loop accepts failures unrelated to the policy guard it is intended to protect.

Provision/stop inputs can fail during contract parsing, while exec still returns UnsupportedPhase after removing the backend policy check because execution is deferred. Consequently these .is_err() assertions can remain green when that guard is removed. Also, runtimeConfig: {} does not exercise the networkProxy-presence flag checked by the guard. The later zero-request assertion does not distinguish an exec policy rejection from the normal unsupported-exec refusal.

Please add engine-fixture cases for exec with network: {} and with runtimeConfig: {"networkProxy":"http://127.0.0.1:8080"}, asserting MxcErrorCode::PolicyValidation and no service requests. Keep contract-parser rejection cases separate. These checks should fail if the backend policy guard is replaced with success. This is missing regression protection, not a claim that policy is currently executed unenforced.

} // Rejected TLS never carries an HTTP request.
seen.lock().unwrap().push(String::from_utf8(data).unwrap());
if let Some(Some(reply)) = responses.lock().unwrap().pop_front() {
stream.write_all(reply.as_bytes()).unwrap();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Medium (testability) - Exercise expiration of the native transport deadlines.

Attribution: introduced_by_change - the new transport fixtures cover replies and immediate disconnects, but not a service that leaves a request or partial reply open.

The client sets 45-second checkout and 30-second release budgets through timeout_global, but none of these scenarios waits for either budget to expire. An HTTP 408 response is a server response, not a client deadline expiration; dropping the connection is different again. Removing timeout_global would leave the current checks passing. The fresh-executor harness's separate 20-second kill limit cannot verify either longer native deadline.

Please add a hold-open/partial-reply fixture mode and an internal test seam for short budgets. Assert bounded uncertain failure for checkout and release, retained release identity, and exactly one request without automatic replay. Keep the budget override internal/test-only rather than adding public timeout controls. The current deadlines are present; the gap is executable regression coverage for them.

Expose checkout and terminal release through the existing Dev JSON entry points. Include valid Provision cleanup identities in error messages without adding SDK error properties. Isolate W365 HTTPS calls on a joined native thread and cover SDK lifecycle and Koffi coexistence through trusted TLS fixtures.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 12, 2026 01:03

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

Needs-Author-Feedback Waiting for additional information or action from the issue or pull-request author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants