fix(cli): treat stdout broken pipe as success - #366
Conversation
WriteJSON returned EPIPE to main, so pipes like wacli accounts list | head exited 1 after a successful listing. Ignore broken pipe on the JSON success write, matching WriteError. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
|
🦞👀 Pull request received. I will update this pull request when review starts. |
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. Reviewed August 23, 2026, 10:02 AM ET / 14:02 UTC. ClawSweeper reviewWhat this changesThe PR makes successful JSON output ignore closed stdout pipes on Unix and Windows while preserving unrelated write errors. Merge readinessKeep open for normal maintainer merge review: current main and v0.17.1 still return stdout write errors, while this focused patch makes expected closed-pipe errors successful and retains other failures. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: PASS (completed) Assertions:
How this fits togetherwacli commands serialize successful results through the output package before the CLI returns an exit status. That output is consumed by terminals, pipes, and scripts, so a stdout write error can otherwise turn a completed command into a failure. flowchart LR
A[Completed CLI command] --> B[JSON output formatter]
B --> C[Write to stdout]
C --> D{Closed pipe?}
D -->|Yes| E[Return success]
D -->|No| F[Return write error]
E --> G[Exit zero]
F --> H[Exit nonzero]
Before merge
Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Merge the focused output-layer handling so completed JSON commands remain successful when a downstream reader intentionally closes stdout. Do we have a high-confidence way to reproduce the issue? Yes. Current main returns the stdout write error through the command root to main's nonzero exit path, and the PR supplies an after-fix real closed-kernel-pipe result. Is this the best way to solve the issue? Yes. Handling only recognized closed-pipe errors at the shared JSON output boundary is the narrowest maintainable repair and preserves unrelated write failures. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 97e14efdf91a. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (2 earlier review cycles) |
Recognize ERROR_BROKEN_PIPE and ERROR_NO_DATA in addition to Unix EPIPE. Leave CHANGELOG.md to the release workflow. Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Fixed on
Dropped the Unreleased changelog hunk in the same commit. @clawsweeper re-review |
clawsweeper
Bot
added
rating: 🐚 platinum hermit
1 participant
JSON success output from
waclishould not fail the process when the downstream reader goes away.What Problem This Solves
internal/out.WriteJSONreturns the error fromfmt.Fprintlnon stdout.cmd/wacli/main.gothen callsos.Exit(1)on any execute error. A successful command that writes JSON to a closed pipe (for examplewacli accounts list | head) can therefore exit 1 even though the listing itself succeeded.WriteErroralready ignores write errors on stderr. The success path did not.This is the same class of CLI contract GitHub CLI already adopted: ignore EPIPE when the pager or pipe reader closes (cli/cli#5143).
Why
The command work is already done when
WriteJSONruns. EPIPE means the reader left, not thatwaclifailed. Propagating that write error makes pipelines andheadlook like product failures.The helper treats
syscall.EPIPEas success and also unwraps*fs.PathError/*os.PathErrorso the usualwrite |1: broken pipeform is covered. Other write errors (for example disk full) still return.User Impact
Scripts and interactive pipes that stop reading early no longer get a false non-zero status from an otherwise successful JSON command.
set -eand CI steps that pipewacli --jsonintohead,jq, or a pager stop failing for a closed stdout.Evidence
Call chain:
accounts list/chats list/messages list(and every other JSON command) ->out.WriteJSON(os.Stdout, ...)->fmt.Fprintln->executeerror ->os.Exit(1)incmd/wacli/main.go.The write error was introduced with the original output helper in
8b725754b368(2025-12-12, 249 days ago).Live closed pipe, unfixed
WriteJSON(reader closed, then write):Live closed pipe after this patch, same program:
The program creates an
os.Pipe, closes the read end, then callsout.WriteJSONon the write end. That is a real kernel EPIPE (write |1: broken pipe), not a stubbed errno.Related:
Real behavior proof
Behavior or issue addressed: Successful JSON writes to a closed stdout pipe returned EPIPE, so
wacliexited 1 after a completed listing.Real environment tested: macOS, Go go1.26.6, branch
fix/stdout-broken-pipeat this patch,/tmp/pr-wacli-epipe.Exact steps or command run after this patch: Restored unfixed
WriteJSONfromupstream/main, ran ago runprogram that writes JSON to a closedos.Pipe, recorded the error. Restored this patch and ran the same program again.Evidence after fix: terminal output from the closed-pipe program:
Unfixed tree:
After this patch:
Observed result after fix: The same closed-pipe write now returns nil, so
executedoes not tripos.Exit(1). Other write errors are still returned.What was not tested: Windows
WSAECONNRESET(no existing helper in-tree). A live WhatsApp-backedaccounts list | headagainst a paired store.Summary
WriteJSONnow treats broken pipe as success.WriteErroris unchanged. Store upsert paths are not touched.