Skip to content

[fix] windows cortex-engine exec for v0.1.10 Release - #47

Merged
echobt merged 1 commit into
mainfrom
cursor/windows-engine-exec-a384
Sep 8, 2026
Merged

[fix] windows cortex-engine exec for v0.1.10 Release#47
echobt merged 1 commit into
mainfrom
cursor/windows-engine-exec-a384

Conversation

@echobt

@echobt echobt commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the cortex-cli-windows-x64 compile failure that blocked Cortex CLI / Cortex Code Release v0.1.10 (run 34173465433). Linux/macOS already built; Create Release and R2 publish were skipped. Tag v0.1.10 exists; there is no GitHub Release and no assets.

Two cortex-engine errors on Windows nightly:

  1. exec/mod.rs re-exported runner while mod runner was cfg-gated to Linux/macOS. The runner is required on Windows (local_shell, plugins). Unix process-group teardown stays cfg-gated inside the runner; Windows uses job objects via the existing sandbox backend.
  2. exec/policy.rs did *network_access on a bool (E0614). Match ergonomics on Windows nightly bind that Copy field by value. The mapping now uses SandboxPolicy::has_full_network_access() so both rustc 1.98 (CI) and nightly (Release) compile.

No product chrome, version bump, or workflow rewrite.

Test plan

  • cargo fmt --all -- --check
  • ./scripts/clippy.sh
  • cargo test -p cortex-engine --lib — 1279 passed, 6 ignored. Full workspace CI / Test green.
  • TUI / snapshot tests — N/A (no TUI surfaces); CI / TUI checks green
  • cargo audit — clean (CI / Security Audit green)
  • Source/dependency policy and generated API contracts verified against the real PR base (CI / Source and dependency policy green)
  • Local functional/security QA and changed-line coverage — coverage gate is cortex-cli / cortex-app-server / cortex-common only; this diff is cortex-engine. CI / Changed-line coverage green.
  • Windows compile: x86_64-pc-windows-msvc rustc target_os="windows" is set, so mod runner + pub use runner now compile in. Full cargo check -p cortex-engine --target x86_64-pc-windows-msvc cannot finish on this Linux agent (aws-lc-sys needs windows.h). Cfg-reviewed both sites; Release Windows job is the real compile.
  • All 16 PR CI checks green on 7e0fc0c

Attestation (required)

I attest that:

  • Security reviewed — exec/sandbox mapping is unchanged in meaning: WorkspaceWrite.network_access still becomes SandboxPolicyType::Custom.network_access via has_full_network_access(). No new egress, path, or secret handling. No secrets, tokens, or keyring dumps.
  • Product-facing errors — no user-visible copy changed.
  • TUI verified — no TUI surface in this diff.
  • Tests added — unit tests cover WorkspaceWrite true/false and ReadOnly network mapping. No mocks that report success.
  • No secrets — no API keys, WorkOS secrets, R2/AWS credentials, or .env files.

Risk

Release / Windows CLI compile only. Exec policy semantics unchanged. After merge, do not re-run failed jobs on 34173465433 — that run is pinned to 3035361 (pre-fix). Do not push tag v0.1.10 again: a push: tags Release would check out the old tag SHA.

release.yml workflow_dispatch builds from the selected branch (main after merge), then git tag v0.1.10 || true (tag already exists; not moved). Create Release can still publish assets onto the existing tag because no GitHub Release exists yet. Same tag v0.1.10 can be reused; no version bump required.

After this PR is merged, Mathis/Oding re-dispatch Release for tag v0.1.10:

gh workflow run Release --repo CortexLM/cli --ref main -f version=0.1.10

Or: Actions → Release → Run workflow → branch main → version 0.1.10.

Optional: if you want the tag commit to match the fixed binaries, move v0.1.10 to the merge SHA before dispatch. Not required to publish assets.

Open in Web Open in Cursor 

Include the exec runner on Windows (cfg + re-export) and treat
WorkspaceWrite network_access as a bool so nightly MSVC builds.

Co-authored-by: Mathis <echobt@users.noreply.github.com>
@echobt
echobt marked this pull request as ready for review September 8, 2026 01:36
@greptile-apps

greptile-apps Bot commented Sep 8, 2026

Copy link
Copy Markdown

Greptile Summary

Summary

Windows command execution is now enabled, but restricted commands still fail before they start and timed-out full-access commands can leave child processes behind. These failures must be resolved before merging.

Confidence Score: 3/5

Not safe to merge: normal restricted Windows commands cannot run, and timed-out full-access commands can leave descendant processes running.

Two independently confirmed command-execution failures affect newly enabled Windows behavior.

Files Needing Attention: src/cortex-engine/src/exec/mod.rs, src/cortex-engine/src/exec/runner.rs, src/cortex-engine/src/exec/policy.rs, and src/cortex-engine/src/sandbox/windows.rs

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced a proof for a posted P1 finding and linked the review comment for details.
  • T-Rex produced a second P1 finding proof backed by a Python artifact and review comment.
  • T-Rex documented general contract validation showing Windows-specific behavior before and after capture, including PR [fix] windows cortex-engine exec for v0.1.10 Release #47 changes and the Windows backend policy mapping.
  • T-Rex documented another general contract validation for the Windows timeout process-tree harness, including before/after logs and the Python harness file.
  • T-Rex produced another P1 finding proof with no artifacts attached.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (2)

  1. General comment

    P1 Windows ReadOnly and WorkspaceWrite commands remain unusable through the newly enabled runner

    • Bug
      • The newly Windows-enabled exec runner calls policy::prepare, which maps ReadOnly and WorkspaceWrite to SandboxPolicyType::Custom. The legacy SandboxRunner only bypasses preparation for full disk-write access, so on Windows it dispatches Custom policies to WindowsBackend::prepare_command, which returns Required Windows sandbox controls are unavailable in this execution path; command was not started. Consequently these policies reject execution rather than running.
    • Cause
      • The Windows runner enablement did not replace or adapt the legacy SandboxRunner preparation path, whose Windows backend intentionally has no implementation for non-full-access command preparation.
    • Fix
      • Route Windows non-full-access exec through an implemented restricted spawn/sandbox path, or keep the runner unavailable until that path can enforce the requested policy. Do not silently fall back to unrestricted execution.

    T-Rex Ran code and verified through T-Rex

  2. General comment

    P1 Windows full-access command timeouts leave descendants outside cleanup

    • Bug
      • src/cortex-engine/src/exec/mod.rs:6-15 newly compiles and exports the execution runner on Windows. For DangerFullAccess, sandbox preparation returns the original command, and the Windows runner has neither process-group teardown nor Job Object ownership/assignment. When its timeout expires, src/cortex-engine/src/exec/runner.rs:182-184 kills and waits only for the immediate child. A child that launches a detached or ordinary descendant can therefore leave that descendant running after the parent is terminated.
    • Cause
      • The Unix process-group creation and ProcessGroup::terminate() cleanup are conditionally compiled only for Unix/Linux/macOS (runner.rs:125-129, 153-156), and the newly reachable Windows branch relies only on tokio::process::Child::kill(). The full-access policy explicitly bypasses the Windows sandbox path that might otherwise create a Job Object.
    • Fix
      • On Windows, create and retain a kill-on-close Job Object for each runner invocation, assign the spawned direct child to it, and explicitly terminate/close the job on timeout and after completion. Add a Windows-only regression test that launches a parent which starts a marker-writing descendant, times out the parent, and asserts the descendant cannot write its delayed marker.

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "fix(engine): compile exec runner on wind..." | Re-trigger Greptile

@echobt
echobt merged commit c7eab96 into main Sep 8, 2026
17 checks passed
#[cfg(any(target_os = "linux", target_os = "macos"))]
// Process-group teardown is Unix-only; the runner itself is required on Windows
// so local_shell / plugin exec still compile (job-object isolation is separate).
#[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Restricted Commands Cannot Run

This change enables the command runner on Windows, but ReadOnly and WorkspaceWrite commands still enter the legacy sandbox preparation path. That path rejects non-full-access policies on Windows before spawning the command, so local-shell and plugin commands using the normal restricted policies fail instead of running.

Artifacts

Evidence from the check

Command output from the check

  • Executed parent-source comparison showing the exec runner cfg excluded Windows before the change, ending with the prior runner-unavailable condition.

Command output from the check

  • Executed validation and focused Cargo tests showing Windows runner enablement while non-full-access policies reach the rejecting legacy Windows backend, ending with confirmed behavior.

Evidence from the check

  • Python harness authored and executed against the parent and PR sources; it verifies the Windows full-access timeout path lacks tree cleanup.

Command output from the check

  • Executed environment check showing only the Linux Rust target is installed and PowerShell, pwsh, and Wine are unavailable; native Windows reproduction cannot run on this host.

Command output from the check

  • Captured execution of the authored harness showing all path assertions passed and confirming the direct-child-only Windows timeout cleanup.

View artifacts

T-Rex Ran code and verified through T-Rex

#[cfg(any(target_os = "linux", target_os = "macos"))]
// Process-group teardown is Unix-only; the runner itself is required on Windows
// so local_shell / plugin exec still compile (job-object isolation is separate).
#[cfg(any(target_os = "linux", target_os = "macos", target_os = "windows"))]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Timeouts Leave Child Processes

This change enables the runner on Windows, but full-access commands receive neither process-group cleanup nor Job Object ownership. When a timeout occurs, the runner kills only the immediate child; descendants it started can remain running after the command is reported as timed out.

Artifacts

Evidence from the check

Command output from the check

  • Executed parent-source comparison showing the exec runner cfg excluded Windows before the change, ending with the prior runner-unavailable condition.

Command output from the check

  • Executed validation and focused Cargo tests showing Windows runner enablement while non-full-access policies reach the rejecting legacy Windows backend, ending with confirmed behavior.

Evidence from the check

  • Python harness authored and executed against the parent and PR sources; it verifies the Windows full-access timeout path lacks tree cleanup.

Command output from the check

  • Executed environment check showing only the Linux Rust target is installed and PowerShell, pwsh, and Wine are unavailable; native Windows reproduction cannot run on this host.

Command output from the check

  • Captured execution of the authored harness showing all path assertions passed and confirming the direct-child-only Windows timeout cleanup.

View artifacts

T-Rex Ran code and verified through T-Rex

@echobt
echobt deleted the cursor/windows-engine-exec-a384 branch September 8, 2026 01:44
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.

1 participant