Add --wait-for-debugger flag for BaseContainer/AppContainer runners

[Alexander Sklar (asklar)]

📖 Description

Adds a Windows-only --wait-for-debugger CLI flag to wxc-exec.exe for the BaseContainer/AppContainer (processcontainer) backends. When set, the sandboxed child is left suspended right after creation — before it has executed any code at all, not even its own loader init — instead of being resumed immediately. This lets an external, unsandboxed debugger attach to the real sandboxed process by PID at its own pace (no timeout, since nothing can run until the debugger releases it).

wxc-exec polls CheckRemoteDebuggerPresent (no timeout) and, the instant a debugger attaches, clears only its own CREATE_SUSPENDED increment — leaving the debugger's own attach-time freeze in place, so a plain g is enough afterward (no ~0 m needed). Stderr/log output on suspend/resume is a single concise line (Process suspended, PID: <pid>... / Process started, PID: <pid>.) rather than a long banner.

Changes are scoped to just this feature: the new CLI flag, the ExecutionRequest.wait_for_debugger field threaded through the wire/domain config, and the two BaseContainer/AppContainer suspend points. README.md is updated with a new "Wait for Debugger" subsection under Debugging.

🔗 References

🔍 Validation

• cargo build -p wxc -p appcontainer_common --target x86_64-pc-windows-msvc (debug) — passes
• cargo fmt --all -- --check — passes
• cargo clippy --target x86_64-pc-windows-msvc --locked --all-targets --all-features --release -- -D warnings (full workspace, matching CI's Windows lint job exactly) — passes with zero warnings
• Manual: built wxc-exec.exe and confirmed it runs

✅ Checklist

• Signed the Contributor License Agreement
• Linked to an issue
• 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 docs/pull-requests.md)

📋 Issue Type

• Bug fix
• Feature
• Task

Microsoft Reviewers: Open in CodeFlow

[Copilot]

Pull request overview

Adds debugger-attach waiting for Windows ProcessContainer backends.

Changes:

• Adds and propagates --wait-for-debugger.
• Polls for debugger attachment before resuming sandbox execution.
• Documents the debugging workflow.

Show a summary per file

File	Description
src/core/wxc/src/main.rs	Adds the CLI flag.
src/core/wxc_common/src/models.rs	Adds request state.
src/core/wxc_common/src/config_parser.rs	Initializes the flag.
src/backends/appcontainer/common/src/lib.rs	Exposes debugger support.
src/backends/appcontainer/common/src/debugger_wait.rs	Implements attach polling and resume.
src/backends/appcontainer/common/src/base_container_runner.rs	Integrates BaseContainer behavior.
src/backends/appcontainer/common/src/appcontainer_runner.rs	Integrates AppContainer behavior.
README.md	Documents usage.

Review details

Comments suppressed due to low confidence (2)

src/backends/appcontainer/common/src/debugger_wait.rs:93

• ResumeThread reports failure with u32::MAX, but this result is discarded and the helper unconditionally logs that the process started. In the AppContainer path this also bypasses the existing SpawnedChild::resume error handling, leaving the child suspended and causing the subsequent wait to hang or time out. Return and handle this error before logging success.

    unsafe {
        ResumeThread(thread);
    }
    let _ = writeln!(logger, "Process started, PID: {pid}.");

src/backends/appcontainer/common/src/debugger_wait.rs:93

• The PR promises both suspend and resume status on stderr, but only the suspend banner calls eprintln!. In the normal non---debug path the logger uses Mode::Buffer, and that buffer is not emitted after a successful run, so users never see Process started... on stderr. Mirror the suspend path for the resume message.

    let _ = writeln!(logger, "Process started, PID: {pid}.");

• Files reviewed: 8/8 changed files
• Comments generated: 4
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (8)

src/core/wxc/src/main.rs:155

• This help text describes the opposite workflow from the implementation: wait_for_debugger_then_resume calls ResumeThread as soon as attachment is detected, so users should issue plain g, not manually run ~0 m. As written, --help instructs users to resume an already-cleared hold.

    /// wxc-exec never calls ResumeThread in this mode. Attaching a debugger
    /// adds its own +1 to the suspend count (symmetric with detach); after
    /// setting breakpoints (necessarily deferred/pending ones, since no
    /// dependent DLL past the main image has loaded yet), clear wxc-exec's
    /// original CREATE_SUSPENDED increment yourself — e.g. WinDbg/cdb's
    /// `~0 m` ("Resume Thread") — then `g`. Windows-only; only the

README.md:238

• The documented attach procedure contradicts the implementation and PR behavior: wxc-exec automatically clears its own suspend hold when it detects attachment. Asking users to run ~0 m is therefore incorrect; they should only need g after setting breakpoints.

Attach a debugger to the printed PID and set breakpoints (necessarily deferred/pending ones, since no dependent DLL past the main image has loaded yet), then clear `wxc-exec`'s own suspend hold — e.g. WinDbg/cdb's `~0 m` ("Resume Thread") — and `g`. Windows-only; only the BaseContainer/AppContainer (`processcontainer`) backends implement the suspend point.

src/backends/appcontainer/common/src/debugger_wait.rs:81

• This loop has no terminal path except observing an attached debugger. If the suspended child is killed before attachment, its process handle remains valid but debugger presence stays false; likewise a permanent CheckRemoteDebuggerPresent error is retried forever. In either case wxc-exec hangs instead of reporting that the attach wait can no longer succeed. Monitor process termination and propagate API failures.

    loop {
        let mut present = BOOL(0);
        // SAFETY: `process` is a valid, open handle for the duration of this
        // call (caller contract); `present` is a valid out-param on the stack.
        let checked = unsafe { CheckRemoteDebuggerPresent(process, &mut present) };
        if checked.is_ok() && present.as_bool() {
            break;
        }
        std::thread::sleep(POLL_INTERVAL);

src/core/wxc_common/src/models.rs:750

• This field documentation is stale relative to the new helper: the helper does call ResumeThread immediately after detecting an attached debugger, so the operator is not expected to clear the original hold with ~0 m. Keeping the domain contract accurate matters for other ExecutionRequest consumers.

    /// Set by `--wait-for-debugger`: never resume the sandboxed child's main
    /// thread after creation (it stays at its `CREATE_SUSPENDED` suspend
    /// count, having executed no code at all — not even its own loader
    /// init). An external, unsandboxed debugger attaches to the real PID at
    /// its own pace, sets breakpoints (necessarily deferred/pending, since
    /// no dependent DLL past the main image has loaded), then clears the
    /// suspend count itself (e.g. WinDbg/cdb's `~0 m` "Resume Thread") and
    /// `g`oes. `wxc-exec` never calls `ResumeThread` in this mode; it just
    /// falls into its normal (already-unbounded-by-default) wait for the
    /// child to exit. `false` (default) is the unchanged, immediate-resume
    /// behavior. Consumed by the BaseContainer/AppContainer runners.

src/core/wxc/src/main.rs:1048

• The flag is accepted for every Windows request, but only ProcessContainer runners inspect the field, and the state-aware branch exits before this assignment. Thus --wait-for-debugger with Windows Sandbox/WSLC/IsolationSession (including state-aware requests) succeeds while the workload runs immediately, violating the flag's contract. Reject the flag outside one-shot ProcessContainer, as --audit already does.

    #[cfg(target_os = "windows")]
    {
        request.wait_for_debugger = cli.wait_for_debugger;
    }

src/backends/appcontainer/common/src/debugger_wait.rs:92

• The return value from ResumeThread is discarded. On failure (u32::MAX), the child remains suspended, but the helper logs that it started and the runner then waits indefinitely. The existing AppContainer SpawnedChild::resume path checks this result and terminates the child; the debugger path needs equivalent error propagation and cleanup.

    unsafe {
        ResumeThread(thread);
    }

src/backends/appcontainer/common/src/debugger_wait.rs:93

• The PR promises a concise resume message on stderr/log output, but this line writes only to the logger. In the normal buffered CLI mode the logger buffer is not printed on success, so the operator sees the suspended message but never Process started... after attachment.

    let _ = writeln!(logger, "Process started, PID: {pid}.");

src/backends/appcontainer/common/src/base_container_runner.rs:1182

• The BaseContainer creation code at lines 916-920 explicitly allows Experimental_CreateProcessInSandbox to ignore CREATE_SUSPENDED, but the new wait branch assumes the child is suspended. On such an OS build the workload executes immediately (breaking this flag's core guarantee), and if it exits before attachment the new poll loop waits forever. The BaseContainer path needs to establish that this API/build honors suspension or reject --wait-for-debugger for that tier.

        if request.wait_for_debugger {
            crate::debugger_wait::wait_for_debugger_then_resume(
                pi.hProcess,
                pi.hThread,
                pi.dwProcessId,
                logger,
            );

• Files reviewed: 8/8 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (6)

src/core/wxc/src/main.rs:154

• The CLI help still describes the old manual-resume workflow, but wait_for_debugger_then_resume now polls for attachment and calls ResumeThread itself. This tells users to issue ~0 m and explicitly promises that wxc-exec never resumes the thread, contradicting the behavior of the flag.

    /// wxc-exec never calls ResumeThread in this mode. Attaching a debugger
    /// adds its own +1 to the suspend count (symmetric with detach); after
    /// setting breakpoints (necessarily deferred/pending ones, since no
    /// dependent DLL past the main image has loaded yet), clear wxc-exec's
    /// original CREATE_SUSPENDED increment yourself — e.g. WinDbg/cdb's

README.md:238

• The user guide still instructs operators to clear the suspend count manually, although the new helper automatically calls ResumeThread immediately after detecting the debugger. This directly conflicts with the PR's intended plain-g workflow and the implemented behavior.

Attach a debugger to the printed PID and set breakpoints (necessarily deferred/pending ones, since no dependent DLL past the main image has loaded yet), then clear `wxc-exec`'s own suspend hold — e.g. WinDbg/cdb's `~0 m` ("Resume Thread") — and `g`. Windows-only; only the BaseContainer/AppContainer (`processcontainer`) backends implement the suspend point.

src/backends/appcontainer/common/src/debugger_wait.rs:94

• Unlike the suspension line, the “Process started” line is only sent through Logger (stdout/buffer) and never to stderr. This does not provide the stated suspend/resume status on stderr, so terminal users watching stderr can miss the transition.

    let _ = writeln!(logger, "Process started, PID: {pid}.");

src/core/wxc_common/src/models.rs:750

• This field documentation contradicts its implementation: both process-container runners call wait_for_debugger_then_resume, which resumes once as soon as an attachment is detected. Keeping the old ~0 m instructions here makes the domain contract inaccurate.

    /// loaded), then clears the suspend count itself (e.g. WinDbg/cdb's
    /// `~0 m` "Resume Thread") and `g`oes. `wxc-exec` never calls
    /// `ResumeThread` in this mode; it just falls into its normal
    /// (already-unbounded-by-default) wait for the child to exit. `false`

src/core/wxc/src/main.rs:1048

• The flag is copied onto every one-shot Windows request without validating request.containment. All non-ProcessContainer runners ignore this field, so --wait-for-debugger can silently launch the target immediately for Windows Sandbox, WSLC, IsolationSession, or MicroVM. Reject incompatible backends, as the nearby --audit path does, rather than silently defeating an explicit suspension request.

    #[cfg(target_os = "windows")]
    {
        request.wait_for_debugger = cli.wait_for_debugger;
    }

src/backends/appcontainer/common/src/debugger_wait.rs:69

• The new debugger-attachment control flow has no tests, although both runner modules have unit-test suites. In particular, attachment polling, API failures, and the single-resume/status behavior are unverified; the unchecked ResumeThread failure in this implementation illustrates the regression risk. Introduce an injectable Win32 wrapper or equivalent seam and cover success and failure paths.

pub fn wait_for_debugger_then_resume(
    process: HANDLE,
    thread: HANDLE,
    pid: u32,
    logger: &mut Logger,
) {

• Files reviewed: 8/8 changed files
• Comments generated: 2
• Review effort level: Medium

[Copilot]

Review details

Comments suppressed due to low confidence (5)

src/core/wxc/src/main.rs:162

• --wait-for-debugger is accepted for every Windows request, but only the ProcessContainer dispatch reads wait_for_debugger; Windows Sandbox, WSLC, IsolationSession, MicroVM, and state-aware requests silently continue without suspending anything (the state-aware branch exits before line 1049 is reached). This violates the flag's core guarantee and can run the child before the operator attaches. Reject the flag unless the parsed request is a one-shot ProcessContainer request, as --audit already does for its backend-specific contract.

    #[arg(long = "wait-for-debugger")]
    wait_for_debugger: bool,

src/backends/appcontainer/common/src/debugger_wait.rs:95

• The loop still does not detect that the child has exited. A Windows process handle remains valid and becomes signaled after termination, so killing the suspended child does not make this handle “go bad”; CheckRemoteDebuggerPresent can continue reporting success with present == false, leaving wxc-exec asleep forever. Poll the process handle for a signaled state (for example, WaitForSingleObject(process, 0)) alongside the debugger check and return an error when it has exited.

        match checked {
            Ok(()) if present.as_bool() => break,
            Ok(()) => std::thread::sleep(POLL_INTERVAL),

src/backends/appcontainer/common/src/debugger_wait.rs:132

• This only writes the resume message through Logger. In the normal CLI mode the logger is buffered and its buffer is not printed on successful completion, so stderr never receives the claimed Process started, PID: ... line (and in --debug mode it goes to stdout). Emit this line to stderr as the suspended banner does if the PR's stated stderr contract is intended.

    let _ = writeln!(logger, "Process started, PID: {pid}.");

src/backends/appcontainer/common/src/base_container_runner.rs:1175

• This entire wait_for_debugger branch is unreachable: the only caller of private spawn_base is SandboxBackend::spawn, which calls self.validate(request)? first, and the new validation rejects every request for which this condition is true (lines 1293-1302). Keeping a second 50-line wait/error/cleanup path here duplicates launch cleanup without providing the claimed bypass defense; retain the original unconditional resume and let validation remain the single BaseContainer guard.

        // `--wait-for-debugger`: block (no timeout) until an external
        // debugger attaches to the real sandboxed PID, then clear only
        // wxc-exec's own CREATE_SUSPENDED hold — see `debugger_wait` module
        // docs for why this leaves the debugger's own attach-time freeze in
        // place (so a plain `g` is enough, no `~0 m` needed). Unreachable in

src/backends/appcontainer/common/src/appcontainer_runner.rs:1354

• The indefinite wait occurs before AppContainerSandboxProcess is constructed, so its Drop implementation cannot terminate the job on interruption. Contrary to the helper's module comment, AppContainer never registers sandbox_tracking cleanup (that handler is BaseContainer-only), and UiJobObject does not enable kill-on-job-close. If wxc-exec receives Ctrl-C/console close or otherwise exits during this wait, closing these ordinary handles can leave the child suspended and orphaned indefinitely. Arm a cleanup guard or register the live job/process with the console handler before blocking.

        if request.wait_for_debugger {
            if let Err(e) = crate::debugger_wait::wait_for_debugger_then_resume(
                child.process.get(),
                child.thread.get(),
                child.pid,
                logger,
            ) {

• Files reviewed: 9/9 changed files
• Comments generated: 0 new
• Review effort level: Medium

[Gudge (MGudgin)]

Adversarial multi-axis review (9 axes). Three merge-blocking findings inline below; the first two are the same 20-line loop viewed from different angles. The suspend-count design itself is correct and no axis disputed it — these are all about the loop having no exits.

[Gudge (MGudgin)]

Ctrl-C during the attach-wait orphans the suspended child permanently.

The module doc (lines 41-44) says this case is covered:

Ctrl-C during the attach-wait is handled by the existing sandbox_tracking console-control handler (registered well before this point), which runs on its own OS thread independent of this poll loop.

That handler runs, but it doesn't touch the child. Tracing what it actually does:

• dacl_ctrl_handler (src/core/wxc/src/main.rs) emits cancellation telemetry, drops the parked DaclManager to restore host ACEs, and cancels an in-flight audit trace. It never terminates the child.
• sandbox_tracking::cleanup_sandbox handles identity/SID/profile teardown, not process termination.
• The job object is never given JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE — job_object.rs only ever calls SetInformationJobObject(JobObjectBasicUIRestrictions) — so closing the job handle at exit doesn't reap it either.

Net effect: wxc-exec exits, the child's handles close, and a process that has never executed a single instruction (not even loader init) is left suspended at suspend-count 1 with no thread to resume it. It survives until someone finds it in Task Manager. This is squarely on the happy path for the feature's intended use — an operator starts the wait, changes their mind, and hits Ctrl-C. Each abandoned attempt leaks another one, and because they're pre-loader-init they're easy to miss.

Two options, both cheap:

1. Kill from the signal path. JobObject::terminate() already exists (job_object.rs:282). Park the job the same way the DaclManager is parked, and call it from dacl_ctrl_handler before yielding to the default handler. This mirrors the existing register_ctrl_c_cleanup/unregister_ctrl_c_cleanup shape in sandbox_tracking.rs:250-264, so it fits the established pattern.
2. Let the OS do it. Add JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE to the job's extended limits when wait_for_debugger is set. Fewer moving parts and it also covers the TerminateProcess-on-parent case that the comment block in main.rs notes bypasses every handler — but it needs a check that it doesn't disturb the normal (non-debugger) lifetime expectations.

I'd lean toward (2) gated on the flag, with (1) as the fallback if kill-on-close turns out to interact badly with the existing teardown ordering.

Worth pairing with a regression test: launch with --wait-for-debugger, send Ctrl-C during the wait, assert the PID printed in the Process suspended, PID: <pid> banner no longer exists.

Either way, please also fix the lines 41-44 comment — right now it documents this as solved, which is what would stop the next reader from noticing.

[Gudge (MGudgin)]

If the suspended child dies before a debugger attaches, this loop spins forever.

The Err arm is doing the fail-closed work here, and its comment states the trigger:

The handle went bad (e.g. the still-suspended child was terminated externally) -- this can never resolve on its own, so fail closed instead of polling forever.

But the handle can't go bad. wait_for_debugger_then_resume's own # Safety contract requires the caller to keep process open for the entire call, and that's exactly what keeps the kernel object alive after the process dies. A HANDLE to a terminated process is still a perfectly valid handle — the object stays referenced until the last handle closes. CheckRemoteDebuggerPresent succeeds on it and reports present = FALSE.

So on external termination the loop takes the Ok(()) arm, sleeps 100 ms, and repeats — forever. The Err arm never fires for the one scenario it was written for, and wxc-exec hangs instead of failing closed. Worth noting the module doc at lines 47-53 builds the whole failure-handling story on this premise, so the design intent is right and only the mechanism is wrong.

WaitForSingleObject fixes it and folds the sleep into the same call, so the loop gets a liveness check for free:

match checked {
    Ok(()) if present.as_bool() => break,
    Ok(()) => {
        // Doubles as the poll interval: returns early only if the child
        // died, which can never resolve into an attach.
        if unsafe { WaitForSingleObject(process, POLL_INTERVAL.as_millis() as u32) }
            == WAIT_OBJECT_0
        {
            return Err(WxcError::Process(format!(
                "PID {pid} exited before a debugger attached"
            )));
        }
    }
    Err(e) => { /* unchanged */ }
}

Keep the Err arm — it's still correct for genuine API failures, it just isn't the path that catches a dead child.

This is also the cheapest of the findings to lock in with a test: spawn any suspended process, TerminateProcess it, and assert the wait returns an error rather than hanging (with a test timeout, so a regression fails loudly instead of wedging CI).

[Gudge (MGudgin)]

--wait-for-debugger is accepted for every backend but only honored by AppContainer.

wait_for_debugger lives on the shared ExecutionRequest root (models.rs:756) and this line sets it unconditionally for every one-shot request, with no reference to request.containment. A full-repo grep shows only three files ever read it back:

• appcontainer_runner.rs:1348 — honors it
• base_container_runner.rs:1293 — rejects it
• dispatcher.rs:343 — steers tier selection away from BaseContainer

Windows Sandbox, IsolationSession, Hyperlight, MicroVM, and WSLC never look at it. Those backends are dispatched directly rather than through appcontainer::dispatcher, so wxc-exec --wait-for-debugger against any of them prints nothing, suspends nothing, and runs the workload straight through. The operator is sitting there waiting to attach to a process that already came and went.

What makes this worth fixing rather than documenting is that the PR already establishes the right convention one file over. BaseContainerRunner::validate goes out of its way to fail closed with a genuinely excellent error — it explains the CREATE_SUSPENDED guarantee problem and names the workaround. Five sibling backends get silence on identical input. That asymmetry is the actual defect; either behavior would be defensible if it were uniform.

Suggest hoisting the check to wxc_common::validator::validate_common, rejecting when wait_for_debugger && containment != ContainmentBackend::ProcessContainer, in the spirit of the BaseContainer message:

--wait-for-debugger is only supported by the processcontainer backend (AppContainer tier); <backend> provides no pre-execution suspend point.

That's one check covering every present and future backend, and it makes the BaseContainerRunner::validate rejection a narrower special case (BaseContainer is processcontainer, but its specific tier can't honor it) rather than the only guard rail in the system. It also closes the same gap on the state-aware path, which currently never copies the flag onto the request at all (main.rs:1028-1029 propagates --experimental and --dry-run but not this one) — harmless today since state-aware never routes to processcontainer, but it's the same root cause and a central check makes it moot.

If you'd rather keep it permissive, then at minimum the README section needs to say the flag is inert outside processcontainer — right now it reads as a general wxc-exec capability.

[Gudge (MGudgin)]

Two follow-on findings from the same review, both about verification rather than defects: the wait loop has no test seam, and the feature's runtime behavior has no automated coverage at all.

[Gudge (MGudgin)]

The wait loop's state machine can't be tested without a real debugger and a real process.

wait_for_debugger_then_resume reaches straight through to CheckRemoteDebuggerPresent, ResumeThread, TerminateProcess, and std::thread::sleep, all inline. That bakes four decisions into a function whose actual logic — poll until attached, resume exactly once, terminate and fail closed on error — is pure state machine and would be trivially testable on its own.

The practical cost is visible in this PR: the branch coverage that exists is on the guards around the feature (dispatcher::tests::wait_for_debugger_skips_base_container_even_when_usable, validate_runner_rejects_wait_for_debugger), because those are the only parts that could be tested without an OS-level fixture. The behavior that actually implements the feature has none. The two hang findings on this PR are both single-branch defects that a mocked loop would have caught immediately.

A closure-based seam is enough here — no trait or new module needed:

fn wait_loop(
    mut is_attached: impl FnMut() -> Result<bool, WxcError>,
    mut is_alive:    impl FnMut() -> bool,
    mut tick:        impl FnMut(),
) -> Result<(), WxcError> { /* the loop, and only the loop */ }

The Win32 version passes real closures; tests pass canned sequences ([false, false, true], is_alive flipping to false mid-wait, is_attached returning Err) and a no-op tick, so the whole matrix runs in microseconds with no child process at all. The public wait_for_debugger_then_resume signature doesn't have to change.

I'd treat this as a should-fix rather than a merge blocker — but it's the difference between the two hang fixes being verified and being merely believed, so it's worth doing in the same change rather than as follow-up.

[Gudge (MGudgin)]

The feature's entire runtime behavior ships with zero automated coverage.

The PR's validation section lists build, fmt, clippy, and "built wxc-exec.exe and confirmed it runs". The tests that did land cover backend selection — that the dispatcher skips BaseContainer and that validate rejects it. Nothing exercises what happens after the process is created suspended.

Concretely, all of these would ship green today:

• ResumeThread called twice, or not at all (the suspend-count reasoning in the module doc is the subtlest part of this PR and is entirely unguarded)
• the terminate-on-error path silently not terminating, leaving the fail-closed contract unmet
• the flag not reaching ExecutionRequest from the CLI at all
• the parsed default flipping from false to true

That last pair is the part I'd push on regardless of how the seam question (see the testability comment above) is resolved, because it needs no Win32 fixture at all:

• a main.rs CLI test asserting --wait-for-debugger present/absent maps to request.wait_for_debugger true/false — main.rs already has CLI parsing tests to extend
• a config_parser.rs assertion that the default is false — the parser suite already constructs ExecutionRequest and now silently carries this field

Both are a few lines each and lock in the plumbing half of the feature.

For the Win32 half, if the loop gets a closure seam then the interesting cases (not-attached-then-attached, resume failure, check failure, child dies mid-wait) are all cheap unit tests. Without a seam, the fallback is a wxc_e2e_tests case that launches a trivial AppContainer process with the flag, asserts the thread's suspend count is non-zero, attaches with DebugActiveProcess from the test itself, and asserts the process then runs to completion — real coverage, but meaningfully more machinery than the seam route, which is part of why the seam is worth adding first.

[Gudge (MGudgin)]

Last two, both non-blocking: dead code left behind by the BaseContainer routing fix, and a note that the loop still has no cooperative exit even once the two hangs are fixed.

[Gudge (MGudgin)]

The prefer_base_container fix made this branch dead code — worth deleting it now.

Following on from the thread above: routing --wait-for-debugger away from BaseContainer via prefer_base_container = !request.wait_for_debugger is the right fix, and the validate() rejection is a sensible backstop for direct callers. Together they mean this branch in spawn_base can no longer execute. spawn_base is private, its only caller runs self.validate(request) first, and validate rejects the flag unconditionally at line 1293 — so request.wait_for_debugger is provably false by the time control reaches line 1181.

That leaves ~50 lines of unreachable code carrying its own error strings (WAIT_FOR_DEBUGGER_FAILED_MSG), its own teardown sequence, and its own ScriptResponse construction, all of which have to be kept in step with the job-setup cleanup path next to them and with the AppContainer copy of the same logic. Defense-in-depth is cheap when it's an assertion; it's less cheap when it's a full parallel error path that no test can reach and no reader can verify.

Suggest deleting the branch and keeping the unconditional resume. If you'd rather keep a tripwire for the "someone refactors spawn_base's caller and drops the validate call" scenario, a one-liner does the same job without the maintenance surface:

debug_assert!(
    !request.wait_for_debugger,
    "wait_for_debugger must be rejected by validate() before reaching spawn_base"
);

Non-blocking — but the two guards you added are strictly better than this branch, and leaving all three in place obscures which one is actually load-bearing.

[Gudge (MGudgin)]

The loop has no cooperative exit — every way out is external.

Flagging this mostly to note it's not fully addressed by the other two loop findings, in case it looks redundant. Adding a liveness check fixes the dead-child case, and killing the child from the Ctrl-C path fixes the orphan case — but both work by destroying the thing the loop is polling. The loop itself still has exactly one voluntary exit (present == true); nothing can ask it to stop.

Two places that matters:

• Tests. A test that wants to verify "loop is still waiting after N ticks, now cancel it" has no way to end the loop except by tearing down a real process. This compounds with the seam comment above — a closure seam plus a cancellation predicate is what makes the wait cheaply testable, and each is half a fix on its own.
• Any future non-signal shutdown. If the wait ever needs to end for a reason that isn't Ctrl-C and isn't child death — a timeout mode, a supervisor asking wxc-exec to stand down — there's no place to put it.

An Option<&AtomicBool> checked alongside the poll, or a should_cancel: impl FnMut() -> bool if you take the closure-seam route, covers both. Low priority relative to the two hangs — file it under "do this if you're already refactoring the loop for the seam", not as separate work.

[Gudge (MGudgin)]

Various comments below? above?