Update documentation with 6 changed files (#4731)

[rysweet]

Summary

Concise workflow-generated PR for documentation.

Issue

Closes #4731

Changed files

• docs/memory.md
• docs/reference/memory-ipc-write-resilience.md
• mkdocs.yml
• src/memory_ipc/client.rs
• src/memory_ipc/mod.rs
• src/memory_ipc/tests_epipe_resilience_4731.rs

Diff stat

 docs/memory.md                                |   1 +
 docs/reference/memory-ipc-write-resilience.md | 232 +++++++++++++
 mkdocs.yml                                    |   1 +
 src/memory_ipc/client.rs                      | 157 ++++++++-
 src/memory_ipc/mod.rs                         |  56 +++-
 src/memory_ipc/tests_epipe_resilience_4731.rs | 460 ++++++++++++++++++++++++++
 6 files changed, 883 insertions(+), 24 deletions(-)

Behavior

Implemented behavior through these branch commits:
4702de9 refactor(memory-ipc): hoist duplicate broken-pipe check in call() retry loop
2817268 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 checking the current state of the branch and changes.

● Get branch, status, and diff stat (shell)
│ cd /home/azureuser/src/Simard-deploy-4049/worktrees/feat/issue-4731-fix-a-recurring-systemic-pro
│ cess-health-defect-in && git branch --show-current && echo "---STATUS---" && git --no-pager
│ status && echo "---DIFF STAT---" && git --no-pager diff --stat HEAD
└ 9 lines…

● Show recent commits and changed files (shell)
│ cd /home/azureuser/src/Simard-deploy-4049/worktrees/feat/issue-4731-fix-a-recurring-systemic-pro
│ cess-health-defect-in && git --no-pager log --oneline -5 && echo "---CHANGED FILES vs origin

Risk

No high-risk subsystem pattern detected from changed paths.

Checklist

• Branch has 2 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

Detected toolchain: Rust CLI crate (Cargo.toml + Cargo.lock at root); cargo 1.95.0 / rustc 1.95.0. Package manager: cargo. No JS/Python/Go manifests. Outside-in boundary = native cargo test (per qa-team Rust-CLI mapping).

Changed surface under test: src/memory_ipc/client.rs, src/memory_ipc/mod.rs, and the focused regression suite src/memory_ipc/tests_epipe_resilience_4731.rs.

Chosen strategy: Run the crate's own outside-in/consumer-boundary tests via cargo test --lib. An isolated CARGO_TARGET_DIR=/tmp/simard-4731-target was used to avoid target-dir contention with concurrent worktree builds on the shared host. Ran one simple scenario (full memory_ipc module) and one edge/integration scenario (mid-write peer-reset resilience).

#	Scenario	Command	Result	Key output
1 (edge/integration)	Large memory-ipc payload survives a mid-write peer reset without data loss; persistent reset surfaces RpcTransportError (never silent Ok); non-Pong reconnect aborts without resending; surfaced error never leaks payload bytes	cargo test --lib memory_ipc::tests_epipe_resilience_4731 -- --nocapture	✅ PASS	test result: ok. 4 passed; 0 failed (mid_write_reset_reconnects_and_delivers_payload, persistent_reset_surfaces_rpc_transport_error_never_silent, non_pong_reconnect_aborts_without_resending_payload, surfaced_error_never_leaks_payload_bytes)
2 (simple/regression)	Full memory-ipc write-path module — no regressions in launcher, shared-store, fail-closed, socket-path behavior	cargo test --lib memory_ipc	✅ PASS	test result: ok. 73 passed; 0 failed

No-silent-drop verification: persistent_reset_surfaces_rpc_transport_error_never_silent confirms an exhausted retry surfaces SimardError::RpcTransportError rather than a silent Ok; mid_write_reset_reconnects_and_delivers_payload confirms a large frame is durably delivered after reconnect. New code uses structured tracing (warn/debug/error/info) only — grep confirms no print!/println! added.

Fix count: 0 (both scenarios passed on first run; no diagnose/fix/push iterations required).

[rysweet]

Step 17b — Comprehensive Code Review (issue #4731 memory-IPC write-path resilience)

Verdict: APPROVE. Independently re-verified all gates green; no blocking findings.

Verification (re-run in this review, not just claimed)

Gate	Result
cargo fmt --check	✅ exit 0
cargo clippy --all-targets	✅ 0 warnings, exit 0
cargo test --lib memory_ipc	✅ 73/73 pass (incl. all 4 tests_epipe_resilience_4731)

Checklist

• Code quality/standards — clean separation via write_frame_raw (raw io::Error) + thin write_frame wrapper; server/other callers unchanged.
• Test coverage adequate — 4 behavioral tests: durable delivery on reconnect, exhaustion→RpcTransportError, non-Pong reconnect abort (no resend), payload-confidentiality. Hermetic scripted UnixListener mock, no global state.
• No TODOs/stubs/swallowed exceptions — none in changed files; all error paths fail-closed.
• No unimplemented functions — none.
• Logic correctness — see below.
• Edge-case handling — non-Pong handshake, reconnect failure, attempt exhaustion, oversized frame all surface errors; no silent Ok.

Logic correctness — key points validated

1. Idempotency reasoning is sound. Retry is confined to the write half: a write-half EPIPE means the server never committed the request, so reconnect+resend cannot duplicate a mutation. Read-half errors are never retried (server may have persisted) — at-most-once preserved. This is the crux of the fix and it is correct.
2. Reconnect cannot be redirected. reconnect() uses the stored immutable socket_path and performs an inline Ping/Pong (not via call()), avoiding the connect()→call(Ping) recursion. Fresh stream is swapped into the held guard only on a verified Pong.
3. Fail-closed everywhere — exhaustion, non-Pong, and reconnect failure all return RpcTransportError; never a silent drop.
4. Bounded — MAX_ATTEMPTS=3, 50ms backoff; zero happy-path overhead (reconnect/metrics/backoff only on the error branch).
5. EPIPE detection matches repo precedent: portable ErrorKind::BrokenPipe OR raw errno == 32.
6. No payload leakage — diagnostics carry only transport metadata (endpoint, phase, errno); confidentiality asserted by a test with a sentinel.
7. tracing + metrics only; no new print!/eprintln!. No wire-format or public-API change; runtime_ipc untouched.

Non-blocking observations (optional; no change required)

• thread::sleep(BACKOFF) runs while holding the stream Mutex guard. Because the single shared stream already serializes all callers through this mutex, it adds no contention beyond the existing design — noted for awareness only.
• Backoff is fixed 50ms (not exponential). With max 3 attempts (~100ms worst-case added latency) this is appropriate for the durability goal.
• The "message too large" path now surfaces via ipc_err("write-len", InvalidInput) rather than the former explicit RpcTransportError{"message too large"}. The byte-count detail is preserved in the message; behavior (fail-closed, non-retriable) is unchanged.

Conclusion: Implementation is correct, complete, well-tested, and philosophy-compliant (surgical edits, fail-closed, no silent fallback). Ready to merge.

[rysweet]

Step 17c — Security Review (issue #4731 memory-IPC write-path resilience)

Verdict: PASS — no security-blocking findings. Reviewed the actual PR diff (6 files: src/memory_ipc/client.rs, src/memory_ipc/mod.rs, tests_epipe_resilience_4731.rs, docs/*, mkdocs.yml), not just claims.

Checklist

• Security requirements met — fail-closed contract holds: retry exhaustion, non-Pong handshake, and reconnect failure all return RpcTransportError; never a silent Ok/dropped write.
• No new vulnerabilities — no unsafe, no std::process/Command, no env::var retry knobs, no chmod/set_permissions, no new deps, no wire-format/public-API change. Attack surface unchanged.
• Sensitive-data handling — verified all warn!/error!/debug! events and the RpcTransportError message carry only transport metadata (endpoint, attempt, phase, error_kind, errno, socket_path). Zero payload/episode/distillation bytes in logs (grep of tracing macros for payload/body/request/episode → NONE).
• AuthN/AuthZ — n/a (local Unix-socket IPC); no auth logic changed.
• Injection — n/a; payload is length-prefixed JSON over a Unix socket, no shell/SQL/interpolation introduced.

Security-relevant strengths

1. TOCTOU / socket-redirection defense — reconnect uses the stored immutable socket_path (&self.socket_path); it never re-derives, creates, chmods, or falls back to a more permissive socket.
2. Peer verification before resend — reconnect performs an inline Ping/Pong and swaps the stream only on an exact Pong; on anything else the real payload is not resent.
3. DoS-hardened bounds — retry is capped by compile-time constants (MAX_ATTEMPTS=3, BACKOFF=50ms), no env/config amplifier; existing 30s socket timeouts + 8 MiB MAX_FRAME re-enforced on every (incl. post-reconnect) read.
4. Data-integrity / at-most-once — retries confined to write-half EPIPE (server never committed); read-half is never retried, avoiding duplicate mutation.

Non-blocking observations (defense-in-depth, no change required)

• Malicious-peer-at-path: a process able to bind {socket_dir}/memory.sock could answer Pong and receive the payload — but this is pre-existing (original connect() already trusts that path) and not broadened here; the immutable-path + no-permissive-fallback design actually tightens it. Mitigation remains socket-dir permissions (out of scope for this PR).
• Sleep-under-lock: BACKOFF + reconnect run while holding the stream Mutex; bounded (~3×(50ms+reconnect)) and required to preserve write ordering — availability tradeoff, not a security issue.

Independently re-verified gates green (fmt, clippy --all-targets 0 warnings, cargo test --lib memory_ipc 73/73 incl. 4 EPIPE regression tests). No code changes required.

[rysweet]

Step 17d — Philosophy Guardian Review (issue #4731 memory-IPC write-path resilience)

Verdict: PASS. Independently read the full PR diff (client.rs, mod.rs, test file, docs) — not just prior claims — and assessed against amplihack philosophy. No blocking findings; no code changes warranted.

Compliance checklist

• Ruthless simplicity achieved — The fix is a single bounded retry loop in call(). Payload is serialized once before the loop and reused across retries. No new abstraction layers, no config machinery, no speculative extensibility. MAX_ATTEMPTS=3 + fixed 50ms backoff are constants, not knobs.
• Bricks & studs pattern followed — Clean, minimal seams: is_broken_pipe(&io::Error) and write_frame_raw are small helpers with single, documented contracts. write_frame is kept as a thin wrapper over write_frame_raw, so the server and every other caller are unchanged. connect_stream is factored out so reconnect reuses the exact connect+timeout setup while doing an inline handshake — deliberately breaking the connect() → call(Ping) recursion rather than papering over it.
• Zero-BS implementation — No stubs, no todo!/unimplemented!, no faked APIs, no swallowed exceptions. Every error path is fail-closed: write-half exhaustion, non-Pong reconnect, and reconnect failure all return RpcTransportError — never a silent Ok, never a dropped write. The only let _ = is on best-effort socket timeout setup (pre-existing, correct semantics).
• No over-engineering — Retry is confined to the write-half EPIPE case only (server never committed ⇒ safe idempotent resend); read-half is never retried (preserves at-most-once, avoids duplicate mutation). No exponential backoff, jitter, or circuit-breaker was added where a bounded loop suffices.
• Clean module boundaries — Transport helpers live in mod.rs; client retry logic in client.rs; regression tests isolated in tests_epipe_resilience_4731.rs. The blast radius stays entirely within src/memory_ipc/.

Additional philosophy-relevant strengths

• Zero happy-path overhead — reconnect, metrics::increment, and backoff execute only on the EPIPE error branch; the success path is unchanged.
• No payload leakage — all diagnostics (tracing + metrics) carry transport metadata only (endpoint, phase, errno, kind), never request contents.
• Regeneratable — the module's contract (serialize once, bounded write-half retry, fail-closed) is documented inline and in docs/reference/memory-ipc-write-resilience.md, linked from docs/memory.md and mkdocs.yml.

One benign, non-blocking observation

thread::sleep(BACKOFF) runs while the stream Mutex guard is held. This is intentional and simpler than release/reacquire: the single UnixStream is inherently serialized, so no concurrent caller could make progress on it anyway. Releasing the lock around the sleep would add complexity for no correctness or throughput gain. No change recommended.

Implementation is correct, minimal, and philosophy-aligned. Approved from a philosophy-compliance standpoint.

[github-actions]

📊 Coverage Summary

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

Module	Lines	Covered	Coverage
Total	199034	167610	84.2%

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

[rysweet]

Auto-closing this stale CONFLICTING auto-generated documentation draft: it can no longer merge cleanly and is superseded by the canonical open auto-doc PR #4826. Enforcing the single-open auto-doc PR invariant (goal_hygiene).