[PR #4038] fix(cli): preserve command status after complete pipe reads #6009

New issue

Open

opened 2026-10-31 01:11:08 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:11:08 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4038
Author: @zhangguiping-xydt
Created: 10/15/2026
Status: 🔄 Open

Base: main ← Head: fix/high-priority-web-20261015

📝 Commits (10+)

f2298cd fix(cli): flush piped diagnostics before exiting
f21a87c chore(cli): integrate upstream cancellation repair
6a7c648 test(providers): retain host prerequisites after upstream sync
b87395b chore(cli): integrate upstream fixture fixes and retain close-order coverage
69b7e7b test(kuna): assert caller context and graph producer attribution
7459b58 chore(cli): integrate canonical Kuna failure-context repair
edb9fce chore(cli): reconcile stdout flushing with upstream error recovery
7400745 fix(cli): preserve status after complete diagnostic reads
3a074f3 Merge branch 'main' into fix/high-priority-web-20261015
544df5e fix(cli): preserve command status after complete pipe reads

📊 Changes

5 files changed (+89 additions, -53 deletions)

View changed files

📝 scripts/rea.mjs (+19 -2)
📝 src/cli/cliResultOutput.ts (+1 -7)
📝 tests/boundary/cli/closedStdout.test.ts (+55 -6)
📝 tests/boundary/cli/jsonOutput.test.ts (+0 -38)
➕ tests/fixtures/stdoutReadCompletion.mjs (+14 -0)

📄 Description

What Problem This Solves

Addresses the maintainer's request to focus on current main. Main already fixes #3894's truncated diagnostics. The remaining case is a reader consuming the complete diagnostic and closing before the writer's completion callback: main's additional empty write can report EPIPE and replace the command's exit 1 with 141.

Why This Change Was Made

The launcher now waits for completion of its actual stdout writes after worker cleanup and SDK-output finalization. Remove the empty-write fence from the result-output owner; its own formatted/JSONL writes already await their callbacks. Genuine EPIPE still yields quiet status 141.

Merged current main 5cf7133f8 and removed the unrelated Cutter synchronization change and the broad truncation-only test. Replace the two result-owner fence tests with public CLI coverage for the remaining lifecycle distinction. The reusable test worker captures delegated writes synchronously, so it needs no replacement fence.

User Impact

A fully delivered diagnostic retains the command's status. Readers closing before or during output still receive quiet status 141. No command, schema, provider, target authority, package metadata, or MCP behavior changes.

Evidence

Validated commit: 544df5e51b09760b68780198d8ca4371936d0c3e. Linux x64; Node 24.22.1 and npm 11.16.0. Focused tests and static checks ran in a Debian Bookworm container because the host glibc cannot run the pinned native compiler. Dependencies are locked and installed with scripts disabled.

Current-main failure: with production sources from 5cf7133f8, the controlled real-subprocess regression receives complete JSON with 2,048 field errors but exits 141; the other five closed-stdout cases pass. The same regression passes with exit 1 at this head. The fixture delays the real write callback until the reader acknowledges receipt and closure; it injects no EPIPE and uses no timing sleep.
Actual built CLI at this commit: file redirection, a draining pipe, an ordinary complete-document close, and the controlled close each deliver identical 404,572 bytes, valid JSON with 2,048 field errors, and exit 1. A reader closed before output exits 141, with no stderr. The ordinary complete-close probe also passed on main; deterministic race coverage is provided by the controlled case.
npm run test:focused -- tests/boundary/cli/closedStdout.test.ts tests/boundary/cli/jsonOutput.test.ts tests/boundary/cli/jsonLinesOutput.test.ts tests/boundary/cli/formattingRecovery.test.ts tests/boundary/cli/dispatcher.test.ts tests/process-global/cliUpdateAlias.test.ts tests/process-global/updateOutput.test.ts — 136 passed across seven files. Includes early/mid-output close, JSON/JSONL formatting, cleanup/output-failure precedence, and update behavior.
npm run check:fast — passed (typecheck, lint, module boundaries).
Scoped oxfmt --check — passed for all five changed files.
npm run verify:test-discovery — passed, all 2,632 test files discovered exactly once.
npm run verify:architecture-guards — passed across 2,149 source files.

Current-head CI completed with unrelated baseline failures. Source tests fail providerBinding.test.ts (bundle-root identity); Windows fails ghidraJavaProbe.test.ts (empty Java bitness). Both exact cases also fail in main CI at 5cf7133f8. An independent checkout of that main commit reproduces the provider-binding failure locally (1 failed, 4 passed), matching this head. All nine failing cases printed in matrix shards 3/7/8 also occur in that main run; the matrix cancelled remaining work, so no full-suite pass is claimed.

The six closed-stdout tests and 44 JSON-output tests passed in remote CI before matrix cancellation. Remote static checks, build, JavaScript recovery, Linux x64/arm64 and macOS arm64 package baselines passed. Greptile reviewed this exact commit: 5/5, no new findings. No local macOS/Windows or real Frida/Hopper/Ghidra verification is claimed. Invalid CLI inputs are rejected before provider/target access.

Author readiness

Human owner: @zhangguiping-xydt

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/4038 **Author:** [@zhangguiping-xydt](https://github.com/zhangguiping-xydt) **Created:** 10/15/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/high-priority-web-20261015` --- ### 📝 Commits (10+) - [`f2298cd`](https://github.com/morluto/rea/commit/f2298cda0984880749306731763c3153759f1911) fix(cli): flush piped diagnostics before exiting - [`f21a87c`](https://github.com/morluto/rea/commit/f21a87c98a8f9ccbf045b48998aa57990354a54d) chore(cli): integrate upstream cancellation repair - [`6a7c648`](https://github.com/morluto/rea/commit/6a7c64811790e4af0434d48f741153ec06e39625) test(providers): retain host prerequisites after upstream sync - [`b87395b`](https://github.com/morluto/rea/commit/b87395b4f1fd57b9c3e00650a5cb32cabb519f0a) chore(cli): integrate upstream fixture fixes and retain close-order coverage - [`69b7e7b`](https://github.com/morluto/rea/commit/69b7e7b761c4d9eddd8c65cc278de1aaa6f1796c) test(kuna): assert caller context and graph producer attribution - [`7459b58`](https://github.com/morluto/rea/commit/7459b5800de1512c32493265e636fe5db2f97bd9) chore(cli): integrate canonical Kuna failure-context repair - [`edb9fce`](https://github.com/morluto/rea/commit/edb9fceab579f0e4981bd4e5b2611576ad386270) chore(cli): reconcile stdout flushing with upstream error recovery - [`7400745`](https://github.com/morluto/rea/commit/74007450c03ae0dca66b81edc70c342203df3c4d) fix(cli): preserve status after complete diagnostic reads - [`3a074f3`](https://github.com/morluto/rea/commit/3a074f38f725b9c8de13a802574bcb0c2b9c7d8d) Merge branch 'main' into fix/high-priority-web-20261015 - [`544df5e`](https://github.com/morluto/rea/commit/544df5e51b09760b68780198d8ca4371936d0c3e) fix(cli): preserve command status after complete pipe reads ### 📊 Changes **5 files changed** (+89 additions, -53 deletions) <details> <summary>View changed files</summary> 📝 `scripts/rea.mjs` (+19 -2) 📝 `src/cli/cliResultOutput.ts` (+1 -7) 📝 `tests/boundary/cli/closedStdout.test.ts` (+55 -6) 📝 `tests/boundary/cli/jsonOutput.test.ts` (+0 -38) ➕ `tests/fixtures/stdoutReadCompletion.mjs` (+14 -0) </details> ### 📄 Description ## What Problem This Solves Addresses the [maintainer's request to focus on current main](https://github.com/morluto/rea/pull/4038#issuecomment-6428054080). Main already fixes #3894's truncated diagnostics. The remaining case is a reader consuming the complete diagnostic and closing before the writer's completion callback: main's additional empty write can report EPIPE and replace the command's exit 1 with 141. ## Why This Change Was Made The launcher now waits for completion of its actual stdout writes after worker cleanup and SDK-output finalization. Remove the empty-write fence from the result-output owner; its own formatted/JSONL writes already await their callbacks. Genuine EPIPE still yields quiet status 141. Merged current main `5cf7133f8` and removed the unrelated Cutter synchronization change and the broad truncation-only test. Replace the two result-owner fence tests with public CLI coverage for the remaining lifecycle distinction. The reusable test worker captures delegated writes synchronously, so it needs no replacement fence. ## User Impact A fully delivered diagnostic retains the command's status. Readers closing before or during output still receive quiet status 141. No command, schema, provider, target authority, package metadata, or MCP behavior changes. ## Evidence Validated commit: `544df5e51b09760b68780198d8ca4371936d0c3e`. Linux x64; Node 24.22.1 and npm 11.16.0. Focused tests and static checks ran in a Debian Bookworm container because the host glibc cannot run the pinned native compiler. Dependencies are locked and installed with scripts disabled. - **Current-main failure:** with production sources from `5cf7133f8`, the controlled real-subprocess regression receives complete JSON with 2,048 field errors but exits **141**; the other five closed-stdout cases pass. The same regression passes with exit **1** at this head. The fixture delays the real write callback until the reader acknowledges receipt and closure; it injects no EPIPE and uses no timing sleep. - **Actual built CLI at this commit:** file redirection, a draining pipe, an ordinary complete-document close, and the controlled close each deliver identical **404,572 bytes**, valid JSON with **2,048 field errors**, and exit **1**. A reader closed before output exits **141**, with no stderr. The ordinary complete-close probe also passed on main; deterministic race coverage is provided by the controlled case. - `npm run test:focused -- tests/boundary/cli/closedStdout.test.ts tests/boundary/cli/jsonOutput.test.ts tests/boundary/cli/jsonLinesOutput.test.ts tests/boundary/cli/formattingRecovery.test.ts tests/boundary/cli/dispatcher.test.ts tests/process-global/cliUpdateAlias.test.ts tests/process-global/updateOutput.test.ts` — **136 passed** across seven files. Includes early/mid-output close, JSON/JSONL formatting, cleanup/output-failure precedence, and update behavior. - `npm run check:fast` — **passed** (typecheck, lint, module boundaries). - Scoped `oxfmt --check` — **passed** for all five changed files. - `npm run verify:test-discovery` — **passed**, all 2,632 test files discovered exactly once. - `npm run verify:architecture-guards` — **passed** across 2,149 source files. [Current-head CI](https://github.com/morluto/rea/actions/runs/40432657587) completed with unrelated baseline failures. Source tests fail `providerBinding.test.ts` (bundle-root identity); Windows fails `ghidraJavaProbe.test.ts` (empty Java bitness). Both exact cases also fail in [main CI at `5cf7133f8`](https://github.com/morluto/rea/actions/runs/40430498397). An independent checkout of that main commit reproduces the provider-binding failure locally (1 failed, 4 passed), matching this head. All nine failing cases printed in matrix shards 3/7/8 also occur in that main run; the matrix cancelled remaining work, so no full-suite pass is claimed. The six closed-stdout tests and 44 JSON-output tests passed in remote CI before matrix cancellation. Remote static checks, build, JavaScript recovery, Linux x64/arm64 and macOS arm64 package baselines passed. Greptile reviewed this exact commit: **5/5**, no new findings. No local macOS/Windows or real Frida/Hopper/Ghidra verification is claimed. Invalid CLI inputs are rejected before provider/target access. ## Author readiness Human owner: @zhangguiping-xydt - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-31 01:11:08 +00:00

bsakaguchi referenced this issue

2026-11-01 21:45:30 +00:00

[PR #6009] [MERGED] fix(reconstruction): keep complete contradictions beside incomplete entries #7118