Bound retained Go command diagnostics

This commit is contained in:
2026-11-02 03:31:59 +00:00
parent 2b23574ece
commit 9cb48a70b7
5 changed files with 230 additions and 45 deletions

View File

@@ -11,18 +11,20 @@ errors.
## Command output ownership ## Command output ownership
`Tool.Run` executes a status-only command. It forwards combined stdout and `Tool.Run` executes a status-only command. It forwards combined stdout and
stderr to the optional writer while the child runs, then releases its capture stderr to the optional writer while the child runs, retaining only the last
on success. Failures include the captured diagnostic text and preserve the 1,048,576 bytes for failure diagnostics. A truncation notice identifies omitted
underlying process or forwarding error. earlier bytes. Successful calls release their capture; failures preserve the
underlying process or forwarding error. Dependency preparation uses the same
bounded diagnostic capture.
`Tool.Output` returns combined stdout and stderr for commands whose output is `Tool.Output` returns combined stdout and stderr for commands whose output is
needed by the caller, such as compiler identification. After the child stops, needed by the caller, such as compiler identification. After the child stops,
the capture's backing bytes transfer to the caller without a second copy. A the capture's backing bytes transfer to the caller without a second copy. A
failed output command returns both those bytes and an error containing the failed output command returns both those bytes and an error containing the
diagnostic text. The capture remains in memory for the duration of the child; diagnostic text. The capture remains in memory for the duration of the child;
the component benchmark compares one 8 MiB write with the same output in the component benchmark compares full and bounded capture for modest output,
32 KiB writes, forwarding to `io.Discard`. Smaller writes incur buffer-growth one 8 MiB write, and 8 MiB in 32 KiB writes, forwarding to `io.Discard`.
allocations. The [output benchmark](../../internal/gotool/performance_test.go) The [output benchmark](../../internal/gotool/performance_test.go)
measures total allocated bytes per operation, excluding subprocess overhead and measures total allocated bytes per operation, excluding subprocess overhead and
diagnostic strings. These totals count all allocations, including superseded diagnostic strings. These totals count all allocations, including superseded
buffers. The returned slice retains the final buffer holding the complete output. buffers. The returned slice retains the final buffer holding the complete output.
@@ -35,8 +37,8 @@ lifecycle and cleanup work and remain a follow-up option.
Dependency preparation uses a separate command environment so proxy, private Dependency preparation uses a separate command environment so proxy, private
module, checksum, cache, and other effective settings remain available. Its module, checksum, cache, and other effective settings remain available. Its
`go mod download -modfile=` command overrides an alternate module file selected `go mod download -modfile=` command overrides an alternate module file selected
through `GOFLAGS`, preserving ordinary module discovery. Its completed capture through `GOFLAGS`, preserving ordinary module discovery. Its bounded capture
uses the same ownership transfer when formatting a failure. is formatted into the error on failure.
## Configuration discovery ## Configuration discovery

View File

@@ -49,7 +49,7 @@ the agent policy; an intermediate compilation dependency is recorded accurately.
| 11 | A07: build and delivery outcomes | Complete — managed and native cleanup tests cover success, failure, cancellation, hosted completion, and CLI reporting. | | 11 | A07: build and delivery outcomes | Complete — managed and native cleanup tests cover success, failure, cancellation, hosted completion, and CLI reporting. |
| 12 | A08 | Complete — redirect boundary, origin authorization, cycle, and delivery outcome tests passed. | | 12 | A08 | Complete — redirect boundary, origin authorization, cycle, and delivery outcome tests passed. |
| 13 | A09 | Complete — bounded reads, size and digest mismatches, stream failures, cancellation, reconciliation, and public prebuilt delivery checks passed. | | 13 | A09 | Complete — bounded reads, size and digest mismatches, stream failures, cancellation, reconciliation, and public prebuilt delivery checks passed. |
| 14 | A10 | Pending | | 14 | A10 | Complete — bounded and full capture, forwarding, error identity, concurrency, and benchmark modes passed. |
| 15 | A13 | Pending | | 15 | A13 | Pending |
| 16 | A15 | Pending | | 16 | A15 | Pending |
| 17 | Remaining A19, A20 | Pending | | 17 | Remaining A19, A20 | Pending |

View File

@@ -95,7 +95,7 @@ func New(ctx context.Context, directory string) (*Tool, error) {
// local Go toolchain. It preserves caller module-network settings and caches, // local Go toolchain. It preserves caller module-network settings and caches,
// while disabling workspace discovery and automatic toolchain downloads. // while disabling workspace discovery and automatic toolchain downloads.
func PrepareBuildDependencies(ctx context.Context, directory string) error { func PrepareBuildDependencies(ctx context.Context, directory string) error {
captured := &combinedWriter{} captured := &combinedWriter{mode: captureStatus}
err := process.Run(ctx, process.Invocation{ err := process.Run(ctx, process.Invocation{
Args: []string{"go", "mod", "download", "-modfile="}, Args: []string{"go", "mod", "download", "-modfile="},
Dir: directory, Dir: directory,
@@ -107,7 +107,8 @@ func PrepareBuildDependencies(ctx context.Context, directory string) error {
Stderr: captured, Stderr: captured,
}) })
if err != nil { if err != nil {
if output := strings.TrimSpace(string(captured.take())); output != "" { omitted := captured.omitted()
if output := diagnosticText(captured.take(), omitted); output != "" {
return fmt.Errorf("go mod download: %w: %s", err, output) return fmt.Errorf("go mod download: %w: %s", err, output)
} }
return fmt.Errorf("go mod download: %w", err) return fmt.Errorf("go mod download: %w", err)
@@ -140,7 +141,11 @@ func (tool *Tool) execute(ctx context.Context, directory string, mode Mode, targ
return nil, errors.New("Go command requires arguments") return nil, errors.New("Go command requires arguments")
} }
environment := tool.environment(mode, target) environment := tool.environment(mode, target)
captured := &combinedWriter{output: output} modeCapture := captureStatus
if keepOutput {
modeCapture = captureFull
}
captured := &combinedWriter{output: output, mode: modeCapture}
commandArgs := append([]string{"go"}, args...) commandArgs := append([]string{"go"}, args...)
err := process.Run(ctx, process.Invocation{ err := process.Run(ctx, process.Invocation{
Args: commandArgs, Args: commandArgs,
@@ -152,16 +157,28 @@ func (tool *Tool) execute(ctx context.Context, directory string, mode Mode, targ
if forwardErr := captured.forwardingError(); forwardErr != nil { if forwardErr := captured.forwardingError(); forwardErr != nil {
err = errors.Join(err, fmt.Errorf("forward Go output: %w", forwardErr)) err = errors.Join(err, fmt.Errorf("forward Go output: %w", forwardErr))
} }
if err == nil && !keepOutput {
return nil, nil
}
omitted := captured.omitted()
contents := captured.take() contents := captured.take()
if err != nil { if err != nil {
if len(contents) != 0 { if diagnostic := diagnosticText(contents, omitted); diagnostic != "" {
return outputBytes(contents, keepOutput), fmt.Errorf("go %s: %w: %s", strings.Join(args, " "), err, strings.TrimSpace(string(contents))) return outputBytes(contents, keepOutput), fmt.Errorf("go %s: %w: %s", strings.Join(args, " "), err, diagnostic)
} }
return outputBytes(contents, keepOutput), fmt.Errorf("go %s: %w", strings.Join(args, " "), err) return outputBytes(contents, keepOutput), fmt.Errorf("go %s: %w", strings.Join(args, " "), err)
} }
return outputBytes(contents, keepOutput), nil return outputBytes(contents, keepOutput), nil
} }
func diagnosticText(contents []byte, omitted bool) string {
text := strings.TrimSpace(string(contents))
if omitted {
return "[earlier Go output omitted]\n" + text
}
return text
}
func outputBytes(contents []byte, keep bool) []byte { func outputBytes(contents []byte, keep bool) []byte {
if keep { if keep {
return contents return contents
@@ -232,17 +249,63 @@ type combinedWriter struct {
mu sync.Mutex mu sync.Mutex
output io.Writer output io.Writer
buffer bytes.Buffer buffer bytes.Buffer
mode captureMode
tail []byte
head int
truncated bool
forwardErr error forwardErr error
} }
type captureMode uint8
const (
captureFull captureMode = iota
captureStatus
statusDiagnosticLimit = 1 << 20
)
func (writer *combinedWriter) capture(contents []byte) {
if writer.mode == captureFull {
_, _ = writer.buffer.Write(contents)
return
}
if len(contents) >= statusDiagnosticLimit {
writer.truncated = writer.truncated || len(writer.tail) != 0 || len(contents) > statusDiagnosticLimit
if cap(writer.tail) < statusDiagnosticLimit {
writer.tail = make([]byte, statusDiagnosticLimit)
} else {
writer.tail = writer.tail[:statusDiagnosticLimit]
}
copy(writer.tail, contents[len(contents)-statusDiagnosticLimit:])
writer.head = 0
return
}
if available := statusDiagnosticLimit - len(writer.tail); available > 0 {
n := min(available, len(contents))
if needed := len(writer.tail) + n; needed > cap(writer.tail) {
capacity := min(statusDiagnosticLimit, max(needed, 2*cap(writer.tail)))
grown := make([]byte, len(writer.tail), capacity)
copy(grown, writer.tail)
writer.tail = grown
}
writer.tail = append(writer.tail, contents[:n]...)
contents = contents[n:]
}
if len(contents) == 0 {
return
}
writer.truncated = true
n := copy(writer.tail[writer.head:], contents)
copy(writer.tail, contents[n:])
writer.head = (writer.head + len(contents)) % statusDiagnosticLimit
}
func (writer *combinedWriter) Write(contents []byte) (int, error) { func (writer *combinedWriter) Write(contents []byte) (int, error) {
writer.mu.Lock() writer.mu.Lock()
defer writer.mu.Unlock() defer writer.mu.Unlock()
writer.capture(contents)
if writer.output == nil { if writer.output == nil {
return writer.buffer.Write(contents) return len(contents), nil
}
if _, err := writer.buffer.Write(contents); err != nil {
return 0, err
} }
written, err := writer.output.Write(contents) written, err := writer.output.Write(contents)
if err == nil && written != len(contents) { if err == nil && written != len(contents) {
@@ -254,6 +317,12 @@ func (writer *combinedWriter) Write(contents []byte) (int, error) {
return written, err return written, err
} }
func (writer *combinedWriter) omitted() bool {
writer.mu.Lock()
defer writer.mu.Unlock()
return writer.truncated
}
func (writer *combinedWriter) forwardingError() error { func (writer *combinedWriter) forwardingError() error {
writer.mu.Lock() writer.mu.Lock()
defer writer.mu.Unlock() defer writer.mu.Unlock()
@@ -265,5 +334,18 @@ func (writer *combinedWriter) forwardingError() error {
func (writer *combinedWriter) take() []byte { func (writer *combinedWriter) take() []byte {
writer.mu.Lock() writer.mu.Lock()
defer writer.mu.Unlock() defer writer.mu.Unlock()
return writer.buffer.Bytes() if writer.mode == captureFull {
contents := writer.buffer.Bytes()
writer.buffer = bytes.Buffer{}
return contents
}
contents := writer.tail
if writer.head != 0 {
contents = make([]byte, 0, len(writer.tail))
contents = append(contents, writer.tail[writer.head:]...)
contents = append(contents, writer.tail[:writer.head]...)
}
writer.tail = nil
writer.head = 0
return contents
} }

View File

@@ -6,11 +6,13 @@ import (
"encoding/json" "encoding/json"
"errors" "errors"
"fmt" "fmt"
"io"
"os" "os"
"os/exec" "os/exec"
"path/filepath" "path/filepath"
"runtime" "runtime"
"strings" "strings"
"sync"
"testing" "testing"
"time" "time"
) )
@@ -453,6 +455,96 @@ func TestOutputReturnsLargeFailureDiagnosticsAndCause(t *testing.T) {
} }
} }
func TestStatusCaptureRetainsTailAndForwardsAll(t *testing.T) {
for _, test := range []struct {
name string
size int
chunkSize int
}{
{"below", statusDiagnosticLimit - 1, 8191},
{"exact", statusDiagnosticLimit, 8191},
{"above", statusDiagnosticLimit + 1, 8191},
{"single oversized", statusDiagnosticLimit*2 + 3, statusDiagnosticLimit*2 + 3},
{"wrapped small writes", statusDiagnosticLimit*3 + 17, 113},
} {
t.Run(test.name, func(t *testing.T) {
input := make([]byte, test.size)
for i := range input {
input[i] = byte(i % 251)
}
var forwarded bytes.Buffer
writer := &combinedWriter{mode: captureStatus, output: &forwarded}
for start := 0; start < len(input); start += test.chunkSize {
chunk := input[start:min(start+test.chunkSize, len(input))]
if n, err := writer.Write(chunk); n != len(chunk) || err != nil {
t.Fatalf("write = %d, %v", n, err)
}
}
if writer.omitted() != (len(input) > statusDiagnosticLimit) || cap(writer.tail) > statusDiagnosticLimit {
t.Fatalf("omitted = %t, capacity = %d", writer.omitted(), cap(writer.tail))
}
want := input[max(0, len(input)-statusDiagnosticLimit):]
if got := writer.take(); !bytes.Equal(got, want) || !bytes.Equal(forwarded.Bytes(), input) {
t.Fatalf("capture size = %d, forwarded size = %d", len(got), forwarded.Len())
}
})
}
}
func TestStatusCaptureSerializesConcurrentWriters(t *testing.T) {
var forwarded bytes.Buffer
writer := &combinedWriter{mode: captureStatus, output: &forwarded}
var group sync.WaitGroup
for _, value := range []byte{'a', 'b'} {
group.Add(1)
go func() {
defer group.Done()
chunk := bytes.Repeat([]byte{value}, 4096)
for i := 0; i < 300; i++ {
_, _ = writer.Write(chunk)
}
}()
}
group.Wait()
contents := forwarded.Bytes()
if !writer.omitted() || !bytes.Equal(writer.take(), contents[len(contents)-statusDiagnosticLimit:]) {
t.Fatal("concurrent capture differs from forwarded tail")
}
}
func TestStatusCommandsRetainBoundedFailureDiagnostics(t *testing.T) {
repository, _ := installGoPreparationFixture(t)
t.Setenv("RELEASEKIT_GO_FIXTURE_MODE", "large-fail")
var forwarded bytes.Buffer
err := fixtureTool().Run(context.Background(), repository, Native, Target{}, &forwarded, "version")
var exitError *exec.ExitError
if !errors.As(err, &exitError) || exitError.ExitCode() != 23 || !strings.Contains(err.Error(), "earlier Go output omitted") || !strings.Contains(err.Error(), "fixture large failure") || len(err.Error()) > statusDiagnosticLimit+200 || forwarded.Len() != (8<<20)+len("fixture large failure\n") {
t.Fatalf("Run() error = %v, forwarded = %d bytes", err, forwarded.Len())
}
err = PrepareBuildDependencies(context.Background(), repository)
if !errors.As(err, &exitError) || exitError.ExitCode() != 23 || !strings.Contains(err.Error(), "earlier Go output omitted") || !strings.Contains(err.Error(), "fixture large failure") || len(err.Error()) > statusDiagnosticLimit+200 {
t.Fatalf("preparation error did not retain bounded diagnostics: %v", err)
}
}
func TestStatusCommandPreservesForwardingError(t *testing.T) {
repository, _ := installGoPreparationFixture(t)
t.Setenv("RELEASEKIT_GO_FIXTURE_MODE", "output")
writerErr := errors.New("forwarding failed")
err := fixtureTool().Run(context.Background(), repository, Native, Target{}, failingOutputWriter{err: writerErr}, "version")
if !errors.Is(err, writerErr) {
t.Fatalf("Run() error = %v", err)
}
err = fixtureTool().Run(context.Background(), repository, Native, Target{}, shortOutputWriter{}, "version")
if !errors.Is(err, io.ErrShortWrite) {
t.Fatalf("Run() short-write error = %v", err)
}
}
type shortOutputWriter struct{}
func (shortOutputWriter) Write([]byte) (int, error) { return 0, nil }
func TestOutputReturnsCancellation(t *testing.T) { func TestOutputReturnsCancellation(t *testing.T) {
repository, recordPath := installGoPreparationFixture(t) repository, recordPath := installGoPreparationFixture(t)
t.Setenv("RELEASEKIT_GO_FIXTURE_MODE", "wait") t.Setenv("RELEASEKIT_GO_FIXTURE_MODE", "wait")

View File

@@ -20,36 +20,45 @@ func BenchmarkPerformanceCombinedOutput(b *testing.B) {
} { } {
input := bytes.Repeat([]byte("go tool diagnostic output\n"), (testCase.size+25)/26) input := bytes.Repeat([]byte("go tool diagnostic output\n"), (testCase.size+25)/26)
input = input[:testCase.size] input = input[:testCase.size]
b.Run(testCase.name+"/discard-forwarding", func(b *testing.B) { for _, mode := range []struct {
// Verify capture and forwarding for this workload outside the measurement. name string
var forwarded bytes.Buffer mode captureMode
verification := &combinedWriter{output: &forwarded} }{{"full", captureFull}, {"status", captureStatus}} {
for offset := 0; offset < len(input); offset += testCase.chunkSize { b.Run(testCase.name+"/"+mode.name, func(b *testing.B) {
chunk := input[offset:min(offset+testCase.chunkSize, len(input))] // Verify capture and forwarding for this workload outside the measurement.
if written, err := verification.Write(chunk); err != nil || written != len(chunk) { var forwarded bytes.Buffer
b.Fatalf("verify output write = %d, err %v; want %d bytes", written, err, len(chunk)) verification := &combinedWriter{output: &forwarded, mode: mode.mode}
}
}
if !bytes.Equal(verification.take(), input) || !bytes.Equal(forwarded.Bytes(), input) {
b.Fatal("captured and forwarded output must equal the complete input")
}
b.ReportAllocs()
b.SetBytes(int64(len(input)))
b.ResetTimer()
b.ReportMetric(float64(len(input)), "bytes/op")
for i := 0; i < b.N; i++ {
writer := &combinedWriter{output: io.Discard}
for offset := 0; offset < len(input); offset += testCase.chunkSize { for offset := 0; offset < len(input); offset += testCase.chunkSize {
chunk := input[offset:min(offset+testCase.chunkSize, len(input))] chunk := input[offset:min(offset+testCase.chunkSize, len(input))]
if written, err := writer.Write(chunk); err != nil || written != len(chunk) { if written, err := verification.Write(chunk); err != nil || written != len(chunk) {
b.Fatalf("write captured output = %d, err %v; want %d bytes", written, err, len(chunk)) b.Fatalf("verify output write = %d, err %v; want %d bytes", written, err, len(chunk))
} }
} }
captured := writer.take() want := input
if len(captured) != len(input) { if mode.mode == captureStatus && len(want) > statusDiagnosticLimit {
b.Fatalf("captured output = %d bytes, want %d", len(captured), len(input)) want = want[len(want)-statusDiagnosticLimit:]
} }
} if !bytes.Equal(verification.take(), want) || !bytes.Equal(forwarded.Bytes(), input) {
}) b.Fatal("captured output or forwarding did not match the selected mode")
}
b.ReportAllocs()
b.SetBytes(int64(len(input)))
b.ResetTimer()
b.ReportMetric(float64(len(input)), "bytes/op")
for i := 0; i < b.N; i++ {
writer := &combinedWriter{output: io.Discard, mode: mode.mode}
for offset := 0; offset < len(input); offset += testCase.chunkSize {
chunk := input[offset:min(offset+testCase.chunkSize, len(input))]
if written, err := writer.Write(chunk); err != nil || written != len(chunk) {
b.Fatalf("write captured output = %d, err %v; want %d bytes", written, err, len(chunk))
}
}
captured := writer.take()
if len(captured) != len(want) {
b.Fatalf("captured output = %d bytes, want %d", len(captured), len(want))
}
}
})
}
} }
} }