test(task): close and drain the pipes the skipped-step CLI test opens

AGENTS.md

@@ -48,6 +48,13 @@ when one is authored, add it here.
 - `cd candy/plugin-task && go test ./...` — the dependency closure, CLI parsing,
   validation, and incremental-semantics tests.
 - `cd candy/plugin-task && gofmt -l .` — formatting (empty output passes).
+- `cd candy/plugin-task && go vet ./...` — the vet pass.
+- `cd candy/plugin-task && GOTOOLCHAIN=go$(go list -m -f '{{.GoVersion}}') golangci-lint run`
+  — the linter, under the toolchain this module's `go.mod` declares: golangci-lint
+  cannot read the export data of a newer GOROOT, so the toolchain must be the
+  module's own, and the linter's version is the one CI pins in
+  `.github/workflows/ci.yml` (one source of truth — read it there). The local gate
+  runs what CI's `go` job runs, so a finding CI would raise is raised locally first.
 - `charly box validate` at the repo root — the structural check (the candy +
   `plugin:` block, CUE schema).
 - `scripts/gate-task.sh` at the repo root — the live gate: it clones charly at

candy/plugin-task/capture_test.go

@@ -0,0 +1,51 @@
+package task
+
+import (
+	"io"
+	"os"
+	"testing"
+)
+
+// captureStreams runs fn with os.Stdout and os.Stderr pointed at pipes and returns
+// what each stream received.
+//
+// ONE shape for the whole package: a test that asserts on what the reporter wrote
+// must not re-implement the capture, because a capture written twice is a capture
+// written once correctly and once wrongly. This one is written correctly:
+//
+//   - BOTH ends of BOTH pipes are kept. os.Pipe returns (read, write); a pipe whose
+//     write end is dropped can never be drained, so the read end would block a later
+//     writer forever and the descriptor would leak.
+//   - The write ends are closed BEFORE the drain. io.ReadAll returns EOF only when
+//     every write end is closed, and a pipe left undrained blocks the next writer.
+//   - The globals go back on every exit path, including a panic in fn — which the
+//     testing package recovers and then runs the NEXT test with. Without the
+//     restore, that test would write its output into a pipe nobody reads.
+//   - The restore happens before the caller asserts, so a failure message is never
+//     written to a closed pipe (and lost).
+func captureStreams(t *testing.T, fn func()) (stdout, stderr string) {
+	t.Helper()
+	outR, outW, err := os.Pipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	errR, errW, err := os.Pipe()
+	if err != nil {
+		_ = outR.Close()
+		_ = outW.Close()
+		t.Fatal(err)
+	}
+	oldOut, oldErr := os.Stdout, os.Stderr
+	os.Stdout, os.Stderr = outW, errW
+	defer func() {
+		os.Stdout, os.Stderr = oldOut, oldErr
+	}()
+	fn()
+	_ = outW.Close()
+	_ = errW.Close()
+	out, _ := io.ReadAll(outR)
+	_ = outR.Close()
+	errOut, _ := io.ReadAll(errR)
+	_ = errR.Close()
+	return string(out), string(errOut)
+}

candy/plugin-task/cli_test.go

@@ -2,8 +2,6 @@ package task
 
 import (
 	"context"
-	"io"
-	"os"
 	"strings"
 	"testing"
 
@@ -44,29 +42,17 @@ func TestRunTaskCLI_OutputMode(t *testing.T) {
 	}
 	defer func() { entityCache = oldCache }()
 
-	oldOut, oldErr := os.Stdout, os.Stderr
-	outR, outW, err := os.Pipe()
-	if err != nil {
-		t.Fatal(err)
-	}
-	errR, errW, err := os.Pipe()
-	if err != nil {
-		t.Fatal(err)
-	}
-	os.Stdout, os.Stderr = outW, errW
-	runErr := runTaskCLI(context.Background(), nil, []string{"capture", "--output"})
-	_ = outW.Close()
-	_ = errW.Close()
-	os.Stdout, os.Stderr = oldOut, oldErr
+	var runErr error
+	stdout, stderr := captureStreams(t, func() {
+		runErr = runTaskCLI(context.Background(), nil, []string{"capture", "--output"})
+	})
 	if runErr != nil {
 		t.Fatalf("runTaskCLI --output: %v", runErr)
 	}
-	stdout, _ := io.ReadAll(outR)
-	stderr, _ := io.ReadAll(errR)
-	if string(stdout) != "CAPTURED-RAW-VALUE\n" {
+	if stdout != "CAPTURED-RAW-VALUE\n" {
 		t.Fatalf("stdout must carry ONLY the raw captured value + newline, got %q", stdout)
 	}
-	if !strings.Contains(string(stderr), "task capture:") {
+	if !strings.Contains(stderr, "task capture:") {
 		t.Fatalf("the human report must be on stderr, got %q", stderr)
 	}
 }

candy/plugin-task/plugin_test.go

@@ -3,7 +3,6 @@ package task
 import (
 	"context"
 	"encoding/json"
-	"io"
 	"os"
 	"path/filepath"
 	"strings"
@@ -396,23 +395,16 @@ func TestFormatStepLine_RendersStepMessage(t *testing.T) {
 // asserts the step message reaches it — the end-to-end arm the pure renderer test
 // cannot cover (a reporter that stopped calling the renderer would pass that one).
 func TestPrintResultText_EmitsStepMessage(t *testing.T) {
-	old := os.Stdout
-	r, w, err := os.Pipe()
-	if err != nil {
-		t.Fatal(err)
-	}
-	os.Stdout = w
-	printResultText(outWriter(), &runResult{
-		Name: "map", Status: "ran",
-		Steps: []spec.StepResult{{
-			Keyword: "run", Text: "print the submodule map",
-			Result: spec.CheckResult{Status: spec.StatusPass, Message: "MAP-TABLE-MARKER"},
-		}},
+	out, _ := captureStreams(t, func() {
+		printResultText(outWriter(), &runResult{
+			Name: "map", Status: "ran",
+			Steps: []spec.StepResult{{
+				Keyword: "run", Text: "print the submodule map",
+				Result: spec.CheckResult{Status: spec.StatusPass, Message: "MAP-TABLE-MARKER"},
+			}},
+		})
 	})
-	_ = w.Close()
-	os.Stdout = old
-	out, _ := io.ReadAll(r)
-	if !strings.Contains(string(out), "MAP-TABLE-MARKER") {
+	if !strings.Contains(out, "MAP-TABLE-MARKER") {
 		t.Fatalf("printResultText must emit the step message, got:\n%s", out)
 	}
 }

candy/plugin-task/skipped_test.go

@@ -4,8 +4,6 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
-	"io"
-	"os"
 	"strings"
 	"testing"
 
@@ -153,31 +151,22 @@ func TestPrintResultsJSON_CarriesTheSkippedCount(t *testing.T) {
 		Skipped: 1,
 		Steps:   []spec.StepResult{{Keyword: "run", Text: "the step that ran nothing", Result: spec.CheckResult{Status: spec.StatusSkip}}},
 	}}
-	oldOut := os.Stdout
-	r, w, err := os.Pipe()
-	if err != nil {
-		t.Fatal(err)
-	}
-	os.Stdout = w
-	jerr := printResultsJSON(results)
-	_ = w.Close()
-	os.Stdout = oldOut
+	var jerr error
+	out, _ := captureStreams(t, func() {
+		jerr = printResultsJSON(results)
+	})
 	if jerr != nil {
 		t.Fatalf("printResultsJSON: %v", jerr)
 	}
-	var buf bytes.Buffer
-	if _, err := buf.ReadFrom(r); err != nil {
-		t.Fatal(err)
-	}
 	var decoded []struct {
 		Failed  int `json:"failed"`
 		Skipped int `json:"skipped"`
 	}
-	if err := json.Unmarshal(buf.Bytes(), &decoded); err != nil {
-		t.Fatalf("the JSON report must decode: %v (%s)", err, buf.String())
+	if err := json.Unmarshal([]byte(out), &decoded); err != nil {
+		t.Fatalf("the JSON report must decode: %v (%s)", err, out)
 	}
 	if len(decoded) != 1 || decoded[0].Skipped != 1 {
-		t.Fatalf("the JSON report must carry the skipped count, got %s", buf.String())
+		t.Fatalf("the JSON report must carry the skipped count, got %s", out)
 	}
 }
 
@@ -200,25 +189,17 @@ func TestRunTaskCLI_SkippedStepExitsZero(t *testing.T) {
 	}
 	defer func() { entityCache = oldCache }()
 
-	oldOut, oldErr := os.Stdout, os.Stderr
-	outR, outW, err := os.Pipe()
-	if err != nil {
-		t.Fatal(err)
-	}
-	errW, _, err := os.Pipe()
-	if err != nil {
-		t.Fatal(err)
-	}
-	os.Stdout, os.Stderr = outW, errW
-	runErr := runTaskCLI(context.Background(), nil, []string{"mixed"})
-	_ = outW.Close()
-	_ = errW.Close()
-	os.Stdout, os.Stderr = oldOut, oldErr
+	var runErr error
+	report, stderrReport := captureStreams(t, func() {
+		runErr = runTaskCLI(context.Background(), nil, []string{"mixed"})
+	})
 	if runErr != nil {
 		t.Fatalf("a task whose only step ran nothing must exit 0, got %v", runErr)
 	}
-	report, _ := io.ReadAll(outR)
-	if !strings.Contains(string(report), "0 failed, 1 skipped") {
+	if !strings.Contains(report, "0 failed, 1 skipped") {
 		t.Fatalf("the report must name the skipped count, got:\n%s", report)
 	}
+	if stderrReport != "" {
+		t.Fatalf("the report belongs on stdout, not stderr, got:\n%s", stderrReport)
+	}
 }