diff --git a/internal/process/process.go b/internal/process/process.go index 44827071be..4d44aaee29 100644 --- a/internal/process/process.go +++ b/internal/process/process.go @@ -29,7 +29,15 @@ const termType = "xterm-256color" // fires before os/exec's WaitDelay-triggered kill, keeping cancellation // behaviour unchanged, while still bounding the post-exit I/O wait so a // leaked stdout/stderr pipe can never hang Cmd.Wait indefinitely. -const waitDelayBuffer = 1 * time.Second +// +// The bound is deliberately generous: when it expires, os/exec force-closes +// the output pipes and any not-yet-drained output is silently lost (Wait +// returns ErrWaitDelay, which complete() treats as a clean exit). A 1s buffer +// proved too tight on loaded Windows CI machines, where the post-exit drain of +// fast hooks intermittently exceeded it and hook output went missing. Hitting +// this bound should only ever mean a genuinely leaked pipe write-end, not a +// slow-but-healthy drain. +const waitDelayBuffer = 30 * time.Second // afterPTYStartHook lets tests force work to happen after the PTY helper // returns so they can verify raw-mode ordering around process startup. @@ -99,6 +107,12 @@ type Config struct { SignalGracePeriod time.Duration Started chan struct{} Done chan struct{} + + // WaitDelay, when positive, overrides the derived Cmd.WaitDelay bound + // (SignalGracePeriod + waitDelayBuffer) for the non-PTY path. Production + // code leaves this zero; tests use it to keep leaked-pipe regression + // tests fast. + WaitDelay time.Duration } // Process is an operating system level process @@ -315,6 +329,9 @@ func (p *Process) startWithoutPTY(context.Context) (func(), error) { // cancellation; this keeps cancellation behaviour identical to before while // still bounding the post-exit I/O wait for the leaked-pipe case. func (p *Process) waitDelay() time.Duration { + if p.conf.WaitDelay > 0 { + return p.conf.WaitDelay + } return max(p.conf.SignalGracePeriod, 0) + waitDelayBuffer } diff --git a/internal/process/process_test.go b/internal/process/process_test.go index bf669643ee..c00e8b3314 100644 --- a/internal/process/process_test.go +++ b/internal/process/process_test.go @@ -460,6 +460,8 @@ func TestProcessRunDoesNotHangWhenChildLeaksStdout(t *testing.T) { Env: []string{"TEST_MAIN=leak-stdout"}, Stdout: w, Stderr: w, + // Shorten the post-exit I/O bound (default 30s) so the test stays fast. + WaitDelay: time.Second, }) // Ensure the leaked grandchild is cleaned up regardless of outcome. diff --git a/internal/shell/export_test.go b/internal/shell/export_test.go index 842e921af4..cca03a5c38 100644 --- a/internal/shell/export_test.go +++ b/internal/shell/export_test.go @@ -5,3 +5,9 @@ import "time" func Round(d time.Duration) time.Duration { return round(d) } + +// WithProcessWaitDelay is a test-only shell option that overrides the derived +// process.Config.WaitDelay bound, keeping leaked-pipe regression tests fast. +func WithProcessWaitDelay(d time.Duration) NewShellOpt { + return func(s *Shell) { s.processWaitDelay = d } +} diff --git a/internal/shell/shell.go b/internal/shell/shell.go index 39cc5fed49..a449a63ba6 100644 --- a/internal/shell/shell.go +++ b/internal/shell/shell.go @@ -68,6 +68,10 @@ type Shell struct { // Amount of time to wait between sending the InterruptSignal and SIGKILL signalGracePeriod time.Duration + // Test-only override for the derived process.Config.WaitDelay bound + // (see WithProcessWaitDelay in export_test.go). Zero means "use default". + processWaitDelay time.Duration + // stdin is an optional input stream used by Run() and friends. // It remains unexported on the assumption that it's not useful except via // CloneWithStdin to get a clone prepared for a single command that needs @@ -151,6 +155,7 @@ func (s *Shell) CloneWithStdin(r io.Reader) *Shell { wd: s.wd, interruptSignal: s.interruptSignal, signalGracePeriod: s.signalGracePeriod, + processWaitDelay: s.processWaitDelay, } } @@ -576,6 +581,7 @@ func (s *Shell) buildCommand(name string, arg ...string) (process.Config, error) Dir: s.wd, InterruptSignal: s.interruptSignal, SignalGracePeriod: s.signalGracePeriod, + WaitDelay: s.processWaitDelay, }, nil } diff --git a/internal/shell/shell_test.go b/internal/shell/shell_test.go index 05bec456c8..e4befe1260 100644 --- a/internal/shell/shell_test.go +++ b/internal/shell/shell_test.go @@ -757,6 +757,8 @@ func TestRunDoesNotReportCleanHookAsFailedWhenChildLeaksStdout(t *testing.T) { sh, err := shell.New( shell.WithStdout(w), shell.WithLogger(shell.DiscardLogger), + // Shorten the post-exit I/O bound (default 30s) so the test stays fast. + shell.WithProcessWaitDelay(time.Second), ) if err != nil { t.Fatalf("shell.New() error = %v", err)