Skip to content

Surface launch diagnostics to SDK callers (e.g. copilot.exe)#353

Merged
bbonaby merged 22 commits into
mainfrom
jsidewhite/mxc_surface_errors_to_copilot
May 22, 2026
Merged

Surface launch diagnostics to SDK callers (e.g. copilot.exe)#353
bbonaby merged 22 commits into
mainfrom
jsidewhite/mxc_surface_errors_to_copilot

Conversation

@jsidewhite

@jsidewhite jsidewhite commented May 20, 2026

Copy link
Copy Markdown
Member

Summary

Add post-failure launch diagnostics that detect known environment issues after CreateProcessW or Experimental_CreateProcessInSandbox fails (or the child exits non-zero), and surface actionable remediation to callers of both wxc-exec.exe and the SDK.

Detected conditions:

  • packaged_app -- MSIX-packaged executables (e.g. Store-installed pwsh.exe) cannot run inside sandboxed containers.
  • missing_filesystem_access -- pwsh.exe < 7.7 requires read-only access to the root drive (C:\) which the sandbox policy may not grant.

Key changes:

  • New shared Rust module launch_diagnostics.rs with detection heuristics, PATH resolution, and unit tests.
  • Both AppContainer and BaseContainer runners enrich error responses and log diagnostics with \Error:\ prefix for red highlighting in the diagnostic console.
  • wxc-exec emits a structured JSON error envelope on stderr for one-shot flows.
  • SDK parses the JSON envelope and rejects with a typed MxcError.

Example messages:

  • Needs UI
image
  • Need C:\ for powershell
image
  • Packaged app
image
  • Need velocity (Gudge had put in a variant of this in – though it is not appearing in copilot.exe for some reason)
image

..and they will also show up with contextual messages in the mxc-diagnostic-console.

image

Closes #348

Microsoft Reviewers: Open in CodeFlow

Add post-failure launch diagnostics that detect known environment issues
after CreateProcessW or Experimental_CreateProcessInSandbox fails (or the
child exits non-zero), and surface actionable remediation guidance.

Detected conditions:
- packaged_app: MSIX-packaged executables (e.g. Store-installed pwsh.exe)
  cannot run inside sandboxed containers.
- missing_filesystem_access: pwsh.exe < 7.7 requires read-only access to
  the root drive (C:\) which the sandbox policy may not grant.

Changes:
- New shared module: wxc_common/src/launch_diagnostics.rs with detection
  heuristics, PATH resolution, and unit tests.
- AppContainer runner: enriches error response and logs diagnostic on
  failure (Error: prefix for red highlighting in diagnostic console).
- BaseContainer runner: same enrichment on both API failure and non-zero
  child exit paths.
- wxc-exec main: emits structured JSON error envelope on stderr for
  one-shot flows so the SDK can parse it programmatically.
- SDK (sandbox.ts): scans for JSON error envelope in PTY output on
  non-zero exit and rejects with a typed MxcError.
- docs/launch-diagnostics.md: design document.

Closes #348

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 20, 2026 00:14
@jsidewhite jsidewhite added the Issue-Feature It's a new feature request label May 20, 2026

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.

Pull request overview

This PR adds “post-failure launch diagnostics” to detect known Windows launch-environment problems (e.g., MSIX-packaged executables, pwsh root-drive access) and attempts to surface them to SDK callers via a structured stderr JSON envelope emitted by wxc-exec.

Changes:

  • Introduces a shared Windows-only Rust module (launch_diagnostics.rs) with heuristics + unit tests for common launch failures.
  • AppContainer and BaseContainer runners append/emit diagnostic remediation text on failures.
  • wxc-exec emits a JSON error envelope on stderr; the TypeScript SDK scans PTY output for the envelope and rejects with MxcError.
Show a summary per file
File Description
src/wxc/src/main.rs Emits a JSON error envelope on stderr when ScriptResponse has an error message.
src/wxc_common/src/lib.rs Exposes the new launch_diagnostics module (Windows-only).
src/wxc_common/src/launch_diagnostics.rs Adds detection heuristics (packaged app, pwsh root readonly) plus unit tests and PATH resolution helper.
src/wxc_common/src/base_container_runner.rs Runs launch diagnostics after Experimental_CreateProcessInSandbox failure and after non-zero child exit.
src/wxc_common/src/appcontainer_runner.rs Runs launch diagnostics after non-zero exit and enriches ScriptResponse error/stderr.
sdk/tests/integration/package-lock.json Adds node-gyp to lockfile devDependencies.
sdk/src/sandbox.ts Parses JSON error envelopes from PTY output and rejects spawnSandboxAsync with MxcError.
docs/launch-diagnostics.md Documents the intended design, envelope shape, and heuristics.

Copilot's findings

Files not reviewed (1)
  • sdk/tests/integration/package-lock.json: Language not supported
  • Files reviewed: 7/8 changed files
  • Comments generated: 8

Comment thread src/wxc/src/main.rs Outdated
Comment on lines +584 to +595
// Emit a structured JSON error envelope on stderr for SDK consumption
// when the runner produced an error message (one-shot flows only).
if response.exit_code != 0 && !response.error_message.is_empty() {
if let Ok(json) = serde_json::to_string(&serde_json::json!({
"error": {
"code": "backend_error",
"message": response.error_message,
}
})) {
eprintln!("{json}");
}
}
Comment thread sdk/src/sandbox.ts
Comment on lines +627 to +646
* Scans a multi-line string for a JSON error envelope emitted by wxc-exec
* on stderr. Returns the first matching envelope, or null if none found.
* The envelope format is: `{"error": {"code": "...", "message": "...", ...}}`
*/
function tryParseErrorEnvelopeFromLines(output: string): MxcError | null {
for (const line of output.split('\n')) {
const trimmed = line.trim();
if (!trimmed.startsWith('{')) continue;
try {
const parsed = JSON.parse(trimmed);
if (parsed && typeof parsed === 'object' && 'error' in parsed) {
const env = parsed.error;
if (env && typeof env.code === 'string' && typeof env.message === 'string') {
return mxcErrorFromCode(env.code, env.message, env.details);
}
}
} catch {
// Not valid JSON on this line, continue scanning.
}
}
Comment on lines +602 to +603
let bare_exe =
std::path::Path::new(request.script_code.split_whitespace().next().unwrap_or(""));
Comment on lines +644 to +669
// Post-failure diagnostics: if the child failed, check for known
// environment issues and enrich the error message with actionable
// guidance.
if response.exit_code != 0 {
let bare_exe =
std::path::Path::new(request.script_code.split_whitespace().next().unwrap_or(""));
let resolved_exe = crate::launch_diagnostics::resolve_exe_on_path(bare_exe);
if let Some(diag) = diagnose_launch_failure(
&resolved_exe,
&request.policy.readonly_paths,
Some(response.exit_code as u32),
) {
let diagnostic_text = format_diagnostic(&diag);
logger.log_line(&format!(
"Error: Launch diagnostic [{}]: {}",
diag.kind, diag.message
));
logger.log_line(&format!("Error: Remediation: {}", diag.remediation));
if response.error_message.is_empty() {
response.error_message = diagnostic_text.trim().to_string();
} else {
response.error_message.push_str(&diagnostic_text);
}
response.standard_err.push_str(&diagnostic_text);
}
}
Comment on lines +88 to +109
/// Attempt to resolve a potentially bare executable name (e.g. `pwsh.exe`)
/// to its full path by searching the system PATH. Returns the original path
/// if resolution fails or the input is already absolute.
pub fn resolve_exe_on_path(exe: &Path) -> std::path::PathBuf {
// If already absolute, return as-is.
if exe.is_absolute() {
return exe.to_path_buf();
}

// Search PATH for the executable.
if let Some(path_var) = std::env::var_os("PATH") {
for dir in std::env::split_paths(&path_var) {
let candidate = dir.join(exe);
if candidate.exists() {
return candidate;
}
}
}

// Fallback: return the original (detection will be best-effort).
exe.to_path_buf()
}
"devDependencies": {
"@types/node": "^20.10.0",
"@types/semver": "^7.7.1",
"node-gyp": "^12.2.0",
Comment on lines +678 to +697
// Post-failure diagnostics: if the child failed, check for known
// environment issues and enrich the error message.
let mut error_message = String::new();
let mut standard_err = String::new();
if exit_code != 0 {
if let Some(diag) = diagnose_launch_failure(
&resolved_exe,
&request.policy.readonly_paths,
Some(exit_code),
) {
let _ = writeln!(
logger,
"Error: Launch diagnostic [{}]: {}",
diag.kind, diag.message
);
let _ = writeln!(logger, "Error: Remediation: {}", diag.remediation);
let diagnostic_text = format_diagnostic(&diag);
error_message = diagnostic_text.trim().to_string();
standard_err = diagnostic_text;
}
Comment thread sdk/src/sandbox.ts
Comment on lines +626 to +646
/**
* Scans a multi-line string for a JSON error envelope emitted by wxc-exec
* on stderr. Returns the first matching envelope, or null if none found.
* The envelope format is: `{"error": {"code": "...", "message": "...", ...}}`
*/
function tryParseErrorEnvelopeFromLines(output: string): MxcError | null {
for (const line of output.split('\n')) {
const trimmed = line.trim();
if (!trimmed.startsWith('{')) continue;
try {
const parsed = JSON.parse(trimmed);
if (parsed && typeof parsed === 'object' && 'error' in parsed) {
const env = parsed.error;
if (env && typeof env.code === 'string' && typeof env.message === 'string') {
return mxcErrorFromCode(env.code, env.message, env.details);
}
}
} catch {
// Not valid JSON on this line, continue scanning.
}
}
Comment thread docs/launch-diagnostics.md Outdated
@@ -0,0 +1,120 @@
# Launch Diagnostics: Surfacing Errors to Callers (#348)

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.

suggestion: can we move this under a new folder called diagnostics under docs? Also can we have an AI agent add a collapable section in the sdk/README.md file about error handling based on this doc? This would appear in the README file in https://www.npmjs.com/package/@microsoft/mxc-sdk and would help future users.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

i deleted this file - it makes no sense for each minor feature to have an .md. i forgot to nuke it earlier.

Comment thread docs/launch-diagnostics.md Outdated
) -> Option<LaunchDiagnostic> { ... }
```

**Checks (in order):**

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 need a follow up for Linux + MacOS I assume. Might need to add a GitHub issue for that. That said, not sure this would work for ones like wslc, nanix or hyperlight.

Comment thread docs/launch-diagnostics.md Outdated
When a `ScriptResponse` contains a diagnostic, emit a JSON error envelope on **stderr**:

```json
{"error": {"code": "backend_error", "message": "...", "details": {"kind": "packaged_app", "remediation": "..."}}}

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: We need to watch out for code paths that just print an error and then do exit(-1), if we're now formalizing how we handle errors.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ScriptResponse is pre-existing, but end-to-end should be formalized yes. it's very confusing.

Comment thread docs/launch-diagnostics.md Outdated
```rust
fn is_packaged_app(exe_path: &Path) -> bool {
let normalized = exe_path.to_string_lossy().to_lowercase();
normalized.contains("\\windowsapps\\")

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: For this snippet and below can't remember but do we allow forward slashes in the path? if so we probably need to detect which one they're using and then do the check right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fxied

Comment thread docs/launch-diagnostics.md Outdated
}
```

Version detection is deferred -- the simpler heuristic is path + filename based. Note from @asklar: this does not apply to `powershell.exe` (inbox Windows PowerShell 5.x), only to `pwsh.exe` < 7.7.

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: we probably can remove the Note: part

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

nuked file. sry!

Comment thread sdk/src/sandbox.ts
// All output is combined
//
// Check for structured error envelopes from wxc-exec on failure.
if (event.exitCode !== 0) {

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: don't we also need this in these places?

  1. function spawnWithConfig(
  2. export function spawnSandboxFromConfig(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

i dont think you can because PTY

@bbonaby bbonaby May 22, 2026

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.

Hmm but there is a ptyProcess return in spawnWithConfig though. Users Can use spawnSandbox by itself rather than spawnSandboxAsync . Same thing with spawnSandboxFromConfig there is a ptyProcess option in there too.

EDIT: Oh wait, I might have misinterpreted what you said. I thought you meant, that we can't because they use PTY

Comment thread sdk/src/sandbox.ts
* on stderr. Returns the first matching envelope, or null if none found.
* The envelope format is: `{"error": {"code": "...", "message": "...", ...}}`
*/
function tryParseErrorEnvelopeFromLines(output: string): MxcError | null {

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.

suggestion: can this be moved out of sandbox.tx and into the helper.ts function? this seems like it can be reused by the state-aware.ts file as well.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

wontfix. because no time.

Comment on lines +621 to +625
let resolved_exe = {
let bare =
std::path::Path::new(request.script_code.split_whitespace().next().unwrap_or(""));
crate::launch_diagnostics::resolve_exe_on_path(bare)
};

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.

suggestion: seems like this can move line 573 since it's used inside the success == 0 and success != 0 lines

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

refactored this mess to be smaller and better

Comment on lines +683 to +696
if let Some(diag) = diagnose_launch_failure(
&resolved_exe,
&request.policy.readonly_paths,
Some(exit_code),
) {
let _ = writeln!(
logger,
"Error: Launch diagnostic [{}]: {}",
diag.kind, diag.message
);
let _ = writeln!(logger, "Error: Remediation: {}", diag.remediation);
let diagnostic_text = format_diagnostic(&diag);
error_message = diagnostic_text.trim().to_string();
standard_err = diagnostic_text;

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.

suggestion: also seems like these lines are good candidates to put in a common helper function since they're also repeated for both the success and failure cases

Comment on lines +114 to +148
fn is_packaged_app(exe_path: &Path) -> bool {
let normalized = exe_path.to_string_lossy().to_lowercase();
normalized.contains("\\windowsapps\\")
}

/// Returns `true` when the executable is `pwsh.exe` and the sandbox policy
/// does not grant read-only access to the drive root.
///
/// Note: This does NOT apply to `powershell.exe` (inbox Windows PowerShell 5.x)
/// and does NOT apply to `pwsh.exe` >= 7.7-preview1.
fn missing_root_readonly(exe_path: &Path, readonly_paths: &[String]) -> bool {
let filename = exe_path
.file_name()
.unwrap_or_default()
.to_string_lossy()
.to_lowercase();
if filename != "pwsh.exe" {
return false;
}
let root = drive_root(exe_path);
!readonly_paths
.iter()
.any(|p| p.eq_ignore_ascii_case(&root) || p == "\\")
}

/// Extract the drive root (e.g. `C:\`) from an absolute path. Falls back to
/// `C:\` when the path is relative or otherwise cannot be parsed.
fn drive_root(exe_path: &Path) -> String {
let s = exe_path.to_string_lossy();
if s.len() >= 3 && s.as_bytes()[1] == b':' {
format!("{}\\", &s[..2])
} else {
"C:\\".to_string()
}
}

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: see my suggestion about forward slashes too in the doc

@bbonaby
bbonaby merged commit 6cf146b into main May 22, 2026
18 checks passed
@bbonaby
bbonaby deleted the jsidewhite/mxc_surface_errors_to_copilot branch May 22, 2026 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Issue-Feature It's a new feature request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants