fix(push): a self-inflicted closed pipe must not mask the remote ingest error

[shujaatTracebloc]

Found while debugging a real failed ingest on Windows. The user saw only this:

Error: building tar archive: packaging ...\sequences\seq_0000004.txt: io: read/write on closed pipe

which says nothing about what actually went wrong.

What actually happened

The remote script is set -e; rm -rf D; mkdir -p D; tar -xf - -C D. The dataset dir was root-owned (hostPath volumes ignore fsGroup), so mkdir failed with EACCES and the shell aborted before tar read a single byte. StreamLayout's own pr.Close() then unblocked the tar goroutine with io.ErrClosedPipe — and because tarErr is reported first, the actionable cause was thrown away:

mkdir: can't create directory '/data/shared/<ds>': Permission denied

That message was already sitting in stderrBuf. Discarding it turned a one-line permissions fix into a long investigation.

The fix

io.ErrClosedPipe on the tar side is not a cause — it is the consequence of our own pr.Close() after exec returned early. So when tarErr is ErrClosedPipe and streamErr is set, defer to streamErr, which carries the remote-stderr hint.

Deliberately narrow, to preserve the existing ordering rationale:

• a tar error from any other cause (e.g. the stream-time size-cap recheck, whose diagnostic Bugbot originally asked to keep) still wins;
• a closed-pipe tar error with no streamErr is still reported, not swallowed.

Tests (+2)

• TestStreamLayout_ClosedPipeDoesNotMaskRemoteError — pins the customer-visible outcome: the remote Permission denied survives and building tar archive does not appear.
• TestStreamLayout_TarErrorStillWinsWhenNotClosedPipe — guards the other direction. It uses a drained stream so the tar goroutine reaches a genuine open error; writing it undrained proved the premise wrong (the first header write blocks and fails with ErrClosedPipe, which is exactly the self-inflicted case), and that mistake is recorded in the test comment.

Full suite green; gofmt + go vet clean.

Note on the underlying permissions bug

The mkdir failure itself is already fixed chart-side (init-writable-data, tracebloc/client#612, chart 1.9.25+). The install that hit this had pulled the published chart 1.9.15, which predates it — so publishing a chart ≥1.9.25 removes the root cause. This PR is about never losing the diagnosis again.

🤖 Generated with Claude Code

---

Note

Low Risk
Narrow change to error reporting order in dataset push streaming only; no protocol or ingest semantics change, with regression tests for both branches.

Overview
When the remote set -e ingest script fails before draining stdin (e.g. mkdir: Permission denied on a root-owned dataset dir), StreamLayout closes the pipe and the local tar goroutine returns io.ErrClosedPipe. That error was reported first as "building tar archive", hiding the actionable remote stderr already captured in stderrBuf.

StreamLayout now treats ErrClosedPipe + non-nil exec/stream error as self-inflicted: it surfaces the streaming error (with the remote stderr hint) instead of the closed-pipe tar message. Genuine tar failures (size cap, missing files, etc.) still win; ErrClosedPipe with no stream error is still returned as a tar build error.

Adds TestStreamLayout_ClosedPipeDoesNotMaskRemoteError and TestStreamLayout_TarErrorStillWinsWhenNotClosedPipe to lock both behaviors.

^{Reviewed by Cursor Bugbot for commit 0cfed0e. Bugbot is set up for automated code reviews on this repo. Configure here.}

[aptracebloc]

Approving — verified the error-precedence logic across every case.

if tarErr != nil && !(errors.Is(tarErr, io.ErrClosedPipe) && streamErr != nil):

• self-inflicted closed pipe + streamErr → skips the tar message, surfaces streamErr with the remote-stderr hint (the fix);
• any other tar error (e.g. the size-cap recheck) → still wins;
• closed pipe with no streamErr → still reported;
• uses errors.Is (wrap-safe), not ==.

Good instinct on the #nosec G304 waiver too — the path comes from the symlink-rejecting walk plus the Lstat re-guard at stream time, so that's a properly-justified per-site waiver for the now-required gosec gate. Tests pin both directions; Bugbot clean, CI green.

Non-blocking nit: the "belt and braces" block after the streamErr return is unreachable —

if tarErr != nil { return fmt.Errorf("building tar archive: %w", tarErr) }

The only way to skip the first if with tarErr != nil is closed-pipe and streamErr != nil, which then returns in the streamErr block; the closed-pipe-no-streamErr case it's meant to guard already returns in the first if. So it can never fire. Harmless (behavior is correct either way) — just dead code you could drop. Not blocking.

LGTM.

— drafted with Claude (Opus 4.8), sent by @aptracebloc