refactor(process): adopt Execa execution layer by steipete · Pull Request #105939 · openclaw/openclaw

steipete · 2026-07-13T03:31:25Z

What Problem This Solves

OpenClaw's main process-execution wrapper manually coordinated spawn, timeout, abort, output, and close races, while several production call sites bypassed that behavior with raw node:child_process calls.

Why This Change Was Made

This adopts Execa 9.6.1 behind the canonical process wrapper, removes the replaced timer/signal race machinery, and moves descendant-pipe draining into src/process/child-process.ts. OpenClaw still owns its security and platform contracts: argv-only execution, .cmd/.bat handling for CVE-2024-27980, Windows console-codepage decoding, environment normalization, process-tree termination, capped output capture, timeout, and abort behavior.

The first migration batch routes agent session exec plus bash/find/grep helpers through the wrapper. The command queue remains unchanged.

User Impact

No intended user-visible behavior change. Process execution now has one canonical lifecycle path, with consistent timeout, abort, environment, output-limit, and descendant-pipe handling across the migrated surfaces.

Evidence

Current branch head: 8c6fbd47e33; current base merged at bafabba0dfc5.
Exact pre-main-merge proof head: 44eaedaf7e4.
AWS Crabbox run run_3fc3e74fa1c4: process tests 161 passed / 1 skipped; TUI tests 27 passed; no-output tests 4 passed; formatting, oxlint, pnpm tsgo:core, pnpm check:test-types, pnpm plugin-sdk:surface:check, and the then-current LOC gate passed.
Focused Windows/PATHEXT AWS Crabbox run run_3d13789c8c78: infra tests 18 passed / 1 skipped; process/Windows tests 43 passed / 1 skipped; formatting, lint, and core types passed.
Fresh autoreview chain clean after all accepted findings, including the final PATHEXT contract fix.
Hosted check-dependencies job passed after making the internal-only invocation type module-local.
Main-merge conflict proof: package and shrinkwrap JSON parsed; pnpm lock YAML parsed; no conflict markers; git diff --cached --check and post-merge git diff HEAD^2..HEAD --check passed. Resolution preserves both Execa's dependency graph and current main's dependency graph; main's removed LOC baseline stays removed.
Hosted CI wait relaxed by maintainer instruction; queued checks are not being treated as a landing blocker.

No changelog entry: internal execution-layer refactor with no intended user-visible behavior change.

github-actions · 2026-07-13T03:31:49Z

Dependency Guard

This PR changes dependency-related files. Maintainers should confirm these changes are intentional.

Changed files:

npm-shrinkwrap.json
package.json
pnpm-lock.yaml

Maintainer follow-up:

Review whether the dependency changes are intentional.
Inspect resolved package deltas when lockfile, shrinkwrap, or workspace dependency policy changes are present.
Treat package-lock.json and npm-shrinkwrap.json diffs as security-review surfaces.
Run pnpm deps:changes:report -- --base-ref origin/main --markdown /tmp/dependency-changes.md --json /tmp/dependency-changes.json locally for detailed release-style evidence.

github-actions · 2026-07-13T03:31:49Z

Dependency graph changes noted

This PR includes dependency graph changes. The dependency guard is informational because the PR author is a repository admin or a member of @openclaw/openclaw-secops.

Current SHA: 8c6fbd47e33e86992b0fac97c0148768a82506e9
Trusted actor: @steipete
Trusted role: pull request author; openclaw-secops

Security review is still recommended before merge when the dependency graph change is intentional.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 44abfe608f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-07-13T03:38:08Z

+  signal?.addEventListener("abort", onAbort, { once: true });
+  armNoOutputTimer();
+
+  child.stdout?.on("data", (chunk) => {


Restore stdout/stderr error handlers

When runCommandWithTimeout is used with a child whose stdout or stderr stream emits an error (for example an EIO/EPIPE from a broken pipe or inherited fd), these streams now only have data listeners, so Node treats the unhandled error event as fatal and can crash the process. The pre-refactor implementation installed no-op error handlers before reading output; keep that guard here as well so process execution failures are reported through the command result instead of an uncaught stream error.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 005c918fc4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-07-13T04:20:33Z

+  child.stdout?.on("data", onData);
+  child.stderr?.on("data", onData);


Register the drain watcher after real output readers

When this helper is installed before the caller's capture/readline listeners, these data listeners put stdout/stderr into flowing mode and can consume any chunk emitted in that gap; onData ignores pre-exit chunks, so fast commands can lose the beginning (or all) of their output. The new callers do exactly that (for example src/process/exec.ts:514 before the capture listeners at src/process/exec.ts:613, and similarly bash/find/grep), whereas the old wait helper was attached after the real readers.

Useful? React with 👍 / 👎.

clawsweeper · 2026-07-13T04:23:49Z

Codex review: needs real behavior proof before merge. Reviewed July 13, 2026, 1:44 AM ET / 05:44 UTC.

Summary
The PR adds Execa 9.6.1 as the core execution dependency, rebuilds the canonical process wrapper around it, centralizes inherited-pipe cleanup, and migrates selected agent exec, bash, find, and grep paths.

PR surface: Source -57, Tests -603, Config +1, Other +313. Total -346 across 23 files.

Reproducibility: yes. at source level: a focused test can return a launched child whose stdout or stderr emits error; the current bash path has no listener that safely absorbs or translates that event. No live failing run was performed during this read-only review.

Review metrics: 1 noteworthy metric.

Root runtime dependencies: 1 added: Execa 9.6.1. The dependency owns core process execution and is explicitly classified as a large transitive cone, so its lifecycle and supply-chain contract warrants focused review.

Stored data model
Persistent data-model change detected: serialized state: src/agents/sessions/exec.test.ts, serialized state: src/agents/sessions/exec.ts, serialized state: src/agents/sessions/tools/bash.test.ts, serialized state: src/agents/sessions/tools/bash.ts, serialized state: src/agents/sessions/tools/find.fd.test.ts, serialized state: src/agents/sessions/tools/find.ts, and 6 more. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #105920
Summary: This PR is the active implementation candidate for the linked request to standardize core process execution on Execa.

Members:

canonical: [Feature]: Standardize child-process execution on Execa #105920 - The issue describes the same Execa migration, process-contract preservation, and bounded initial caller migration, and the PR uses closing syntax for it.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🌊 off-meta tidepool
Patch quality: 🦐 gold shrimp
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

[P2] Restore shared stdout/stderr error guards and add a regression test that emits stream errors after process launch.
Rerun the focused process and agent-session test set on the repaired exact head.

Risk before merge

[P1] Merging the current head can allow an stdout/stderr EIO or EPIPE during local bash execution to become an uncaught EventEmitter error and terminate the agent process instead of settling through the command result path.

Maintainer options:

Restore shared stream guards (recommended)
Add non-fatal stdout/stderr error handling to the shared lifecycle helper and regression coverage before merging so migrated process callers cannot crash on stream errors.

Next step before merge

[P1] This protected maintainer-labeled PR should remain in normal review; the required repair is clearly stated in the finding, but ClawSweeper should not dispatch a separate fix lane.

Security
Cleared: The Execa dependency addition is intentional, pinned, integrity-locked, assigned explicit core ownership, and accompanied by successful dependency and security guard checks; no concrete supply-chain or execution-boundary regression beyond the functional availability finding was identified.

Review findings

[P2] Restore stream error guards in the shared lifecycle helper — src/process/child-process.ts:52-53

Review details

Best possible solution:

Restore the old stream-error safety contract in the shared child-process lifecycle layer, retain caller-specific error reporting where needed, and add a focused regression test that emits stdout and stderr errors after launch before merging the Execa migration.

Do we have a high-confidence way to reproduce the issue?

Yes at source level: a focused test can return a launched child whose stdout or stderr emits error; the current bash path has no listener that safely absorbs or translates that event. No live failing run was performed during this read-only review.

Is this the best way to solve the issue?

No, not at the current head. Adopting one canonical Execa-backed wrapper is a maintainable direction, but the shared replacement should preserve the previous stream-error contract rather than leaving each migrated caller vulnerable or requiring scattered guards.

Full review comments:

[P2] Restore stream error guards in the shared lifecycle helper — src/process/child-process.ts:52-53
The replacement helper listens for stdout/stderr data but no longer installs the error guards provided by the removed waitForChildProcess implementation. The migrated bash caller also has only data listeners, so an EIO/EPIPE event is unhandled and can terminate the agent process; preserve the old non-fatal guard in the shared helper and add a focused stream-error regression test.
Confidence: 0.95

Overall correctness: patch is incorrect
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 6a0eecdb4451.

Label changes

Label changes:

add merge-risk: 🚨 availability: The migrated bash path can expose an unhandled child-stream error that terminates the hosting process after merge.
add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The proof gate is not applied because repository automation identifies the author as a trusted repository administrator/openclaw-secops participant; the supplied Testbox runs, Windows checks, build, type, lint, dependency, and focused process tests remain useful supplemental evidence.

Label justifications:

P2: The remaining defect can crash an agent process under a bounded stream-error condition, but no evidence shows a widespread current-user emergency.
merge-risk: 🚨 availability: The migrated bash path can expose an unhandled child-stream error that terminates the hosting process after merge.
rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The proof gate is not applied because repository automation identifies the author as a trusted repository administrator/openclaw-secops participant; the supplied Testbox runs, Windows checks, build, type, lint, dependency, and focused process tests remain useful supplemental evidence.

Evidence reviewed

PR surface:

Source -57, Tests -603, Config +1, Other +313. Total -346 across 23 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	8	452	509	-57
Tests	9	488	1091	-603
Docs	0	0	0	0
Config	1	1	0	+1
Generated	0	0	0	0
Other	5	356	43	+313
Total	23	1297	1643	-346

What I checked:

Remaining stream-error regression: The new shared cleanup helper registers stdout/stderr data listeners but no error listeners, while the removed waitForChildProcess helper guarded both streams; bash adds only data listeners, so an EIO/EPIPE stream error can remain unhandled. (src/process/child-process.ts:52, c6a3dc8d7726)
Affected caller: createLocalBashOperations installs data readers and the shared release helper but does not install stdout/stderr error handlers or otherwise translate stream errors into the returned command result. (src/agents/sessions/tools/bash.ts:70, c6a3dc8d7726)
Prior contract on current main: The replaced agent child-process waiter explicitly attached stdout and stderr error listeners, demonstrating that suppressing otherwise-fatal stream errors was part of the existing process lifecycle behavior. (src/agents/utils/child-process.ts:53, 6a0eecdb4451)
Regression coverage gap: The new bash test covers a shell launch failure but does not emit an error from an already-launched child's stdout or stderr, so it does not prove preservation of the removed stream-error contract. (src/agents/sessions/tools/bash.test.ts:34, c6a3dc8d7726)
Review continuity: The current PR discussion already records the same remaining bash stream-error concern after the earlier output-consumer ordering finding; the latest supplied head does not show the missing guard restored. (src/agents/sessions/tools/bash.ts:70, c6a3dc8d7726)
Dependency intent and ownership: The new root dependency is pinned to Execa 9.6.1 and assigned to core process execution with process-execution and large-transitive-cone risk metadata; dependency, shrinkwrap, and security guard checks are reported successful. (scripts/lib/dependency-ownership.json:111, c6a3dc8d7726)

Likely related people:

steipete: The repository dependency guard identifies this person as a trusted repository administrator and openclaw-secops participant, and the PR history shows sustained work preserving Windows, launch-error, shim-completion, and process-lifecycle contracts. (role: process-execution and security reviewer; confidence: high; commits: 22564a40652a, fd624a5c0744, fbfa3ff3a0d6; files: src/process/exec.ts, src/process/child-process.ts, src/agents/sessions/tools/bash.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Review history (2 earlier review cycles)

reviewed 2026-07-13T04:22:03.969Z sha 005c918 :: needs real behavior proof before merge. :: [P2] Restore stdout and stderr error guards
reviewed 2026-07-13T05:16:21.351Z sha 1681295 :: needs real behavior proof before merge. :: [P1] Attach the drain watcher after output consumers

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0346352907

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-07-13T05:28:38Z

-        // on inherited stdio handles held by detached descendants.
-        waitForChildProcess(child)
-          .then((code) => {
+        void child


Restore bash stream error guards

When the local bash child’s stdout or stderr emits an error event (for example EIO/EPIPE from a broken pipe or inherited fd), this Execa promise replacement no longer installs the no-op stream error handlers that the removed waitForChildProcess helper provided, while the code still only adds data listeners above. Node treats an unhandled stream error as fatal, so a bash tool execution can crash the process instead of settling through the existing command result path.

Useful? React with 👍 / 👎.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4904d68815

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-07-13T09:16:28Z

 // Public process helpers for plugins that spawn or probe local commands.

-export * from "../process/exec.js";
+export {


Preserve existing process-runtime exports

When a third-party plugin imports a helper that was previously exposed by the wildcard export from openclaw/plugin-sdk/process-runtime (for example spawnCommand or SpawnCommandOptions from src/process/exec.ts), this explicit allowlist removes that export from the public SDK subpath declared in package.json, so those plugins fail at import/typecheck time after upgrading. Keep the old exports as deprecated compatibility or add a documented major-version removal path instead of silently narrowing this public SDK surface.

Useful? React with 👍 / 👎.

steipete · 2026-07-13T09:20:32Z

Land-ready at 8c6fbd47e33e86992b0fac97c0148768a82506e9.

The first hosted dependency run exposed one related issue: SafeChildProcessInvocation was exported but only used inside src/process/windows-command.ts. Commit 8c6fbd47e33 makes that alias module-local; runtime behavior is unchanged. Fresh focused autoreview (.agents/skills/autoreview/scripts/autoreview --mode uncommitted) is clean with 0.96 correctness confidence, and the targeted hosted check-dependencies job now passes.

Core proof remains:

AWS Crabbox run_3fc3e74fa1c4: process 161 passed / 1 skipped, TUI 27 passed, no-output 4 passed, plus formatting, oxlint, core/test types, Plugin SDK surface, and LOC gate.
AWS Crabbox run_3d13789c8c78: infra 18 passed / 1 skipped, process/Windows 43 passed / 1 skipped, plus formatting, lint, and core types.
Hosted Windows test on the merged-main head passed. Package/shrinkwrap JSON, pnpm lock YAML, conflict-marker scan, and diff checks passed after resolving main.
Whole-branch autoreview and focused follow-up reviews are clean.

Known proof gap: no native Windows live run. Windows-specific command, codepage, PATHEXT, and taskkill behavior has focused mocked-platform coverage plus the hosted Windows test. Per maintainer instruction, the remaining hosted CI wait is relaxed; related dependency validation is green.

* refactor(process): adopt execa execution layer * fix(process): preserve launch error semantics * chore(process): ratchet exec size baseline * fix(process): preserve Windows and error contracts * chore(plugin-sdk): ratchet wildcard surface budget * test(process): allow resolved Windows executable paths * fix(process): preserve Windows shim completion * test(process): isolate Execa mocks * style(process): format Windows exec test * style(process): apply Windows test formatting * test(process): normalize Windows system path casing * fix(process): harden execa edge handling * test(tui): preserve ESM fixture semantics * fix(process): preserve PATHEXT lookup contract * fix(process): keep invocation type internal

steipete requested a review from a team as a code owner July 13, 2026 03:31

openclaw-barnacle Bot added scripts Repository scripts agents Agent runtime and tooling size: XL maintainer Maintainer-authored PR labels Jul 13, 2026

github-actions Bot added the dependencies-changed PR changes dependency-related files label Jul 13, 2026

chatgpt-codex-connector Bot reviewed Jul 13, 2026

View reviewed changes

clawsweeper Bot mentioned this pull request Jul 13, 2026

[Feature]: Standardize child-process execution on Execa #105920

Closed

steipete force-pushed the codex/adopt-execa-process-layer branch from 44abfe6 to 005c918 Compare July 13, 2026 04:16

chatgpt-codex-connector Bot reviewed Jul 13, 2026

View reviewed changes

steipete force-pushed the codex/adopt-execa-process-layer branch 2 times, most recently from 1379e05 to 1681295 Compare July 13, 2026 04:51

steipete added 8 commits July 13, 2026 06:10

refactor(process): adopt execa execution layer

22564a4

fix(process): preserve launch error semantics

fd624a5

chore(process): ratchet exec size baseline

9eedadf

fix(process): preserve Windows and error contracts

fbfa3ff

chore(plugin-sdk): ratchet wildcard surface budget

153ceb9

test(process): allow resolved Windows executable paths

ed6d57d

fix(process): preserve Windows shim completion

7864a8c

test(process): isolate Execa mocks

7e18480

steipete force-pushed the codex/adopt-execa-process-layer branch from 1681295 to 7e18480 Compare July 13, 2026 05:17

style(process): format Windows exec test

0346352

chatgpt-codex-connector Bot reviewed Jul 13, 2026

View reviewed changes

steipete added 2 commits July 13, 2026 06:30

style(process): apply Windows test formatting

0fbd764

test(process): normalize Windows system path casing

c6a3dc8

steipete added 4 commits July 13, 2026 09:37

fix(process): harden execa edge handling

5baee11

test(tui): preserve ESM fixture semantics

a2c2cf9

fix(process): preserve PATHEXT lookup contract

44eaeda

chore: merge main into Execa migration

4904d68

chatgpt-codex-connector Bot reviewed Jul 13, 2026

View reviewed changes

fix(process): keep invocation type internal

8c6fbd4

steipete merged commit 4f287dd into main Jul 13, 2026
109 of 110 checks passed

steipete deleted the codex/adopt-execa-process-layer branch July 13, 2026 09:21

vincentkoc mentioned this pull request Jul 13, 2026

fix(ci): refresh plugin SDK API baseline #106253

Merged

steipete mentioned this pull request Jul 13, 2026

refactor(process): route bounded commands through Execa #106495

Merged

		child.stdout?.on("data", onData);
		child.stderr?.on("data", onData);

Uh oh!

Uh oh!

Conversation

steipete commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

github-actions Bot commented Jul 13, 2026

Dependency Guard

Uh oh!

github-actions Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Dependency graph changes noted

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

clawsweeper Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

steipete commented Jul 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

steipete commented Jul 13, 2026 •

edited

Loading

github-actions Bot commented Jul 13, 2026 •

edited

Loading

clawsweeper Bot commented Jul 13, 2026 •

edited

Loading