Skip to content

fix: make command and pipeline lifecycles failure-safe#5

Merged
cmilesio merged 2 commits into
mainfrom
chore/library-quality-pass
Jul 16, 2026
Merged

fix: make command and pipeline lifecycles failure-safe#5
cmilesio merged 2 commits into
mainfrom
chore/library-quality-pass

Conversation

@cmilesio

@cmilesio cmilesio commented Jul 15, 2026

Copy link
Copy Markdown
Member

Why

Process execution gets difficult when only part of an operation succeeds: a downstream pipeline stage fails to start, stdout and stderr arrive together, a writer accepts only part of a chunk, or a timeout replaces an already-canceled context.

This pass makes those paths settle cleanly without losing output, leaking processes, racing callbacks, or changing execx's public v1 error model.

What

  • Track pipeline setup, start, wait, output, and cleanup failures separately.
  • Close pipes, kill, and reap already-started stages when partial startup fails.
  • Serialize stdout/stderr callbacks and caller-provided writers.
  • Capture bytes before writer fan-out so the received prefix survives writer failure.
  • Flush the final unterminated line for normal and PTY output.
  • Return reader, writer, and PTY copy failures through ErrExec.
  • Keep the original context parent when replacing timeouts or deadlines.
  • Copy StdinBytes input so later caller mutation cannot change execution.
  • Avoid writer-identity panics for non-comparable dynamic values.
  • Document pipeline selection, mutability, shell, logging, and error boundaries.

Before / after

Area Before After
Final output line Dropped without \n Flushed when the stream closes
Writer failure Error or rejected chunk could disappear Captured prefix retained; ErrExec returned
stdout/stderr Could enter shared callbacks concurrently Serialized delivery
Context replacement Could detach from a canceled parent Parent cancellation is preserved
StdinBytes Retained the caller's slice Copies at configuration time
Pipeline startup No complete partial-start cleanup path Pipes close; started stages are killed and reaped
PTY copy Failures could be lost Uses normal execution-error reporting

The existing API now also delivers a child's last unterminated line:

_, _ = execx.Command("printf", "ready").
	OnStdout(func(line string) { lines = append(lines, line) }).
	Run()

// lines == []string{"ready"}

Compatibility

This is not a breaking API release: exported signatures and execx's v1 child-exit model remain unchanged.

The observable corrections surface failures that were previously lost, retain final unterminated output, preserve parent cancellation, and copy caller-owned input. Callers that unknowingly relied on swallowed reader, writer, or PTY errors may now receive ErrExec; that is the documented error boundary working as intended rather than a new error model.

The non-Unix build fallbacks are additive and keep the same no-op process-control contract used on unsupported platforms.

Notes

There are no exported signature changes. Non-zero child exit remains data in Result.ExitCode, not a Go error.

Intentional behavior corrections:

  • unterminated output reaches callbacks;
  • caller writer, input reader, and PTY copy failures return errors;
  • StdinBytes no longer observes later slice mutation;
  • replacement timeouts and deadlines stay attached to their parent.

Callbacks are serialized, so slow callback work applies backpressure. Cmd remains mutable and should not be configured or executed concurrently.

Review

  1. pipeline.go covers partial startup, wait ordering, cleanup, and strict/best-effort result selection.
  2. execx.go contains writer construction, context ancestry, and input ownership.
  3. execx_test.go and PTY tests capture the corrected failure paths.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cmilesio cmilesio changed the title Harden command execution lifecycle fix: make command and pipeline lifecycles failure-safe Jul 15, 2026
@cmilesio
cmilesio merged commit 7bbe63d into main Jul 16, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants