Skip to content

refactor(process): add bounded blocking execution - #2671

Draft
lamalex wants to merge 1 commit into
moonrepo:masterfrom
lamalex:vcs/process-refactor
Draft

refactor(process): add bounded blocking execution#2671
lamalex wants to merge 1 commit into
moonrepo:masterfrom
lamalex:vcs/process-refactor

Conversation

@lamalex

@lamalex lamalex commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Adds bounded blocking process execution to moon_process.

VCS plugins invoke executables through synchronous extism host functions. Centralizing this behavior in moon_process avoids embedding generic subprocess infrastructure in the plugin host, and enables sharing some of moon's existing sub process code.

Copilot AI lite review requested due to automatic review settings August 17, 2026 01:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a blocking (synchronous) process execution path to moon_process intended for environments that can’t use async execution (like synchronous Extism host calls), while still supporting bounded execution via timeouts and output limits.

Changes:

  • Introduces a new blocking capture executor (CaptureOptions, blocking capture methods, and process-group/job-object cleanup) in crates/process/src/blocking.rs.
  • Adds new ProcessError variants for output limits and timeouts to surface bounded-execution failures.
  • Expands test coverage for blocking capture behavior (timeouts, output limits, byte-preserving output, and descendant cleanup).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
crates/process/tests/exec_command_test.rs Adds unit tests covering blocking capture behavior, bounds enforcement, and descendant cleanup.
crates/process/src/shared_child.rs Exposes convert_exit_status internally for reuse by the blocking executor.
crates/process/src/process_error.rs Adds OutputLimitExceeded and Timeout error variants (with diagnostics).
crates/process/src/lib.rs Wires in the new blocking module and re-exports its public API.
crates/process/src/exec_command.rs Exposes internal helpers (create_sync_command, handle_nonzero_status) for blocking executor reuse.
crates/process/src/blocking.rs New blocking capture implementation with timeout/output-limit enforcement and process-tree cleanup.
crates/process/Cargo.toml Updates Windows windows-sys features to support job objects used for cleanup on Windows.

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

Comment thread crates/process/Cargo.toml
Comment thread crates/process/src/blocking.rs
Comment thread crates/process/tests/exec_command_test.rs
@codspeed-hq

codspeed-hq Bot commented Aug 17, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 43.81%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚡ 1 improved benchmark
✅ 22 untouched benchmarks

Performance Changes

Mode Benchmark BASE HEAD Efficiency
WallTime hash_files[1000] 17.5 ms 12.1 ms +43.81%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing lamalex:vcs/process-refactor (556463a) with master (c3de6c9)

Open in CodSpeed

@lamalex
lamalex force-pushed the vcs/process-refactor branch from 0d236d0 to 556463a Compare August 17, 2026 01:30
@lamalex
lamalex marked this pull request as draft August 17, 2026 02:44
@lamalex

lamalex commented Aug 17, 2026

Copy link
Copy Markdown
Contributor Author

Setting to draft while command changes are being extracted to starbase

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants