Update Cargo.toml with 6 changed files (#4506)

[rysweet]

Summary

Concise workflow-generated PR for Cargo.toml.

Issue

Closes #4506

Changed files

• Cargo.lock
• Cargo.toml
• docs/index.md
• docs/reference/base-type-adapters.md
• docs/reference/rustyclawd-echild-reap-tolerance.md
• src/base_type_rustyclawd/tool_executor.rs

Diff stat

 Cargo.lock                                         |   2 +-
 Cargo.toml                                         |   2 +-
 docs/index.md                                      |   1 +
 docs/reference/base-type-adapters.md               |   9 +
 docs/reference/rustyclawd-echild-reap-tolerance.md | 225 +++++++++++++++++++++
 src/base_type_rustyclawd/tool_executor.rs          | 103 +++++++++-
 6 files changed, 335 insertions(+), 7 deletions(-)

Behavior

Implemented behavior through these branch commits:
38a2f38 feat: ℹ NODE_OPTIONS=--max-old-space-size=32768 (saved preference). To chang
362a57f refactor(rustyclawd): remove redundant ECHILD reap test, drop stale RED comment (#4506)
48ddb88 wip: checkpoint after implementation (steps 7-8)

Validation

ℹ NODE_OPTIONS=--max-old-space-size=32768 (saved preference). To change: /home/azureuser/.amplihack/config
I'll start by understanding the current state of the branch and what changed.

● Get branch name and diff stat (shell)
│ cd /home/azureuser/src/Simard-deploy-4049/worktrees/feat/issue-4506-nodeoptions-max-old-space-si
│ ze32768-saved-preferen && git branch --show-current && echo "---MAIN DIFF---" && git --no-pager
│ diff origin/main...HEAD --stat 2>/dev/null | head -50
└ 8 lines…

● View commits and code diff (shell)
│ cd /home/azureuser/src/Simard-deploy-4049/worktrees/feat/issue-4506-nodeoptions-max-old-space-si
│ ze32768-saved-preferen && git --no-pager log --oneline origin/main..HEAD | head -20 && echo

Risk

No high-risk subsystem pattern detected from changed paths.

Checklist

• Branch has 3 commit(s) ahead of main
• Code review completed
• Philosophy check passed

---

This PR was created as a draft for review before merging.

---

Step 16b: Outside-In Testing Results

Tested from the PR branch as the deploy-gate unit-test consumer would, exercising the ECHILD reap-tolerance fix (#4506) through the same cargo test boundary the self-deploy gate shells out to (src/self_relaunch/gates.rs::run_unit_test_gate).

Detected toolchain: Rust CLI crate (Cargo.toml at root, simard v0.37.0) — cargo 1.95.0 / rustc 1.95.0. No Node/Python/Go manifests relevant to the change. Changed files: src/base_type_rustyclawd/tool_executor.rs, Cargo.toml, Cargo.lock, docs/.

Chosen strategy: Native cargo test (per QA-team Rust CLI repo-type detection), matching the deploy-gate unit-test command (cargo test --manifest-path Cargo.toml). One simple scenario proving the fix, one edge/integration scenario for regression + real bash-child behavior.

#	Scenario	Command	Result	Key output
1 (simple)	Targeted ECHILD reap mapping — proves the exact path that red-canaried the gate	cargo test --lib status_from_reap_error -- --nocapture	✅ PASS	2 passed; 0 failed — status_from_reap_error_synthesizes_success_on_echild, status_from_reap_error_preserves_other_errors
2 (edge/integration)	Full tool_executor module: real bash children (echo/stderr/non-zero exit), idle-liveness reaping (#2607), empty-command, plus the ECHILD guard	cargo test --lib base_type_rustyclawd::tool_executor	✅ PASS	26 passed; 0 failed — incl. execute_tool_locally_bash_failing_command_has_nonzero_exit, bash_idle_child_is_reaped_with_honest_error_and_no_orphan_2607, both new ECHILD tests

Result: Both scenarios green on the first run. The ECHILD race is now tolerated as a logged exit_code: 0 success (proving the deploy-gate no longer red-canaries on fast/empty commands), while every non-ECHILD errno (e.g. EPERM) still surfaces as ClientError::Unknown — genuine failures are never reclassified. No regressions in the 24 pre-existing tool_executor tests.

Fix count during outside-in testing: 0 (no failures encountered; no additional commits required).

[github-actions]

📊 Coverage Summary

Generated by cargo llvm-cov --workspace --summary-only (nightly, excluding test files)

Module	Lines	Covered	Coverage
Total	196173	164908	84.1%

_{Coverage data from CI run. Test files matching tests?/ are excluded from line counts.}

[rysweet]

Step 17b — Comprehensive Code Review

Verdict: APPROVE. The ECHILD reap-tolerance fix (#4506) is correct, minimal, well-tested, and well-documented. No blocking issues. Independently verified below.

Independent verification

• cargo test --lib base_type_rustyclawd::tool_executor → 26 passed / 0 failed (incl. previously-failing execute_tool_locally_bash_missing_command_runs_empty_string and both new status_from_reap_error_* tests).
• cargo clippy --lib → clean (0 warnings/errors).

Review checklist

• Code quality & standards — status_from_reap_error is a small, pure errno→status mapper; libc::ECHILD fully-qualified (matches existing libc::kill); function-scoped ExitStatusExt import; Unix-only, consistent with the module's existing setsid/libc::kill assumptions.
• Test coverage adequate — two deterministic, env-independent guard tests: ECHILD→Some(0) and non-ECHILD(EPERM)→ClientError::Unknown. The EPERM test pins the exact-match so the tolerance can't be over-broadened. Correctly avoids relying on flaky real SIGCHLD reaping.
• No TODOs, stubs, or swallowed exceptions — none. ECHILD is logged via tracing::warn! (structured reap_point + error fields only), not silenced. Zero-BS / no-silent-degradation upheld. No print!/println!.
• No unimplemented functions — none.
• Logic correctness — both reap arms route through the helper. ? propagates non-ECHILD errors unchanged (byte-for-byte prior behavior). ExitStatus::from_raw(0) decodes as WIFEXITED/code 0, so the unchanged status.code().unwrap_or(-1) emits exit_code: 0 (not -1); the test pins this invariant. Reap point A sets exit_status + break; since it fires only under !streams_open, out_buf/err_buf are already complete, so no output is lost.
• Edge case handling — real exit codes (incl. non-zero), idle-liveness reaping (ClientError::Timeout, #2607), and the success-JSON shape are all untouched. Additive/non-breaking; execution.rs callers need no change.

Non-blocking observations (no action required)

1. Reap point B (None => child.wait(), terminal arm) is effectively unreachable in the current loop. Every non-hung exit from the loop goes through the try_wait match, which always sets exit_status = Some(...) before break (including the new Err path). So the None branch — and thus its new Err(e) => status_from_reap_error(e, "wait")? — cannot be hit today. This is pre-existing control-flow, not introduced by this PR, and the defensive handling is harmless (and correct if the loop structure ever changes). Flagging for awareness only.
2. Accepted residual risk R1: on the ECHILD race, a command that actually exited non-zero would be reported as exit_code: 0. Since the status was externally reaped and is unrecoverable, 0 is the only defensible synthesized value, and the alternative (red-canarying every fast/empty command) is strictly worse. Documented and acceptable.

Scope / hygiene

• Change is correctly scoped to tool_executor.rs + supporting docs; version bump 0.36.0 → 0.37.0 in Cargo.toml/Cargo.lock is consistent.
• Docs (rustyclawd-echild-reap-tolerance.md) are thorough and accurately describe the implemented behavior, including the "logging not silencing" rationale and the from_raw(0) invariant.

No changes requested. Ready to merge once required CI checks (incl. the unit-test deploy-gate) are green.

[rysweet]

Step 17c — Security Review

Verdict: APPROVE — no security-blocking findings. The ECHILD reap-tolerance fix (#4506) introduces no new attack surface. Independently verified against the committed diff.

Scope reviewed

src/base_type_rustyclawd/tool_executor.rs (the only executable change), Cargo.toml/Cargo.lock (version bump), and 3 docs files. Additions confined to a private errno-mapping helper + two reap-arm rewires + deterministic guard tests.

Findings

1. Injection vulnerabilities — NONE.
No new command construction, argument interpolation, or shell string building. The sh -c "" path is pre-existing. The change touches only post-execution errno classification (try_wait/wait Err arms); no attacker-controlled data reaches a new sink.

2. Sensitive-data handling — PASS.
The tracing::warn! on ECHILD logs only a static reap_point label ("try_wait"/"wait") and the kernel io::Error display ("No child processes (os error 10)"). It deliberately excludes out_buf/err_buf (tool stdout/stderr), the command string, and the environment. No secrets, tokens, PII, or tool output can leak into logs/OTel via this path. Structured tracing only — no print!/println!.

3. Authentication / authorization — N/A.
Process-reaping logic; no auth, session, credential, or permission surface is touched.

4. Failure-masking / integrity (residual risk R1) — LOW, accepted.
Synthesizing exit_code: 0 on ECHILD could in principle mask a genuine non-zero exit if the child is externally reaped mid-race. Assessed low severity because:

• Not externally triggerable. The external reap requires an in-process waiter (tokio's signal-driven child reaper or a process-wide waitpid(-1)) already inside the trust boundary. A remote/unprivileged attacker cannot induce ECHILD; there is no cross-privilege escalation.
• Status is genuinely unrecoverable. Once the kernel has collected the child, no exit status remains to read — 0 is the only defensible synthesized value, and every synthesis is logged (no silent degradation).
• Match is exact. Some(libc::ECHILD) only — never a range or substring — so the tolerance cannot be over-broadened to swallow EPERM/other real failures. Pinned by test status_from_reap_error_preserves_other_errors.

5. Supply chain — PASS.
Cargo.toml change is a version bump (0.36.0 → 0.37.0) only; Cargo.lock adds no new dependencies. libc is a pre-existing dependency, referenced fully-qualified (libc::ECHILD) consistent with existing libc::kill usage. No new crates, no new transitive surface.

6. Denial-of-service / resource handling — PASS.
No new loops, allocations, or unbounded waits introduced. The try_wait arm now breaks (bounded) instead of returning; idle-liveness timeout behavior is byte-for-byte unchanged, so a genuinely hung child is still killed and surfaced as ClientError::Timeout.

Verification

• Errno match is exact and non-broadenable (confirmed in source, L89).
• Log fields confirmed to exclude output buffers/command/env (source L92–96).
• exit_code invariant: ExitStatus::from_raw(0) → .code() == Some(0), so no -1 leakage; pinned by status_from_reap_error_synthesizes_success_on_echild.

No security requirements are violated. No new vulnerabilities. Recommend proceeding.

[rysweet]

Step 17d — Philosophy Guardian Review

Verdict: PASS. The ECHILD reap-tolerance fix (#4506) is philosophy-compliant. No blocking findings; no changes required.

Compliance checklist

• Ruthless simplicity — The entire fix is one ~18-line pure helper (status_from_reap_error) plus two one-line call sites at the existing try_wait/wait reap points. No new types, traits, config, or abstraction layers. The simplest thing that correctly handles the race.
• Bricks & studs — Single, well-defined seam: io::Error → Result<ExitStatus, ClientError>. The helper is a self-contained brick with one responsibility (classify a reap error); the two call sites are its only studs. Clean, narrow contract.
• Zero-BS implementation — No stubs, no faked APIs, no swallowed exceptions. ECHILD is tolerated deliberately, not silenced: every synthesis emits tracing::warn! with reap_point + error, leaving an audit trail. Any non-ECHILD errno still surfaces as ClientError::Unknown, so a genuine failure is never reclassified as success.
• No over-engineering — reap_point is a plain &'static str label used only for the trace event; no enum ceremony. No speculative generality (no handling of hypothetical future errnos, no configurable policy). Scope is exactly the observed race and nothing more.
• Clean module boundaries — Change is confined to tool_executor.rs (the process-execution brick that owns child reaping). ExitStatusExt is imported function-locally, keeping the unix-specific dependency at its point of use. No leakage into callers or unrelated modules.

Notes

• Doc weight is justified, not bloat. The additions include a dedicated reference doc (docs/reference/rustyclawd-echild-reap-tolerance.md) and inline why comments (L69–84). This is documenting a subtle concurrency invariant (ECHILD ⇒ status unrecoverable ⇒ exit_code: 0 is the only defensible value) — exactly the kind of non-obvious decision that should be written down. Not over-documentation.
• Residual risk R1 (ECHILD could mask a real non-zero exit under the race) is explicitly documented and accepted: once the child is externally reaped its status is gone, so 0 is the only recoverable value. Correct trade-off, honestly disclosed — consistent with zero-BS.

Confirmation: Philosophy compliance verified against committed state (362a57f2). Posting as required evidence. No re-implementation needed.