[gunwooterry]

TL;DR

• Make the closed-stdout CLI regression deterministic without changing CLI behavior.

Tier: internal

Background

The macOS merge-queue check intermittently observed exit 0 instead of 141 on a closed stdout pipe. Another test thread can spawn a process while the pipe reader is briefly inheritable, leaving a reader alive after the test drops its own handle. Small writes then succeed legitimately.

What changes

CLI and library behavior are unchanged.

• Covered: the existing command matrix, exit 141 and empty-stderr assertions, executed in an isolated single-test subprocess with an exact success witness.
• Not covered: changes to Rust's pipe implementation or other subprocess tests.

After merging: None.

Decisions

• Isolate pipe ownership rather than increase payloads, add sleeps, retry failures or serialize the entire suite. The existing tiny-PCM regression uses the same subprocess pattern.
• Keep the original assertions and command coverage. The parent requires the child's named success line, so selecting zero tests cannot pass.

Evidence

Verification run: On macOS arm64, 100 unmodified CLI-suite loops with --test-threads=32 passed the target test. With temporary instrumentation spawning /bin/sleep 0.01 continuously from 16 parent threads, the original assertion failed 9/500 times, always with exit 0 instead of 141. With isolation and the same parent spawn pressure, 0/500 failed. Pressure instrumentation is not included in this PR.

A separate 100,000-pipe diagnostic accepted 19 writes after dropping the local reader during concurrent spawns. Every affected pipe returned BrokenPipe once the spawned children exited. With no concurrent spawns, 0/100,000 writes succeeded.

Local checks passed: cargo fmt --all --check, cargo clippy -p coreav-cli --test cli -- -D warnings, cargo test -p coreav-cli --test cli -- --test-threads=32 with 67 passed and 7 ignored, another 100/100 complete uninstrumented CLI-suite loops at the committed head, cargo deny --all-features check, all 31 fuzz-project license checks, workflow and public-text self-tests.

At this head, the Linux check and macOS check both logged test a_closed_standard_output_exits_141_quietly ... ok. Their CLI suites finished with 68 passed and 7 ignored on Linux, and 67 passed and 7 ignored on macOS.

Similarity passed at 317f8048adaddd039a02e155fb31594fbf858f88 against the main allowlist: 110 spans of 50+ tokens, 0 not allowlisted, 0 problems, 1 stale entries with stale entries reported rather than failed.

Links: None.

• Every commit is signed off (git commit -s, DCO).
• I copied nothing from other implementations.

---

^{Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.}

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The closed-standard-output test now runs in a guarded subprocess and checks its status and passing-test output. The CI documentation describes the subprocess checks for both closed-reader tests.

Changes: Closed-stdout test isolation

Layer / File(s)	Summary
Guarded test execution crates/coreav-cli/tests/cli.rs, docs/agents/ci.md	The test launches itself with COREAV_CLI_CLOSED_READER_PROBE set, then checks that the subprocess succeeds and reports the test as passing. The CI table documents the subprocess checks for both closed-reader tests.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Other

Merge Risk: Minimal · up to 317f8

No merge-blocking issue is identified. The change isolates the test without an established production behavior change.

Pre-merge checks: 5

Check name	Status	Explanation
Title check		The title "test(cli): isolate closed stdout assertions" matches the main change. The PR runs the closed-stdout CLI test in an isolated single-test subprocess with an exact success witness, so the asse…
Description check		The description follows the template's main sections: TL;DR, Background, What changes, Decisions, and Evidence. It explains the flaky macOS symptom (exit 0 instead of 141), the cause (a pipe reader le…
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Title check

Explanation

The title "test(cli): isolate closed stdout assertions" matches the main change. The PR runs the closed-stdout CLI test in an isolated single-test subprocess with an exact success witness, so the assertions no longer depend on other test threads. It follows Conventional Commits format, is under 72 characters, and names the scope and outcome.

Full details: Description check

Explanation

The description follows the template's main sections: TL;DR, Background, What changes, Decisions, and Evidence. It explains the flaky macOS symptom (exit 0 instead of 141), the cause (a pipe reader left inheritable during concurrent spawns), the isolation fix, and the verification run. Gaps: the TL;DR does not state the change for a specific audience, the 'Tier: internal' line is outside the template, the 'How it works' section is omitted (acceptable, since the change stays inside one test layer), and the 'Links' entry says None. The checklist is ticked. Linux and macOS CI evidence is still pending, which the author states.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files. (1 skipped: 1 unsupported.)

Finishing Touches

Generate docstrings

• Commit to this branch
• Create a new PR

Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

• Source mode: Auto
• Repository: coreav-dev/coreav
• Branch: main
• File: .coderabbit.yaml
• Commit: 0fc9f53
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9969e626-f4e7-403c-ae34-d2b44cc59a67

Commits

Reviewing files that changed from the base of the PR and between 0fc9f53 and 317f804.

Files selected for processing: 2

• crates/coreav-cli/tests/cli.rs
• docs/agents/ci.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

[capy-ai]

Review: no blocking issue

Reviewed head 317f8048adaddd039a02e155fb31594fbf858f88 (diff: crates/coreav-cli/tests/cli.rs +21, docs/agents/ci.md one row).

Same behaviour proved. The probe body below the new guard is byte-for-byte the old test: the same command matrix (succeeding_commands, remux, sanitize, extract-audio, clip, both HLS commands), a pipe whose reader is dropped before spawn, and status.code() == Some(141) plus empty stderr for every command. Only where the pipe is created moved.

Isolation cannot mask a regression. The CLI exiting 0 or writing to stderr fails an assertion in the child, the child exits non-zero, and the parent's result.status.success() fails with the child's output in the message. A filter that selects zero tests (a rename, the test moving into a module) is caught by the required test a_closed_standard_output_exits_141_quietly ... ok witness, because libtest still exits 0 with "0 passed". The child runs --test-threads=1, so nothing else in that process spawns while its pipes exist, which removes the inheritance race the PR describes. This matches the existing tiny_pcm_stdout_reports_a_closed_reader_during_final_flush pattern in tests/ffmpeg_pcm.rs.

CI runs it by name. check (Linux), check-macos and check-windows run cargo test --workspace, and tests/cli.rs is a default test target. scripts/oracle_free.py passes through unchanged on darwin arm64 and only adds --skip for names in test-support/oracle-dependent.json elsewhere, and this name isn't in that list. These jobs were still queued at review time, so before merge the check and check-macos logs should show test a_closed_standard_output_exits_141_quietly ... ok.

Non-blocking:

• The parent's own .output() capture pipes can still be inherited by a sibling test's concurrent spawn. That only delays EOF until the sibling's child exits. It can't flip the verdict.
• The docs row now covers both tests and reads accurately.

CodeRabbit had no inline findings when this was posted (still running). CodeRabbit: 0 confirmed, 0 minor, 0 false positive