From 3e5df175e8e13237914564312268f17eeadd2e12 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 21:07:00 -0400 Subject: [PATCH 01/19] feat(ci): local overhead harness and benchmarks (P7-CI-46) --- free/ci/architecture/layers.yaml | 2 + free/ci/architecture/layers_test.go | 3 + free/ci/bench/bench_test.go | 79 ++++++++ free/ci/bench/budget.go | 60 ++++++ free/ci/bench/build.go | 46 +++++ free/ci/bench/cmd/cibench/main.go | 177 ++++++++++++++++++ free/ci/bench/cmd/cibench/main_test.go | 88 +++++++++ free/ci/bench/fixtures.go | 84 +++++++++ free/ci/bench/fixtures_test.go | 79 ++++++++ free/ci/bench/measure.go | 110 +++++++++++ free/ci/bench/ratio-exceptions.json | 4 + free/ci/bench/ratio.go | 106 +++++++++++ free/ci/bench/ratio_test.go | 85 +++++++++ free/ci/bench/result.go | 154 +++++++++++++++ free/ci/bench/result_test.go | 124 ++++++++++++ free/ci/bench/run.go | 161 ++++++++++++++++ free/ci/bench/services.go | 99 ++++++++++ free/ci/bench/spawn.go | 85 +++++++++ .../testdata/medium/.nself/ci.yaml.fixture | 24 +++ free/ci/bench/testdata/medium/cmd/app/main.go | 10 + free/ci/bench/testdata/medium/go.mod.fixture | 3 + .../testdata/medium/internal/alpha/alpha.go | 24 +++ .../medium/internal/alpha/alpha_test.go | 21 +++ .../testdata/medium/internal/beta/beta.go | 24 +++ .../medium/internal/beta/beta_test.go | 21 +++ .../testdata/medium/internal/delta/delta.go | 24 +++ .../medium/internal/delta/delta_test.go | 21 +++ .../medium/internal/epsilon/epsilon.go | 24 +++ .../medium/internal/epsilon/epsilon_test.go | 21 +++ .../bench/testdata/medium/internal/eta/eta.go | 24 +++ .../testdata/medium/internal/eta/eta_test.go | 21 +++ .../testdata/medium/internal/gamma/gamma.go | 24 +++ .../medium/internal/gamma/gamma_test.go | 21 +++ .../testdata/medium/internal/theta/theta.go | 24 +++ .../medium/internal/theta/theta_test.go | 21 +++ .../testdata/medium/internal/zeta/zeta.go | 24 +++ .../medium/internal/zeta/zeta_test.go | 21 +++ free/ci/bench/testdata/medium/package.json | 10 + free/ci/bench/testdata/medium/src/check.mjs | 5 + free/ci/bench/testdata/medium/src/greet.mjs | 1 + .../bench/testdata/medium/test/greet.test.mjs | 7 + .../testdata/tiny/.nself/ci.yaml.fixture | 17 ++ free/ci/bench/testdata/tiny/go.mod.fixture | 3 + free/ci/bench/testdata/tiny/tiny.go | 4 + free/ci/bench/testdata/tiny/tiny_test.go | 9 + free/ci/bench/timeit.go | 10 + .../nodes/provision/single_exec_test.go | 2 + 47 files changed, 2011 insertions(+) create mode 100644 free/ci/bench/bench_test.go create mode 100644 free/ci/bench/budget.go create mode 100644 free/ci/bench/build.go create mode 100644 free/ci/bench/cmd/cibench/main.go create mode 100644 free/ci/bench/cmd/cibench/main_test.go create mode 100644 free/ci/bench/fixtures.go create mode 100644 free/ci/bench/fixtures_test.go create mode 100644 free/ci/bench/measure.go create mode 100644 free/ci/bench/ratio-exceptions.json create mode 100644 free/ci/bench/ratio.go create mode 100644 free/ci/bench/ratio_test.go create mode 100644 free/ci/bench/result.go create mode 100644 free/ci/bench/result_test.go create mode 100644 free/ci/bench/run.go create mode 100644 free/ci/bench/services.go create mode 100644 free/ci/bench/spawn.go create mode 100644 free/ci/bench/testdata/medium/.nself/ci.yaml.fixture create mode 100644 free/ci/bench/testdata/medium/cmd/app/main.go create mode 100644 free/ci/bench/testdata/medium/go.mod.fixture create mode 100644 free/ci/bench/testdata/medium/internal/alpha/alpha.go create mode 100644 free/ci/bench/testdata/medium/internal/alpha/alpha_test.go create mode 100644 free/ci/bench/testdata/medium/internal/beta/beta.go create mode 100644 free/ci/bench/testdata/medium/internal/beta/beta_test.go create mode 100644 free/ci/bench/testdata/medium/internal/delta/delta.go create mode 100644 free/ci/bench/testdata/medium/internal/delta/delta_test.go create mode 100644 free/ci/bench/testdata/medium/internal/epsilon/epsilon.go create mode 100644 free/ci/bench/testdata/medium/internal/epsilon/epsilon_test.go create mode 100644 free/ci/bench/testdata/medium/internal/eta/eta.go create mode 100644 free/ci/bench/testdata/medium/internal/eta/eta_test.go create mode 100644 free/ci/bench/testdata/medium/internal/gamma/gamma.go create mode 100644 free/ci/bench/testdata/medium/internal/gamma/gamma_test.go create mode 100644 free/ci/bench/testdata/medium/internal/theta/theta.go create mode 100644 free/ci/bench/testdata/medium/internal/theta/theta_test.go create mode 100644 free/ci/bench/testdata/medium/internal/zeta/zeta.go create mode 100644 free/ci/bench/testdata/medium/internal/zeta/zeta_test.go create mode 100644 free/ci/bench/testdata/medium/package.json create mode 100644 free/ci/bench/testdata/medium/src/check.mjs create mode 100644 free/ci/bench/testdata/medium/src/greet.mjs create mode 100644 free/ci/bench/testdata/medium/test/greet.test.mjs create mode 100644 free/ci/bench/testdata/tiny/.nself/ci.yaml.fixture create mode 100644 free/ci/bench/testdata/tiny/go.mod.fixture create mode 100644 free/ci/bench/testdata/tiny/tiny.go create mode 100644 free/ci/bench/testdata/tiny/tiny_test.go create mode 100644 free/ci/bench/timeit.go diff --git a/free/ci/architecture/layers.yaml b/free/ci/architecture/layers.yaml index bc7aa2299..ed96f1944 100644 --- a/free/ci/architecture/layers.yaml +++ b/free/ci/architecture/layers.yaml @@ -172,6 +172,8 @@ layers: - tools/deps - internal/storetest - tools/schemagen + - bench + - bench/cmd/cibench - bench/sched - bench/cmd/schedbench - bench/cache diff --git a/free/ci/architecture/layers_test.go b/free/ci/architecture/layers_test.go index dd4e96bbd..7b680970f 100644 --- a/free/ci/architecture/layers_test.go +++ b/free/ci/architecture/layers_test.go @@ -90,6 +90,9 @@ func checkImport(m layerMap, pkg, file, imported string) error { } local := strings.TrimPrefix(imported, module) isLocal := imported != local + if imported == "os/exec" && pkg == "bench" && file == "spawn.go" && kind == "supporting" { + return nil // the timing harness starts the processes it times, outside the engine's funnel (P7-CI-46) + } if imported == "os/exec" && pkg != "internal/exec" && pkg != "internal/legacy" { // These frozen pre-fabric sites are tracked by the single-exec-site test. for _, old := range []struct{ pkg, file string }{{"internal/forgejo", "forgejo.go"}, {"internal/nodes/provision", "exec.go"}, {"internal/serve", "serve_job.go"}, {"internal/serve", "serve_job_report.go"}} { diff --git a/free/ci/bench/bench_test.go b/free/ci/bench/bench_test.go new file mode 100644 index 000000000..e714c405f --- /dev/null +++ b/free/ci/bench/bench_test.go @@ -0,0 +1,79 @@ +package bench + +// Purpose: the Go benchmarks of the in-process scenarios (`go test -bench`). +// They call the same functions cibench times, so a benchmark number and a +// result-JSON number measure the same code. + +import ( + "context" + "testing" +) + +func benchInproc(b *testing.B) *Inproc { + b.Helper() + dir := b.TempDir() + if err := Materialize(context.Background(), Tiny, dir+"/repo"); err != nil { + b.Fatal(err) + } + p, err := PrepareInproc(dir+"/repo", dir+"/scratch") + if err != nil { + b.Fatal(err) + } + return p +} + +func BenchmarkDetect(b *testing.B) { + p := benchInproc(b) + b.ResetTimer() + for i := 0; i < b.N; i++ { + if err := p.Detect(); err != nil { + b.Fatal(err) + } + } +} + +func BenchmarkPlan(b *testing.B) { + p := benchInproc(b) + b.ResetTimer() + for i := 0; i < b.N; i++ { + if err := p.Plan(); err != nil { + b.Fatal(err) + } + } +} + +func BenchmarkStoreOpen(b *testing.B) { + p := benchInproc(b) + b.ResetTimer() + for i := 0; i < b.N; i++ { + if err := p.StoreOpen(); err != nil { + b.Fatal(err) + } + } +} + +func BenchmarkExecOverhead(b *testing.B) { + p := benchInproc(b) + b.ResetTimer() + for i := 0; i < b.N; i++ { + if _, err := p.ExecOverhead(context.Background()); err != nil { + b.Fatal(err) + } + } +} + +func BenchmarkRawSpawn(b *testing.B) { + p := benchInproc(b) + b.ResetTimer() + for i := 0; i < b.N; i++ { + if _, err := p.RawSpawn(context.Background()); err != nil { + b.Fatal(err) + } + } +} + +func BenchmarkCapacityProbe(b *testing.B) { + for i := 0; i < b.N; i++ { + CapacityProbe() + } +} diff --git a/free/ci/bench/budget.go b/free/ci/bench/budget.go new file mode 100644 index 000000000..ed218ebda --- /dev/null +++ b/free/ci/bench/budget.go @@ -0,0 +1,60 @@ +package bench + +// Purpose: the perf-budget.json document (perfbench-budget/v1) and its +// derivation from a recorded baseline. +// Inputs: a baseline Result, a headroom factor, a metric filter. +// Outputs: a Budget whose absolute p95 ceilings are the baseline p95 plus the +// headroom, rounded up to a whole millisecond. +// Constraints: the field names are the ones `perfbench check` decodes with +// DisallowUnknownFields; a metric that is not in the baseline is never given a +// ceiling (an invented number is the one thing this Ticket must not ship). + +import ( + "fmt" + "math" + "sort" +) + +// BudgetSchemaV1 is the value of the "schema" field of the budget file. +const BudgetSchemaV1 = "perfbench-budget/v1" + +// Headroom is the allowed growth over the baseline p95 (Ticket constraint: +25%). +const Headroom = 1.25 + +// Budget is the budget document. P95MS maps a metric name to its p95 ceiling in ms. +type Budget struct { + Note string `json:"_note"` + Schema string `json:"schema"` + Platform string `json:"platform"` + P95MS map[string]float64 `json:"p95_ms"` +} + +// BudgetFromResult derives a budget from a baseline. keep selects the metrics +// that get a ceiling (nil keeps every metric measured in milliseconds). +func BudgetFromResult(base Result, note string, keep func(name string) bool) (Budget, error) { + b := Budget{Note: note, Schema: BudgetSchemaV1, + Platform: fmt.Sprintf("%s/%s", base.GOOS, base.GOARCH), P95MS: map[string]float64{}} + for _, m := range base.Metrics { + if m.Unit != "ms" || (keep != nil && !keep(m.Name)) { + continue + } + if math.IsNaN(m.P95) || m.P95 <= 0 { + return b, fmt.Errorf("metric %s: baseline p95 %v is not a usable timing", m.Name, m.P95) + } + b.P95MS[m.Name] = math.Ceil(m.P95 * Headroom) + } + if len(b.P95MS) == 0 { + return b, fmt.Errorf("baseline has no metric to budget") + } + return b, nil +} + +// Names lists the budgeted metric names, sorted. +func (b Budget) Names() []string { + out := make([]string, 0, len(b.P95MS)) + for n := range b.P95MS { + out = append(out, n) + } + sort.Strings(out) + return out +} diff --git a/free/ci/bench/build.go b/free/ci/bench/build.go new file mode 100644 index 000000000..efeb8d2dd --- /dev/null +++ b/free/ci/bench/build.go @@ -0,0 +1,46 @@ +package bench + +// Purpose: build the binary under test and name the revision measured. +// Inputs: the free/ci module root and an output path. +// Outputs: the binary path; the git HEAD of the tree, with -dirty appended when +// tracked files changed. +// Constraints: CGO_ENABLED=0 and -mod=vendor, as every build of this module. + +import ( + "context" + "fmt" + "strings" +) + +// BuildBinary builds ./cmd of the module at root into out. +func BuildBinary(ctx context.Context, root, out string) (string, error) { + env := append(baseEnv(), "CGO_ENABLED=0") + if _, err := must(ctx, []string{"go", "build", "-mod=vendor", "-o", out, "./cmd"}, root, env); err != nil { + return "", fmt.Errorf("build nself-ci: %w", err) + } + return out, nil +} + +// TreeSHA is the git HEAD of dir, "-dirty" when tracked files are modified, or +// "" when dir is not a checkout. +func TreeSHA(ctx context.Context, dir string) string { + r, err := must(ctx, []string{"git", "rev-parse", "HEAD"}, dir, baseEnv()) + if err != nil { + return "" + } + sha := strings.TrimSpace(r.Tail) + if st, err := must(ctx, []string{"git", "status", "--porcelain", "--untracked-files=no"}, dir, baseEnv()); err == nil && st.Tail != "" { + sha += "-dirty" + } + return sha +} + +// Format renders a result as aligned text lines (stderr summary). +func Format(r Result) string { + var b strings.Builder + fmt.Fprintf(&b, "%s/%s runs=%d sha=%s\n", r.GOOS, r.GOARCH, r.Runs, r.SHA) + for _, m := range r.Metrics { + fmt.Fprintf(&b, "%-24s p50 %8.1f %s p95 %8.1f %s max %8.1f %s n=%d\n", m.Name, m.P50, m.Unit, m.P95, m.Unit, m.Max, m.Unit, m.N) + } + return strings.TrimRight(b.String(), "\n") +} diff --git a/free/ci/bench/cmd/cibench/main.go b/free/ci/bench/cmd/cibench/main.go new file mode 100644 index 000000000..bfbfdf73b --- /dev/null +++ b/free/ci/bench/cmd/cibench/main.go @@ -0,0 +1,177 @@ +// Command cibench is the process-level harness of the V15 ci engine +// (P7-CI-46). It times the nself-ci binary against the frozen v1.4 gate on the +// fixture repos and emits contract:cli.perfbench v1 result JSON. +// +// cibench [-runs 10] [-warmup 2] [-bin path] [-json] [-services [-cold]] +// cibench -ratio-check -in result.json [-exceptions file] +// cibench -write-budget perf-budget.json -in result.json [-only-core] +// +// Run it from the free/ci module root; without -bin it builds ./cmd first. +// Results go to stdout with -json (everything else goes to stderr). Exit 0 on +// success, 1 when a check fails, 2 on a usage or environment error. +package main + +import ( + "context" + "flag" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "time" + + "github.com/nself-org/plugins/free/ci/bench" +) + +func main() { os.Exit(realMain(os.Args[1:], os.Stdout, os.Stderr)) } + +type options struct { + runs, warmup, coldRuns int + bin, nself, sha, in, out string + exceptions, writeBudget, note string + jsonOut, ratio, services bool + cold, skipMedium, onlyCore bool +} + +func realMain(args []string, stdout, stderr io.Writer) int { + var o options + fs := flag.NewFlagSet("cibench", flag.ContinueOnError) + fs.SetOutput(stderr) + fs.IntVar(&o.runs, "runs", 10, "measured iterations") + fs.IntVar(&o.warmup, "warmup", 2, "discarded warm-up iterations") + fs.IntVar(&o.coldRuns, "cold-runs", 0, "cold service samples (default: -runs)") + fs.StringVar(&o.bin, "bin", "", "nself-ci binary (default: build ./cmd)") + fs.StringVar(&o.nself, "nself", "nself", "nself CLI for -services") + fs.StringVar(&o.sha, "sha", "", "revision measured (default: git HEAD of the working directory)") + fs.StringVar(&o.in, "in", "", "perfbench/v1 result file (-ratio-check, -write-budget)") + fs.StringVar(&o.out, "out", "", "write the result JSON here instead of stdout") + fs.StringVar(&o.exceptions, "exceptions", "bench/ratio-exceptions.json", "owner-accepted ratio exceptions") + fs.StringVar(&o.writeBudget, "write-budget", "", "derive a budget file from -in and write it here") + fs.StringVar(&o.note, "note", "", "_note text of a derived budget") + fs.BoolVar(&o.jsonOut, "json", false, "print the result JSON to stdout") + fs.BoolVar(&o.ratio, "ratio-check", false, "assert the V15/v1.4 relative budget on -in") + fs.BoolVar(&o.services, "services", false, "also measure service start (needs Docker and a v1.5 nself CLI)") + fs.BoolVar(&o.cold, "cold", false, "with -services: also measure the cold start (removes the project's images)") + fs.BoolVar(&o.skipMedium, "skip-medium", false, "skip the medium fixture (needs node and npm)") + fs.BoolVar(&o.onlyCore, "only-core", false, "with -write-budget: leave service metrics out") + if err := fs.Parse(args); err != nil { + return 2 + } + switch { + case o.ratio: + return ratioCmd(o, stdout, stderr) + case o.writeBudget != "": + return budgetCmd(o, stderr) + } + return runCmd(o, stdout, stderr) +} + +func say(w io.Writer, format string, a ...any) { _, _ = fmt.Fprintf(w, format+"\n", a...) } + +// ratioCmd implements -ratio-check. +func ratioCmd(o options, stdout, stderr io.Writer) int { + if o.in == "" { + say(stderr, "cibench: -ratio-check needs -in") + return 2 + } + res, err := bench.ReadResult(o.in) + if err != nil { + say(stderr, "cibench: %v", err) + return 2 + } + ex, err := bench.LoadExceptions(o.exceptions) + if err != nil { + say(stderr, "cibench: %v", err) + return 2 + } + violations, passes := bench.RatioCheck(res, ex, time.Now()) + for _, p := range passes { + say(stdout, "%s", p) + } + if len(violations) > 0 { + say(stderr, "cibench: FAIL\n%s", strings.Join(violations, "\n")) + return 1 + } + return 0 +} + +// budgetCmd implements -write-budget. +func budgetCmd(o options, stderr io.Writer) int { + if o.in == "" { + say(stderr, "cibench: -write-budget needs -in") + return 2 + } + res, err := bench.ReadResult(o.in) + if err != nil { + say(stderr, "cibench: %v", err) + return 2 + } + keep := func(name string) bool { + return !(o.onlyCore && strings.HasPrefix(name, "ci.service_")) && !informational[name] + } + b, err := bench.BudgetFromResult(res, o.note, keep) + if err != nil { + say(stderr, "cibench: %v", err) + return 2 + } + raw, err := bench.Marshal(b) + if err == nil { + err = os.WriteFile(o.writeBudget, raw, 0o644) + } + if err != nil { + say(stderr, "cibench: %v", err) + return 2 + } + return 0 +} + +// informational metrics are recorded to read the others against; they get no ceiling. +var informational = map[string]bool{bench.MetricExecBaseline: true, bench.MetricCapacityProbe: true} + +// runCmd measures and prints the result. +func runCmd(o options, stdout, stderr io.Writer) int { + ctx, stop := context.WithCancel(context.Background()) + defer stop() + dir, err := os.MkdirTemp(os.Getenv("TMPDIR"), "cibench-") + if err != nil { + say(stderr, "cibench: %v", err) + return 2 + } + defer func() { _ = os.RemoveAll(dir) }() + if o.bin == "" { + if o.bin, err = bench.BuildBinary(ctx, ".", filepath.Join(dir, "nself-ci")); err != nil { + say(stderr, "cibench: %v", err) + return 2 + } + } + if o.sha == "" { + o.sha = bench.TreeSHA(ctx, ".") + } + cfg := bench.Config{Bin: o.bin, Runs: o.runs, Warmup: o.warmup, Dir: dir, SHA: o.sha, + SkipMedium: o.skipMedium, Log: stderr} + if o.services { + cfg.Services = &bench.ServiceConfig{Nself: o.nself, Cold: o.cold, ColdRuns: o.coldRuns} + } + res, err := bench.Run(ctx, cfg) + if err != nil { + say(stderr, "cibench: %v", err) + return 2 + } + raw, err := bench.Marshal(res) + if err != nil { + say(stderr, "cibench: %v", err) + return 2 + } + if o.out != "" { + if err := os.WriteFile(o.out, raw, 0o644); err != nil { + say(stderr, "cibench: %v", err) + return 2 + } + } + if o.jsonOut { + _, _ = stdout.Write(raw) + } + say(stderr, "%s", bench.Format(res)) + return 0 +} diff --git a/free/ci/bench/cmd/cibench/main_test.go b/free/ci/bench/cmd/cibench/main_test.go new file mode 100644 index 000000000..2b95bb49b --- /dev/null +++ b/free/ci/bench/cmd/cibench/main_test.go @@ -0,0 +1,88 @@ +package main + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/nself-org/plugins/free/ci/bench" +) + +func writeResult(t *testing.T, dir string, v14, v15 float64) string { + t.Helper() + r := bench.NewResult("abc", 10, []bench.Metric{ + {Name: bench.MetricTinyV14, Unit: "ms", P50: v14, P95: v14, Max: v14, N: 10}, + {Name: bench.MetricTinyV15, Unit: "ms", P50: v15, P95: v15, Max: v15, N: 10}, + {Name: bench.MetricExecBaseline, Unit: "ms", P50: 2, P95: 3, Max: 3, N: 10}, + {Name: bench.MetricServiceWarm, Unit: "ms", P50: 900, P95: 1000, Max: 1000, N: 10}, + }) + raw, err := bench.Marshal(r) + if err != nil { + t.Fatal(err) + } + path := filepath.Join(dir, "result.json") + if err := os.WriteFile(path, raw, 0o644); err != nil { + t.Fatal(err) + } + return path +} + +func TestRatioCheckExitCodes(t *testing.T) { + dir := t.TempDir() + ex := filepath.Join(dir, "none.json") + for _, c := range []struct { + name string + v14 float64 + v15 float64 + args []string + want int + wantOut string + }{ + {"pass", 300, 500, nil, 0, "median 500.0 ms"}, + {"fail", 300, 5000, nil, 1, ""}, + {"no input", 0, 0, []string{"-ratio-check"}, 2, ""}, + } { + in := writeResult(t, dir, c.v14, c.v15) + args := c.args + if args == nil { + args = []string{"-ratio-check", "-in", in, "-exceptions", ex} + } + var out, errw bytes.Buffer + if got := realMain(args, &out, &errw); got != c.want || !strings.Contains(out.String(), c.wantOut) { + t.Errorf("%s: exit %d, stdout %q, stderr %q", c.name, got, out.String(), errw.String()) + } + } +} + +func TestWriteBudget(t *testing.T) { + dir := t.TempDir() + in := writeResult(t, dir, 300, 500) + out := filepath.Join(dir, "budget.json") + var so, se bytes.Buffer + if got := realMain([]string{"-write-budget", out, "-in", in, "-only-core", "-note", "n"}, &so, &se); got != 0 { + t.Fatalf("exit %d: %s", got, se.String()) + } + raw, err := os.ReadFile(out) + if err != nil { + t.Fatal(err) + } + s := string(raw) + if !strings.Contains(s, `"perfbench-budget/v1"`) || !strings.Contains(s, `"ci.e2e_tiny.v15": 625`) { + t.Fatalf("budget:\n%s", s) + } + if strings.Contains(s, "service_start") || strings.Contains(s, "exec_baseline") { + t.Fatalf("service and informational metrics must get no ceiling:\n%s", s) + } + if got := realMain([]string{"-write-budget", out}, &so, &se); got != 2 { + t.Fatalf("missing -in must be a usage error, got %d", got) + } +} + +func TestBadFlag(t *testing.T) { + var so, se bytes.Buffer + if got := realMain([]string{"-nope"}, &so, &se); got != 2 { + t.Fatalf("exit %d", got) + } +} diff --git a/free/ci/bench/fixtures.go b/free/ci/bench/fixtures.go new file mode 100644 index 000000000..0f65b0c1b --- /dev/null +++ b/free/ci/bench/fixtures.go @@ -0,0 +1,84 @@ +package bench + +// Purpose: the two fixture repositories the end-to-end scenarios run on: a +// tiny Go module and a medium Go plus Node repo (bench/testdata). +// Inputs: the embedded fixture trees and a destination directory. +// Outputs: a committed git repository at the destination, with the V15 config +// (.nself/ci.yaml) naming exactly the checks the v1.4 gate runs on that repo. +// Constraints: files named *.fixture are stored with that suffix (embed refuses +// a go.mod, and a ci.yaml would be picked up by this repo's own gate) and are +// written without it; the git identity is fixed so the commit is hermetic. + +import ( + "context" + "embed" + "fmt" + "io/fs" + "os" + "path" + "path/filepath" + "strings" +) + +//go:embed all:testdata/tiny all:testdata/medium +var fixtureFS embed.FS + +// Fixture names. +const ( + Tiny = "tiny" + Medium = "medium" +) + +// gitEnv makes git ignore the operator's configuration. +func gitEnv() []string { + return append(baseEnv(), "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null", + "GIT_AUTHOR_NAME=cibench", "GIT_AUTHOR_EMAIL=cibench@example.invalid", + "GIT_COMMITTER_NAME=cibench", "GIT_COMMITTER_EMAIL=cibench@example.invalid") +} + +// baseEnv is the part of the operator's environment every child needs. +func baseEnv() []string { + var env []string + for _, k := range []string{"PATH", "HOME", "USER", "LANG", "TMPDIR", "GOCACHE", "GOMODCACHE", "GOPATH", + "GOROOT", "GOFLAGS", "GOTOOLCHAIN", "GOPROXY", "GONOSUMDB", "GOSUMDB", "GOPRIVATE", "NVM_DIR", "PNPM_HOME", + "DOCKER_HOST", "DOCKER_CONTEXT", "DOCKER_CONFIG", "COLIMA_HOME", "XDG_CONFIG_HOME", "XDG_CACHE_HOME", "XDG_RUNTIME_DIR"} { + if v, ok := os.LookupEnv(k); ok { + env = append(env, k+"="+v) + } + } + return env +} + +// Materialize writes fixture name under dst and commits it. +func Materialize(ctx context.Context, name, dst string) error { + root := path.Join("testdata", name) + err := fs.WalkDir(fixtureFS, root, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + rel := strings.TrimPrefix(strings.TrimPrefix(p, root), "/") + if rel == "" { + return os.MkdirAll(dst, 0o755) + } + target := filepath.Join(dst, strings.TrimSuffix(filepath.FromSlash(rel), ".fixture")) + if d.IsDir() { + return os.MkdirAll(target, 0o755) + } + data, err := fixtureFS.ReadFile(p) + if err != nil { + return err + } + return os.WriteFile(target, data, 0o644) + }) + if err != nil { + return fmt.Errorf("fixture %s: %w", name, err) + } + env := gitEnv() + for _, args := range [][]string{{"init", "-q", "-b", "main"}, {"add", "-A"}, + {"-c", "commit.gpgsign=false", "commit", "-q", "-m", "fixture"}} { + if _, err := must(ctx, append([]string{"git"}, args...), dst, env); err != nil { + return err + } + } + return nil +} diff --git a/free/ci/bench/fixtures_test.go b/free/ci/bench/fixtures_test.go new file mode 100644 index 000000000..244ad871c --- /dev/null +++ b/free/ci/bench/fixtures_test.go @@ -0,0 +1,79 @@ +package bench + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/nself-org/plugins/free/ci/internal/config" +) + +func TestMaterialize(t *testing.T) { + for _, name := range []string{Tiny, Medium} { + dir := filepath.Join(t.TempDir(), name) + if err := Materialize(context.Background(), name, dir); err != nil { + t.Fatalf("%s: %v", name, err) + } + for _, f := range []string{"go.mod", config.FileName, ".git/HEAD"} { + if _, err := os.Stat(filepath.Join(dir, f)); err != nil { + t.Errorf("%s: %v", name, err) + } + } + if _, err := os.Stat(filepath.Join(dir, "go.mod.fixture")); err == nil { + t.Errorf("%s: the .fixture suffix was kept", name) + } + loaded, err := config.Load(filepath.Join(dir, config.FileName)) + if err != nil { + t.Fatalf("%s: V15 config does not load: %v", name, err) + } + if got := loaded.Config.Pipelines["gate"].Jobs; len(got) < 3 { + t.Errorf("%s: gate pipeline lists %v", name, got) + } + r, err := must(context.Background(), []string{"git", "log", "--oneline"}, dir, gitEnv()) + if err != nil || r.Tail == "" { + t.Errorf("%s: no commit: %v", name, err) + } + } +} + +func TestInprocScenariosRun(t *testing.T) { + dir := t.TempDir() + if err := Materialize(context.Background(), Tiny, dir+"/repo"); err != nil { + t.Fatal(err) + } + p, err := PrepareInproc(dir+"/repo", dir+"/scratch") + if err != nil { + t.Fatal(err) + } + for name, fn := range map[string]func() error{"detect": p.Detect, "plan": p.Plan, "store": p.StoreOpen} { + if err := fn(); err != nil { + t.Errorf("%s: %v", name, err) + } + } + if len(p.facts) == 0 { + t.Error("detect found no facts in a Go module") + } + if _, err := p.ExecOverhead(context.Background()); err != nil { + t.Errorf("exec overhead: %v", err) + } + if r, err := p.RawSpawn(context.Background()); err != nil || r.Wall <= 0 { + t.Errorf("raw spawn: %+v %v", r, err) + } +} + +func TestSpawnReportsExitCode(t *testing.T) { + r, err := spawn(context.Background(), []string{"sh", "-c", "echo boom; exit 3"}, "", baseEnv()) + if err != nil || r.ExitCode != 3 || r.Tail != "boom" || r.Wall <= 0 { + t.Fatalf("%+v %v", r, err) + } + if _, err := must(context.Background(), []string{"sh", "-c", "exit 1"}, "", baseEnv()); err == nil { + t.Fatal("must accepted exit 1") + } + if _, err := spawn(context.Background(), []string{"/nonexistent/program"}, "", nil); err == nil { + t.Fatal("a program that cannot start must be an error") + } + if _, err := spawn(context.Background(), nil, "", nil); err == nil { + t.Fatal("empty argv accepted") + } +} diff --git a/free/ci/bench/measure.go b/free/ci/bench/measure.go new file mode 100644 index 000000000..3b0090dc5 --- /dev/null +++ b/free/ci/bench/measure.go @@ -0,0 +1,110 @@ +package bench + +// Purpose: the in-process scenarios: detect, plan, store open and migrate, the +// per-job executor overhead and the host capacity probe. Both the Go +// benchmarks (bench_test.go) and the cibench harness call these, so a number in +// the result JSON and a `go test -bench` number measure the same code. +// Inputs: a prepared fixture directory. +// Outputs: one function per scenario that does a single iteration. +// Constraints: no scenario reads the operator's home or state.db; every +// iteration starts from the same inputs (the store always opens a fresh file). + +import ( + "context" + "fmt" + "os" + "path/filepath" + "runtime" + "sync/atomic" + + "github.com/nself-org/plugins/free/ci/internal/config" + "github.com/nself-org/plugins/free/ci/internal/detect" + ciexec "github.com/nself-org/plugins/free/ci/internal/exec" + "github.com/nself-org/plugins/free/ci/internal/gates" + "github.com/nself-org/plugins/free/ci/internal/model" + "github.com/nself-org/plugins/free/ci/internal/plan" + "github.com/nself-org/plugins/free/ci/internal/store" +) + +// Inproc holds the inputs the in-process scenarios share. +type Inproc struct { + Root string // fixture repository + Home string // scratch directory for stores and executor workspaces + cfg model.PipelineConfig + facts []detect.Fact + world plan.World + seq atomic.Uint64 +} + +// PrepareInproc loads the fixture's V15 config and detects its facts once. +func PrepareInproc(root, home string) (*Inproc, error) { + loaded, err := config.Load(filepath.Join(root, config.FileName)) + if err != nil { + return nil, fmt.Errorf("fixture config: %w", err) + } + p := &Inproc{Root: root, Home: home, cfg: loaded.Config} + if err := p.Detect(); err != nil { + return nil, err + } + p.world = plan.World{Platform: runtime.GOOS + "/" + runtime.GOARCH, Tools: []string{"go", "gofmt", "npm"}, + CPU: runtime.NumCPU(), MemMB: 8192} + return p, nil +} + +// Detect is one detection pass over the fixture (ci.detect). +func (p *Inproc) Detect() error { + facts, err := detect.DefaultRegistry().Detect(os.DirFS(p.Root), gates.DefaultRegistry().Has) + if err != nil { + return err + } + p.facts = facts + return nil +} + +// Plan is one planner pass over the detected facts (ci.plan). The planner is +// pure, so the revision and world are fixed values. +func (p *Inproc) Plan() error { + _, err := plan.Build(plan.Input{Config: p.cfg, Facts: p.facts, World: p.world, + Revision: plan.Revision{Branch: "main", DefaultBranch: true}, Pipeline: "gate"}) + return err +} + +// StoreOpen opens and migrates a fresh state.db, then closes it (ci.store_open). +func (p *Inproc) StoreOpen() error { + dir := filepath.Join(p.Home, fmt.Sprintf("store-%d", p.seq.Add(1))) + if err := os.MkdirAll(dir, 0o700); err != nil { + return err + } + defer func() { _ = os.RemoveAll(dir) }() + st, err := store.Open(filepath.Join(dir, "state.db"), store.Options{}) + if err != nil { + return err + } + return st.Close() +} + +// ExecOverhead runs one no-op job through the executor funnel and returns the +// funnel's wall time for it (ci.exec_overhead): workspace, environment, process +// start, process-tree tracking and cleanup, with a job that does no work. +func (p *Inproc) ExecOverhead(ctx context.Context) (Timing, error) { + argv := []string{"true"} + n := p.seq.Add(1) + ex := &ciexec.Executor{Env: ciexec.AllowlistEnv{}} + res := ex.Run(ctx, ciexec.JobSpec{AttemptID: fmt.Sprintf("bench-%d", n), CoordinatorID: "cibench", + Job: model.Job{Run: argv}, Command: argv, Home: filepath.Join(p.Home, "exec")}) + if res.Err != nil || res.ExitCode != 0 { + return Timing{}, fmt.Errorf("executor no-op job: exit %d: %v", res.ExitCode, res.Err) + } + return Timing{Wall: res.Finished.Sub(res.Started)}, nil +} + +// RawSpawn starts the same no-op program directly, with no funnel +// (ci.exec_baseline): the floor ExecOverhead is read against. +func (p *Inproc) RawSpawn(ctx context.Context) (Timing, error) { + r, err := must(ctx, []string{"true"}, p.Root, baseEnv()) + return r, err +} + +// CapacityProbe reads the host quotas the way `nself ci` does before a run +// (ci.capacity_probe). +func CapacityProbe() ciexec.Capacity { return ciexec.Probe() } diff --git a/free/ci/bench/ratio-exceptions.json b/free/ci/bench/ratio-exceptions.json new file mode 100644 index 000000000..fa9e7ce6b --- /dev/null +++ b/free/ci/bench/ratio-exceptions.json @@ -0,0 +1,4 @@ +{ + "schema": "cibench-ratio-exceptions/v1", + "exceptions": [] +} diff --git a/free/ci/bench/ratio.go b/free/ci/bench/ratio.go new file mode 100644 index 000000000..000a188b4 --- /dev/null +++ b/free/ci/bench/ratio.go @@ -0,0 +1,106 @@ +package bench + +// Purpose: the relative budget of the local fast path (Ticket constraint): +// ci.e2e_tiny.v15 median <= ci.e2e_tiny.v14 median * 1.10 + 250 ms. +// Inputs: a Result carrying both e2e metrics, the recorded exceptions, a clock. +// Outputs: one line per violated or passed assertion. +// Constraints: a miss fails unless the owner accepted a written exception for +// this platform (cause, gap, expiry) and it has not expired; an exception +// without accepted_by is a request, not an acceptance, and does not count. The +// medium fixture is recorded but not asserted (the Ticket budgets tiny only). + +import ( + "encoding/json" + "fmt" + "os" + "time" +) + +// Relative budget terms, from the Ticket. +const ( + RatioFactor = 1.10 + RatioSlackMS = 250.0 +) + +// Metric names of the end-to-end comparison. +const ( + MetricTinyV14 = "ci.e2e_tiny.v14" + MetricTinyV15 = "ci.e2e_tiny.v15" +) + +// ExceptionsSchemaV1 is the value of the "schema" field of the exceptions file. +const ExceptionsSchemaV1 = "cibench-ratio-exceptions/v1" + +// Exception is one owner-accepted miss of the relative budget. +type Exception struct { + Platform string `json:"platform"` // goos/goarch + Cause string `json:"cause"` + GapMS float64 `json:"gap_ms"` // the largest accepted excess over the budget + Expires string `json:"expires"` // YYYY-MM-DD, inclusive + AcceptedBy string `json:"accepted_by"` + AcceptedOn string `json:"accepted_on"` +} + +// Exceptions is the exceptions file. +type Exceptions struct { + Schema string `json:"schema"` + Exceptions []Exception `json:"exceptions"` +} + +// LoadExceptions reads the exceptions file; a missing file is no exceptions. +func LoadExceptions(path string) (Exceptions, error) { + var ex Exceptions + raw, err := os.ReadFile(path) + if os.IsNotExist(err) { + return Exceptions{Schema: ExceptionsSchemaV1}, nil + } + if err != nil { + return ex, err + } + if err := json.Unmarshal(raw, &ex); err != nil { + return ex, fmt.Errorf("exceptions %s: %w", path, err) + } + if ex.Schema != ExceptionsSchemaV1 { + return ex, fmt.Errorf("exceptions %s: schema %q, want %q", path, ex.Schema, ExceptionsSchemaV1) + } + return ex, nil +} + +// covers reports whether an accepted, unexpired exception admits a miss of gap +// ms on platform at time now. +func (ex Exceptions) covers(platform string, gap float64, now time.Time) (Exception, bool) { + for _, e := range ex.Exceptions { + until, err := time.Parse("2006-01-02", e.Expires) + if e.Platform != platform || e.AcceptedBy == "" || err != nil || gap > e.GapMS { + continue + } + if now.Before(until.Add(24 * time.Hour)) { + return e, true + } + } + return Exception{}, false +} + +// RatioLimit is the V15 median the relative budget allows for a v1.4 median. +func RatioLimit(v14 float64) float64 { return v14*RatioFactor + RatioSlackMS } + +// RatioCheck asserts the relative budget on res. It fails closed: a missing +// metric is a violation. +func RatioCheck(res Result, ex Exceptions, now time.Time) (violations, passes []string) { + old, ok14 := res.Find(MetricTinyV14) + cur, ok15 := res.Find(MetricTinyV15) + if !ok14 || !ok15 { + return []string{MetricTinyV14 + " and " + MetricTinyV15 + " must both be in the result"}, nil + } + limit := RatioLimit(old.P50) + line := fmt.Sprintf("%s median %.1f ms vs limit %.1f ms (%s median %.1f ms x %.2f + %.0f ms)", + MetricTinyV15, cur.P50, limit, MetricTinyV14, old.P50, RatioFactor, RatioSlackMS) + if cur.P50 <= limit { + return nil, []string{line} + } + platform := res.GOOS + "/" + res.GOARCH + if e, ok := ex.covers(platform, cur.P50-limit, now); ok { + return nil, []string{line + fmt.Sprintf("; MISS accepted by %s on %s until %s: %s", e.AcceptedBy, e.AcceptedOn, e.Expires, e.Cause)} + } + return []string{line + fmt.Sprintf("; over by %.1f ms and no accepted exception for %s", cur.P50-limit, platform)}, nil +} diff --git a/free/ci/bench/ratio_test.go b/free/ci/bench/ratio_test.go new file mode 100644 index 000000000..7f003a037 --- /dev/null +++ b/free/ci/bench/ratio_test.go @@ -0,0 +1,85 @@ +package bench + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" +) + +func tinyResult(v14, v15 float64) Result { + r := NewResult("", 10, []Metric{ + {Name: MetricTinyV14, Unit: "ms", P50: v14, P95: v14, Max: v14, N: 10}, + {Name: MetricTinyV15, Unit: "ms", P50: v15, P95: v15, Max: v15, N: 10}, + }) + r.GOOS, r.GOARCH = "linux", "amd64" + return r +} + +func TestRatioLimit(t *testing.T) { + if got := RatioLimit(1000); got != 1350 { + t.Fatalf("limit %v, want 1000 x 1.10 + 250", got) + } +} + +func TestRatioCheck(t *testing.T) { + now := time.Date(2026, 10, 18, 12, 0, 0, 0, time.UTC) + accepted := Exception{Platform: "linux/amd64", Cause: "ps polling", GapMS: 800, Expires: "2026-11-18", AcceptedBy: "owner", AcceptedOn: "2026-10-18"} + cases := []struct { + name string + res Result + ex []Exception + wantFail bool + wantText string + }{ + {"within budget", tinyResult(300, 580), nil, false, "median 580.0 ms vs limit 580.0 ms"}, + {"over budget", tinyResult(300, 581), nil, true, "no accepted exception for linux/amd64"}, + {"accepted exception", tinyResult(300, 1100), []Exception{accepted}, false, "MISS accepted by owner"}, + {"request without acceptance", tinyResult(300, 1100), []Exception{{Platform: "linux/amd64", GapMS: 800, Expires: "2026-11-18"}}, true, "no accepted exception"}, + {"expired", tinyResult(300, 1100), []Exception{{Platform: "linux/amd64", GapMS: 800, Expires: "2026-10-17", AcceptedBy: "owner"}}, true, "no accepted exception"}, + {"last day counts", tinyResult(300, 1100), []Exception{{Platform: "linux/amd64", GapMS: 800, Expires: "2026-10-18", AcceptedBy: "owner"}}, false, "MISS accepted"}, + {"other platform", tinyResult(300, 1100), []Exception{{Platform: "darwin/arm64", GapMS: 800, Expires: "2026-11-18", AcceptedBy: "owner"}}, true, "no accepted exception"}, + {"gap larger than accepted", tinyResult(300, 2000), []Exception{accepted}, true, "no accepted exception"}, + {"missing metric", NewResult("", 1, nil), nil, true, "must both be in the result"}, + } + for _, c := range cases { + violations, passes := RatioCheck(c.res, Exceptions{Exceptions: c.ex}, now) + out := strings.Join(append(violations, passes...), "\n") + if (len(violations) > 0) != c.wantFail || !strings.Contains(out, c.wantText) { + t.Errorf("%s: fail=%v out=%q", c.name, len(violations) > 0, out) + } + } +} + +func TestLoadExceptions(t *testing.T) { + dir := t.TempDir() + if ex, err := LoadExceptions(filepath.Join(dir, "none.json")); err != nil || len(ex.Exceptions) != 0 { + t.Fatalf("a missing file is no exceptions: %+v %v", ex, err) + } + path := filepath.Join(dir, "ex.json") + write := func(s string) { + if err := os.WriteFile(path, []byte(s), 0o644); err != nil { + t.Fatal(err) + } + } + write(`{"schema":"cibench-ratio-exceptions/v1","exceptions":[{"platform":"linux/amd64","gap_ms":5,"expires":"2026-12-01","accepted_by":"owner"}]}`) + if ex, err := LoadExceptions(path); err != nil || len(ex.Exceptions) != 1 { + t.Fatalf("%+v %v", ex, err) + } + write(`{"schema":"x","exceptions":[]}`) + if _, err := LoadExceptions(path); err == nil { + t.Fatal("wrong schema accepted") + } + write(`{`) + if _, err := LoadExceptions(path); err == nil { + t.Fatal("broken JSON accepted") + } +} + +// TestShippedExceptionsFile keeps the file the workflow reads valid. +func TestShippedExceptionsFile(t *testing.T) { + if _, err := LoadExceptions("ratio-exceptions.json"); err != nil { + t.Fatal(err) + } +} diff --git a/free/ci/bench/result.go b/free/ci/bench/result.go new file mode 100644 index 000000000..336d8ea8a --- /dev/null +++ b/free/ci/bench/result.go @@ -0,0 +1,154 @@ +// Package bench measures the local overhead of the V15 ci engine against the +// frozen v1.4 gate (P7-CI-46). +// +// Purpose: produce contract:cli.perfbench v1 result JSON that the cli's own +// `perfbench check` accepts, set the local fast-path budget from a recorded +// baseline, and assert the V15/v1.4 wall-time ratio. +// Inputs: the nself-ci binary under test, fixture repos (testdata), the host. +// Outputs: Result documents (perfbench/v1), Budget documents +// (perfbench-budget/v1) and the ratio verdict. +// Constraints: the JSON field order and the nearest-rank percentile match +// cli:tools/perfbench exactly, because `perfbench check` is the one gate that +// reads these files; scenarios stay in this module (perfbench scenarios are cli +// code and the plugin cannot register one). +package bench + +import ( + "bytes" + "encoding/json" + "fmt" + "math" + "os" + "runtime" + "sort" +) + +// SchemaV1 is the value of the "schema" field of every result document. +const SchemaV1 = "perfbench/v1" + +// Metric is one summarized measurement. Field order is the JSON key order. +type Metric struct { + Name string `json:"name"` + Unit string `json:"unit"` + P50 float64 `json:"p50"` + P95 float64 `json:"p95"` + Max float64 `json:"max"` + N int `json:"n"` +} + +// Result is the perfbench/v1 document. Field order is the JSON key order. +type Result struct { + Schema string `json:"schema"` + Scenario string `json:"scenario"` + SHA string `json:"sha"` + GOOS string `json:"goos"` + GOARCH string `json:"goarch"` + Runs int `json:"runs"` + Injected bool `json:"injected"` + Metrics []Metric `json:"metrics"` +} + +// Scenario is the scenario name of every result this package emits. +const Scenario = "ci-local-overhead" + +// NewResult fills the environment fields of a result. sha names what was +// measured (the plugin revision); it may be empty when unknown. +func NewResult(sha string, runs int, metrics []Metric) Result { + return Result{Schema: SchemaV1, Scenario: Scenario, SHA: sha, GOOS: runtime.GOOS, + GOARCH: runtime.GOARCH, Runs: runs, Metrics: metrics} +} + +// Find returns the metric called name. +func (r Result) Find(name string) (Metric, bool) { + for _, m := range r.Metrics { + if m.Name == name { + return m, true + } + } + return Metric{}, false +} + +// Marshal renders v as 2-space-indented JSON with a trailing newline. +func Marshal(v any) ([]byte, error) { + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + enc.SetIndent("", " ") + if err := enc.Encode(v); err != nil { + return nil, err + } + return buf.Bytes(), nil +} + +// ReadResult loads and validates a perfbench/v1 document. +func ReadResult(path string) (Result, error) { + var r Result + raw, err := os.ReadFile(path) + if err != nil { + return r, err + } + if err := json.Unmarshal(raw, &r); err != nil { + return r, fmt.Errorf("result %s: %w", path, err) + } + if r.Schema != SchemaV1 { + return r, fmt.Errorf("result %s: schema %q, want %q", path, r.Schema, SchemaV1) + } + return r, nil +} + +// percentile returns the nearest-rank p-th percentile (0 < p <= 100) of +// samples; the slice is not modified and an empty slice yields 0. +func percentile(samples []float64, p float64) float64 { + if len(samples) == 0 { + return 0 + } + s := append([]float64(nil), samples...) + sort.Float64s(s) + rank := int(math.Ceil(p / 100 * float64(len(s)))) + rank = min(max(rank, 1), len(s)) + return s[rank-1] +} + +// round3 rounds to a microsecond. perfbench itself rounds to 0.1 ms; the planner +// takes tens of microseconds, and a p95 rounded to 0.0 is not a usable timing. +func round3(v float64) float64 { return math.Round(v*1000) / 1000 } + +// Summarize turns the raw samples of one metric into the JSON metric: +// nearest-rank p50, p95 and max, each rounded to 0.001 ms. +func Summarize(name, unit string, samples []float64) Metric { + top := 0.0 + for i, v := range samples { + if i == 0 || v > top { + top = v + } + } + return Metric{Name: name, Unit: unit, P50: round3(percentile(samples, 50)), + P95: round3(percentile(samples, 95)), Max: round3(top), N: len(samples)} +} + +// Collector gathers samples per metric and keeps first-appearance order. +type Collector struct { + order []string + unit map[string]string + vals map[string][]float64 +} + +// Add records one sample (milliseconds unless unit says otherwise). +func (c *Collector) Add(name, unit string, v float64) { + if c.unit == nil { + c.unit, c.vals = map[string]string{}, map[string][]float64{} + } + if _, seen := c.unit[name]; !seen { + c.order = append(c.order, name) + c.unit[name] = unit + } + c.vals[name] = append(c.vals[name], v) +} + +// Metrics summarizes every collected metric in first-appearance order. +func (c *Collector) Metrics() []Metric { + out := make([]Metric, 0, len(c.order)) + for _, name := range c.order { + out = append(out, Summarize(name, c.unit[name], c.vals[name])) + } + return out +} diff --git a/free/ci/bench/result_test.go b/free/ci/bench/result_test.go new file mode 100644 index 000000000..56684c675 --- /dev/null +++ b/free/ci/bench/result_test.go @@ -0,0 +1,124 @@ +package bench + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestSummarizeNearestRank(t *testing.T) { + ten := []float64{5, 1, 9, 3, 7, 2, 8, 4, 6, 10} + m := Summarize("x", "ms", ten) + if m.P50 != 5 || m.P95 != 10 || m.Max != 10 || m.N != 10 { + t.Fatalf("ten samples: %+v (p95 of 10 is the maximum)", m) + } + twenty := make([]float64, 20) + for i := range twenty { + twenty[i] = float64(i + 1) + } + if m := Summarize("x", "ms", twenty); m.P50 != 10 || m.P95 != 19 { + t.Fatalf("twenty samples: %+v", m) + } + if m := Summarize("x", "ms", []float64{0.0004, 0.0123}); m.P95 != 0.012 { + t.Fatalf("sub-millisecond timings keep microsecond resolution: %+v", m) + } + if m := Summarize("x", "ms", nil); m.N != 0 || m.P95 != 0 { + t.Fatalf("empty: %+v", m) + } +} + +func TestSummarizeLeavesInputAlone(t *testing.T) { + in := []float64{3, 1, 2} + Summarize("x", "ms", in) + if in[0] != 3 || in[1] != 1 || in[2] != 2 { + t.Fatalf("input reordered: %v", in) + } +} + +// TestResultKeyOrder pins the JSON key order of contract:cli.perfbench v1. +func TestResultKeyOrder(t *testing.T) { + raw, err := Marshal(NewResult("abc", 10, []Metric{Summarize("ci.startup", "ms", []float64{1})})) + if err != nil { + t.Fatal(err) + } + last := -1 + for _, key := range []string{`"schema"`, `"scenario"`, `"sha"`, `"goos"`, `"goarch"`, `"runs"`, `"injected"`, `"metrics"`, + `"name"`, `"unit"`, `"p50"`, `"p95"`, `"max"`, `"n"`} { + at := strings.Index(string(raw), key) + if at <= last { + t.Fatalf("key %s out of order in:\n%s", key, raw) + } + last = at + } + if !strings.HasSuffix(string(raw), "}\n") || !strings.Contains(string(raw), `"schema": "perfbench/v1"`) { + t.Fatalf("not the indented perfbench/v1 form:\n%s", raw) + } +} + +func TestReadResult(t *testing.T) { + dir := t.TempDir() + good := filepath.Join(dir, "good.json") + raw, _ := Marshal(NewResult("", 1, nil)) + if err := os.WriteFile(good, raw, 0o644); err != nil { + t.Fatal(err) + } + if _, err := ReadResult(good); err != nil { + t.Fatal(err) + } + bad := filepath.Join(dir, "bad.json") + doc, _ := json.Marshal(map[string]any{"schema": "other/v9"}) + if err := os.WriteFile(bad, doc, 0o644); err != nil { + t.Fatal(err) + } + if _, err := ReadResult(bad); err == nil || !strings.Contains(err.Error(), "schema") { + t.Fatalf("wrong schema accepted: %v", err) + } + if _, err := ReadResult(filepath.Join(dir, "missing.json")); err == nil { + t.Fatal("missing file accepted") + } +} + +func TestCollectorKeepsFirstAppearanceOrder(t *testing.T) { + var c Collector + c.Add("b", "ms", 1) + c.Add("a", "ms", 2) + c.Add("b", "ms", 3) + ms := c.Metrics() + if len(ms) != 2 || ms[0].Name != "b" || ms[0].N != 2 || ms[1].Name != "a" { + t.Fatalf("%+v", ms) + } +} + +func TestBudgetFromResult(t *testing.T) { + base := NewResult("", 10, []Metric{ + Summarize("ci.startup", "ms", []float64{10}), + Summarize("ci.plan", "ms", []float64{0.04}), + Summarize("ci.bytes", "B", []float64{500}), + Summarize("ci.service_start_warm", "ms", []float64{1000}), + }) + b, err := BudgetFromResult(base, "note", func(n string) bool { return !strings.HasPrefix(n, "ci.service_") }) + if err != nil { + t.Fatal(err) + } + if b.Schema != BudgetSchemaV1 || b.P95MS["ci.startup"] != 13 || b.P95MS["ci.plan"] != 1 { + t.Fatalf("%+v", b) + } + if _, ok := b.P95MS["ci.bytes"]; ok { + t.Fatal("a non-ms metric got a ms ceiling") + } + if _, ok := b.P95MS["ci.service_start_warm"]; ok { + t.Fatal("the filter was ignored") + } + if got := strings.Join(b.Names(), ","); got != "ci.plan,ci.startup" { + t.Fatalf("names %s", got) + } + zero := NewResult("", 1, []Metric{{Name: "ci.x", Unit: "ms"}}) + if _, err := BudgetFromResult(zero, "", nil); err == nil { + t.Fatal("a zero p95 was given a ceiling") + } + if _, err := BudgetFromResult(NewResult("", 1, nil), "", nil); err == nil { + t.Fatal("an empty baseline produced a budget") + } +} diff --git a/free/ci/bench/run.go b/free/ci/bench/run.go new file mode 100644 index 000000000..dffea95d1 --- /dev/null +++ b/free/ci/bench/run.go @@ -0,0 +1,161 @@ +package bench + +// Purpose: the harness loop. Each iteration times the startup of the binary, +// the in-process scenarios, and the end-to-end gate on the tiny and medium +// fixtures, v1.4 and V15 interleaved (the order alternates per iteration so +// neither side always runs on a warm cache the other filled). +// Inputs: the nself-ci binary, run and warm-up counts, a scratch directory. +// Outputs: a perfbench/v1 Result. +// Constraints: warm-up iterations run the same code and are discarded; every +// end-to-end run must exit 0 (a failing gate is a fixture bug, not a timing); +// the same binary serves both modes, selected by NSELF_V15 as in production +// (compat.V15), and both use one NSELF_CI_HOME so the store is warm for both. + +import ( + "context" + "fmt" + "io" + "os" + "path/filepath" +) + +// Metric names (Ticket constraint; unit ms). +const ( + MetricStartup = "ci.startup" + MetricDetect = "ci.detect" + MetricPlan = "ci.plan" + MetricStoreOpen = "ci.store_open" + MetricExecOverhead = "ci.exec_overhead" + MetricExecBaseline = "ci.exec_baseline" + MetricCapacityProbe = "ci.capacity_probe" + MetricMediumV14 = "ci.e2e_medium.v14" + MetricMediumV15 = "ci.e2e_medium.v15" + MetricServiceWarm = "ci.service_start_warm" + MetricServiceCold = "ci.service_start_cold" +) + +// Config is one harness run. +type Config struct { + Bin string // the nself-ci binary + Runs int // measured iterations + Warmup int // discarded iterations + Dir string // scratch directory, created by the caller + SHA string // revision measured, "" when unknown + SkipMedium bool + Services *ServiceConfig // nil skips the service scenarios + Log io.Writer +} + +func (c Config) logf(format string, a ...any) { + if c.Log != nil { + _, _ = fmt.Fprintf(c.Log, format+"\n", a...) + } +} + +// mode is one engine under test. +type mode struct { + label string + env string // value of NSELF_V15 + args func(dir string) []string +} + +var modes = []mode{ + {"v14", "0", func(dir string) []string { return []string{"--no-status", "--no-gitleaks", dir} }}, + {"v15", "1", func(dir string) []string { return []string{"--no-status", "--pipeline", "gate", dir} }}, +} + +// Run executes the harness. +func Run(ctx context.Context, cfg Config) (Result, error) { + if cfg.Runs < 1 || cfg.Warmup < 0 { + return Result{}, fmt.Errorf("runs must be >= 1 and warmup >= 0") + } + repos := map[string]string{Tiny: filepath.Join(cfg.Dir, Tiny)} + if !cfg.SkipMedium { + repos[Medium] = filepath.Join(cfg.Dir, Medium) + } + for name, dir := range repos { + if err := Materialize(ctx, name, dir); err != nil { + return Result{}, err + } + } + home := filepath.Join(cfg.Dir, "home") + if err := os.MkdirAll(home, 0o700); err != nil { + return Result{}, err + } + inproc, err := PrepareInproc(repos[Tiny], filepath.Join(cfg.Dir, "scratch")) + if err != nil { + return Result{}, err + } + var col Collector + for i := -cfg.Warmup; i < cfg.Runs; i++ { + keep := i >= 0 + add := func(name string, ms float64) { + if keep { + col.Add(name, "ms", ms) + } + } + if err := iteration(ctx, cfg, repos, home, inproc, i, add); err != nil { + return Result{}, err + } + cfg.logf("iteration %d/%d done", i+cfg.Warmup+1, cfg.Runs+cfg.Warmup) + } + if cfg.Services != nil { + if err := measureServices(ctx, cfg, home, &col); err != nil { + return Result{}, err + } + } + return NewResult(cfg.SHA, cfg.Runs, col.Metrics()), nil +} + +// iteration runs every scenario once. +func iteration(ctx context.Context, cfg Config, repos map[string]string, home string, p *Inproc, i int, add func(string, float64)) error { + env := func(v15 string) []string { + return append(baseEnv(), "NSELF_CI_HOME="+home, "NSELF_TELEMETRY_OPT_OUT=1", "NSELF_V15="+v15) + } + r, err := must(ctx, []string{cfg.Bin, "--help"}, cfg.Dir, env("1")) + if err != nil { + return err + } + add(MetricStartup, r.MS()) + steps := []struct { + name string + fn func() error + }{ + {MetricDetect, p.Detect}, {MetricPlan, p.Plan}, {MetricStoreOpen, p.StoreOpen}, + } + for _, s := range steps { + wall, err := timeIt(s.fn) + if err != nil { + return fmt.Errorf("%s: %w", s.name, err) + } + add(s.name, wall) + } + if r, err = p.ExecOverhead(ctx); err != nil { + return err + } + add(MetricExecOverhead, r.MS()) + if r, err = p.RawSpawn(ctx); err != nil { + return err + } + add(MetricExecBaseline, r.MS()) + wall, _ := timeIt(func() error { CapacityProbe(); return nil }) + add(MetricCapacityProbe, wall) + order := modes + if i%2 != 0 { + order = []mode{modes[1], modes[0]} + } + for _, name := range []string{Tiny, Medium} { + dir, ok := repos[name] + if !ok { + continue + } + for _, m := range order { + r, err := must(ctx, append([]string{cfg.Bin}, m.args(dir)...), dir, env(m.env)) + if err != nil { + return fmt.Errorf("e2e %s %s: %w", name, m.label, err) + } + add(fmt.Sprintf("ci.e2e_%s.%s", name, m.label), r.MS()) + } + } + return nil +} diff --git a/free/ci/bench/services.go b/free/ci/bench/services.go new file mode 100644 index 000000000..88fdaf2a5 --- /dev/null +++ b/free/ci/bench/services.go @@ -0,0 +1,99 @@ +package bench + +// Purpose: the service-start scenarios: how long a disposable service +// dependency (Epic D9) takes to come up through the nself CLI, warm (image +// already pulled) and cold (image removed first, so the pull is in the time). +// Inputs: an nself CLI binary and a Docker daemon, both on this host. +// Outputs: ci.service_start_warm and ci.service_start_cold samples. +// Constraints: opt-in (-services) because it needs Docker and a v1.5 nself CLI; +// the cold scenario removes only the images the throwaway project itself used, +// and only when docker agrees nothing else uses them (no force), so a +// developer machine loses at most a cached image it can pull again. + +import ( + "context" + "fmt" + "strings" + + "github.com/nself-org/plugins/free/ci/internal/services" +) + +// ServiceConfig selects the service scenarios. +type ServiceConfig struct { + Nself string // the nself CLI binary + Component string // the component to start, default postgres + Cold bool // also measure the cold start + ColdRuns int // cold samples, default the run count +} + +// measureServices adds the service samples to col. +func measureServices(ctx context.Context, cfg Config, home string, col *Collector) error { + sc := cfg.Services + comp := sc.Component + if comp == "" { + comp = "postgres" + } + prov := &services.Provisioner{Run: &services.Funnel{Home: home}, Home: home, Nself: sc.Nself} + specs := []services.Spec{{Name: comp, Component: comp}} + up := func() (float64, string, error) { + var h *services.Handle + ms, err := timeIt(func() (e error) { h, e = prov.Up(ctx, specs); return }) + if err != nil { + return 0, "", fmt.Errorf("service %s: %w", comp, err) + } + images := imagesOf(ctx, h.Project) + if err := prov.Down(ctx, h); err != nil { + return 0, "", fmt.Errorf("service %s teardown: %w", comp, err) + } + return ms, images, nil + } + _, images, err := up() // pulls the image; not a sample + if err != nil { + return err + } + for i := 0; i < cfg.Runs; i++ { + ms, _, err := up() + if err != nil { + return err + } + col.Add(MetricServiceWarm, "ms", ms) + } + if !sc.Cold { + return nil + } + n := sc.ColdRuns + if n < 1 { + n = cfg.Runs + } + for i := 0; i < n; i++ { + for _, img := range strings.Fields(images) { + if _, err := must(ctx, []string{"docker", "image", "rm", img}, cfg.Dir, baseEnv()); err != nil { + return fmt.Errorf("cold start needs the image removed: %w", err) + } + } + ms, _, err := up() + if err != nil { + return err + } + col.Add(MetricServiceCold, "ms", ms) + } + return nil +} + +// imagesOf lists the images of the containers of compose project name. +func imagesOf(ctx context.Context, project string) string { + r, err := must(ctx, []string{"docker", "ps", "-a", "--filter", "label=com.docker.compose.project=" + project, + "--format", "{{.Image}}"}, "", baseEnv()) + if err != nil { + return "" + } + seen := map[string]bool{} + var out []string + for _, img := range strings.Fields(r.Tail) { + if !seen[img] { + seen[img] = true + out = append(out, img) + } + } + return strings.Join(out, " ") +} diff --git a/free/ci/bench/spawn.go b/free/ci/bench/spawn.go new file mode 100644 index 000000000..f84bdae65 --- /dev/null +++ b/free/ci/bench/spawn.go @@ -0,0 +1,85 @@ +package bench + +// Purpose: start the processes the harness times (the nself-ci binary under +// test, go build, git for fixtures) and report their wall time. +// Inputs: argv, working directory, environment. +// Outputs: wall time of the child, its exit code, and a short output tail. +// Constraints: this is the one place the harness starts a process, and it is a +// timing tool, not a job runner: it must not add the engine's own funnel +// (workspace copy, process-tree polling) to the time it measures, so it uses +// os/exec directly. architecture/layers_test.go and +// internal/nodes/provision/single_exec_test.go list this file by name (P7-CI-46, +// lead ruling in the CI Epic Invalidations). + +import ( + "bytes" + "context" + "errors" + "fmt" + "os/exec" + "strings" + "time" +) + +// tailLimit bounds the output kept from a child, for failure messages. +const tailLimit = 4096 + +// tailBuffer keeps the last tailLimit bytes written to it. +type tailBuffer struct{ b []byte } + +func (t *tailBuffer) Write(p []byte) (int, error) { + t.b = append(t.b, p...) + if len(t.b) > tailLimit { + t.b = t.b[len(t.b)-tailLimit:] + } + return len(p), nil +} + +// Timing is the outcome of one timed child. +type Timing struct { + Wall time.Duration + ExitCode int + Tail string +} + +// MS is the wall time in milliseconds. +func (r Timing) MS() float64 { return float64(r.Wall.Microseconds()) / 1000 } + +// spawn runs argv in dir with env (nil env means an empty environment) and +// times it. A child that exits non-zero is a Timing with ExitCode set and a nil +// error; only a child that could not be started returns an error. +func spawn(ctx context.Context, argv []string, dir string, env []string) (Timing, error) { + if len(argv) == 0 { + return Timing{}, fmt.Errorf("spawn: empty command") + } + cmd := exec.CommandContext(ctx, argv[0], argv[1:]...) + cmd.Dir = dir + cmd.Env = env + var tail tailBuffer + cmd.Stdout, cmd.Stderr = &tail, &tail + start := time.Now() + err := cmd.Run() + wall := time.Since(start) + run := Timing{Wall: wall, Tail: strings.TrimSpace(string(bytes.TrimSpace(tail.b)))} + if err == nil { + return run, nil + } + var exit *exec.ExitError + if errors.As(err, &exit) { + run.ExitCode = exit.ExitCode() + return run, nil + } + return run, err +} + +// must runs argv and fails unless it exits 0. +func must(ctx context.Context, argv []string, dir string, env []string) (Timing, error) { + r, err := spawn(ctx, argv, dir, env) + if err != nil { + return r, fmt.Errorf("%s: %w", strings.Join(argv, " "), err) + } + if r.ExitCode != 0 { + return r, fmt.Errorf("%s: exit %d: %s", strings.Join(argv, " "), r.ExitCode, r.Tail) + } + return r, nil +} diff --git a/free/ci/bench/testdata/medium/.nself/ci.yaml.fixture b/free/ci/bench/testdata/medium/.nself/ci.yaml.fixture new file mode 100644 index 000000000..da2ce7763 --- /dev/null +++ b/free/ci/bench/testdata/medium/.nself/ci.yaml.fixture @@ -0,0 +1,24 @@ +# The checks the v1.4 gate runs on a Go plus Node repo (gofmt, go vet, go test, +# and the package's lint and test scripts), so the V15 and v1.4 wall times +# compare equal work. Run with --pipeline gate. +version: 1 +jobs: + fmt: + kind: static + run: [gofmt, -l, .] + vet: + kind: static + run: [go, vet, ./...] + test: + kind: test + run: [go, test, -count=1, ./...] + web-lint: + kind: static + run: [npm, run, lint] + web-test: + kind: test + run: [npm, run, test] +pipelines: + gate: + triggers: [manual] + jobs: [fmt, vet, test, web-lint, web-test] diff --git a/free/ci/bench/testdata/medium/cmd/app/main.go b/free/ci/bench/testdata/medium/cmd/app/main.go new file mode 100644 index 000000000..1caa57d7d --- /dev/null +++ b/free/ci/bench/testdata/medium/cmd/app/main.go @@ -0,0 +1,10 @@ +package main + +import ( + "fmt" + + "example.com/medium/internal/alpha" + "example.com/medium/internal/beta" +) + +func main() { fmt.Println(alpha.Name(), beta.Name()) } diff --git a/free/ci/bench/testdata/medium/go.mod.fixture b/free/ci/bench/testdata/medium/go.mod.fixture new file mode 100644 index 000000000..967a5e99f --- /dev/null +++ b/free/ci/bench/testdata/medium/go.mod.fixture @@ -0,0 +1,3 @@ +module example.com/medium + +go 1.22 diff --git a/free/ci/bench/testdata/medium/internal/alpha/alpha.go b/free/ci/bench/testdata/medium/internal/alpha/alpha.go new file mode 100644 index 000000000..e0f085e74 --- /dev/null +++ b/free/ci/bench/testdata/medium/internal/alpha/alpha.go @@ -0,0 +1,24 @@ +package alpha + +import "strings" + +// Name returns the package name in upper case. +func Name() string { return strings.ToUpper("alpha") } + +// Sum adds the values. +func Sum(values ...int) int { + total := 0 + for _, v := range values { + total += v + } + return total +} + +// Repeat joins n copies of s with a comma. +func Repeat(s string, n int) string { + parts := make([]string, 0, n) + for i := 0; i < n; i++ { + parts = append(parts, s) + } + return strings.Join(parts, ",") +} diff --git a/free/ci/bench/testdata/medium/internal/alpha/alpha_test.go b/free/ci/bench/testdata/medium/internal/alpha/alpha_test.go new file mode 100644 index 000000000..7aca34d32 --- /dev/null +++ b/free/ci/bench/testdata/medium/internal/alpha/alpha_test.go @@ -0,0 +1,21 @@ +package alpha + +import "testing" + +func TestName(t *testing.T) { + if Name() == "" { + t.Fatal("empty name") + } +} + +func TestSum(t *testing.T) { + if got := Sum(1, 2, 3); got != 6 { + t.Fatalf("Sum = %d", got) + } +} + +func TestRepeat(t *testing.T) { + if got := Repeat("a", 3); got != "a,a,a" { + t.Fatalf("Repeat = %q", got) + } +} diff --git a/free/ci/bench/testdata/medium/internal/beta/beta.go b/free/ci/bench/testdata/medium/internal/beta/beta.go new file mode 100644 index 000000000..4fc07c2df --- /dev/null +++ b/free/ci/bench/testdata/medium/internal/beta/beta.go @@ -0,0 +1,24 @@ +package beta + +import "strings" + +// Name returns the package name in upper case. +func Name() string { return strings.ToUpper("beta") } + +// Sum adds the values. +func Sum(values ...int) int { + total := 0 + for _, v := range values { + total += v + } + return total +} + +// Repeat joins n copies of s with a comma. +func Repeat(s string, n int) string { + parts := make([]string, 0, n) + for i := 0; i < n; i++ { + parts = append(parts, s) + } + return strings.Join(parts, ",") +} diff --git a/free/ci/bench/testdata/medium/internal/beta/beta_test.go b/free/ci/bench/testdata/medium/internal/beta/beta_test.go new file mode 100644 index 000000000..4e8efb8e5 --- /dev/null +++ b/free/ci/bench/testdata/medium/internal/beta/beta_test.go @@ -0,0 +1,21 @@ +package beta + +import "testing" + +func TestName(t *testing.T) { + if Name() == "" { + t.Fatal("empty name") + } +} + +func TestSum(t *testing.T) { + if got := Sum(1, 2, 3); got != 6 { + t.Fatalf("Sum = %d", got) + } +} + +func TestRepeat(t *testing.T) { + if got := Repeat("a", 3); got != "a,a,a" { + t.Fatalf("Repeat = %q", got) + } +} diff --git a/free/ci/bench/testdata/medium/internal/delta/delta.go b/free/ci/bench/testdata/medium/internal/delta/delta.go new file mode 100644 index 000000000..f730963b0 --- /dev/null +++ b/free/ci/bench/testdata/medium/internal/delta/delta.go @@ -0,0 +1,24 @@ +package delta + +import "strings" + +// Name returns the package name in upper case. +func Name() string { return strings.ToUpper("delta") } + +// Sum adds the values. +func Sum(values ...int) int { + total := 0 + for _, v := range values { + total += v + } + return total +} + +// Repeat joins n copies of s with a comma. +func Repeat(s string, n int) string { + parts := make([]string, 0, n) + for i := 0; i < n; i++ { + parts = append(parts, s) + } + return strings.Join(parts, ",") +} diff --git a/free/ci/bench/testdata/medium/internal/delta/delta_test.go b/free/ci/bench/testdata/medium/internal/delta/delta_test.go new file mode 100644 index 000000000..2ab8537dc --- /dev/null +++ b/free/ci/bench/testdata/medium/internal/delta/delta_test.go @@ -0,0 +1,21 @@ +package delta + +import "testing" + +func TestName(t *testing.T) { + if Name() == "" { + t.Fatal("empty name") + } +} + +func TestSum(t *testing.T) { + if got := Sum(1, 2, 3); got != 6 { + t.Fatalf("Sum = %d", got) + } +} + +func TestRepeat(t *testing.T) { + if got := Repeat("a", 3); got != "a,a,a" { + t.Fatalf("Repeat = %q", got) + } +} diff --git a/free/ci/bench/testdata/medium/internal/epsilon/epsilon.go b/free/ci/bench/testdata/medium/internal/epsilon/epsilon.go new file mode 100644 index 000000000..703f9b8ac --- /dev/null +++ b/free/ci/bench/testdata/medium/internal/epsilon/epsilon.go @@ -0,0 +1,24 @@ +package epsilon + +import "strings" + +// Name returns the package name in upper case. +func Name() string { return strings.ToUpper("epsilon") } + +// Sum adds the values. +func Sum(values ...int) int { + total := 0 + for _, v := range values { + total += v + } + return total +} + +// Repeat joins n copies of s with a comma. +func Repeat(s string, n int) string { + parts := make([]string, 0, n) + for i := 0; i < n; i++ { + parts = append(parts, s) + } + return strings.Join(parts, ",") +} diff --git a/free/ci/bench/testdata/medium/internal/epsilon/epsilon_test.go b/free/ci/bench/testdata/medium/internal/epsilon/epsilon_test.go new file mode 100644 index 000000000..ae0a9b2a2 --- /dev/null +++ b/free/ci/bench/testdata/medium/internal/epsilon/epsilon_test.go @@ -0,0 +1,21 @@ +package epsilon + +import "testing" + +func TestName(t *testing.T) { + if Name() == "" { + t.Fatal("empty name") + } +} + +func TestSum(t *testing.T) { + if got := Sum(1, 2, 3); got != 6 { + t.Fatalf("Sum = %d", got) + } +} + +func TestRepeat(t *testing.T) { + if got := Repeat("a", 3); got != "a,a,a" { + t.Fatalf("Repeat = %q", got) + } +} diff --git a/free/ci/bench/testdata/medium/internal/eta/eta.go b/free/ci/bench/testdata/medium/internal/eta/eta.go new file mode 100644 index 000000000..141c60e97 --- /dev/null +++ b/free/ci/bench/testdata/medium/internal/eta/eta.go @@ -0,0 +1,24 @@ +package eta + +import "strings" + +// Name returns the package name in upper case. +func Name() string { return strings.ToUpper("eta") } + +// Sum adds the values. +func Sum(values ...int) int { + total := 0 + for _, v := range values { + total += v + } + return total +} + +// Repeat joins n copies of s with a comma. +func Repeat(s string, n int) string { + parts := make([]string, 0, n) + for i := 0; i < n; i++ { + parts = append(parts, s) + } + return strings.Join(parts, ",") +} diff --git a/free/ci/bench/testdata/medium/internal/eta/eta_test.go b/free/ci/bench/testdata/medium/internal/eta/eta_test.go new file mode 100644 index 000000000..38b3e3571 --- /dev/null +++ b/free/ci/bench/testdata/medium/internal/eta/eta_test.go @@ -0,0 +1,21 @@ +package eta + +import "testing" + +func TestName(t *testing.T) { + if Name() == "" { + t.Fatal("empty name") + } +} + +func TestSum(t *testing.T) { + if got := Sum(1, 2, 3); got != 6 { + t.Fatalf("Sum = %d", got) + } +} + +func TestRepeat(t *testing.T) { + if got := Repeat("a", 3); got != "a,a,a" { + t.Fatalf("Repeat = %q", got) + } +} diff --git a/free/ci/bench/testdata/medium/internal/gamma/gamma.go b/free/ci/bench/testdata/medium/internal/gamma/gamma.go new file mode 100644 index 000000000..1f0f8970d --- /dev/null +++ b/free/ci/bench/testdata/medium/internal/gamma/gamma.go @@ -0,0 +1,24 @@ +package gamma + +import "strings" + +// Name returns the package name in upper case. +func Name() string { return strings.ToUpper("gamma") } + +// Sum adds the values. +func Sum(values ...int) int { + total := 0 + for _, v := range values { + total += v + } + return total +} + +// Repeat joins n copies of s with a comma. +func Repeat(s string, n int) string { + parts := make([]string, 0, n) + for i := 0; i < n; i++ { + parts = append(parts, s) + } + return strings.Join(parts, ",") +} diff --git a/free/ci/bench/testdata/medium/internal/gamma/gamma_test.go b/free/ci/bench/testdata/medium/internal/gamma/gamma_test.go new file mode 100644 index 000000000..7a8cff14e --- /dev/null +++ b/free/ci/bench/testdata/medium/internal/gamma/gamma_test.go @@ -0,0 +1,21 @@ +package gamma + +import "testing" + +func TestName(t *testing.T) { + if Name() == "" { + t.Fatal("empty name") + } +} + +func TestSum(t *testing.T) { + if got := Sum(1, 2, 3); got != 6 { + t.Fatalf("Sum = %d", got) + } +} + +func TestRepeat(t *testing.T) { + if got := Repeat("a", 3); got != "a,a,a" { + t.Fatalf("Repeat = %q", got) + } +} diff --git a/free/ci/bench/testdata/medium/internal/theta/theta.go b/free/ci/bench/testdata/medium/internal/theta/theta.go new file mode 100644 index 000000000..3a4bc4e3d --- /dev/null +++ b/free/ci/bench/testdata/medium/internal/theta/theta.go @@ -0,0 +1,24 @@ +package theta + +import "strings" + +// Name returns the package name in upper case. +func Name() string { return strings.ToUpper("theta") } + +// Sum adds the values. +func Sum(values ...int) int { + total := 0 + for _, v := range values { + total += v + } + return total +} + +// Repeat joins n copies of s with a comma. +func Repeat(s string, n int) string { + parts := make([]string, 0, n) + for i := 0; i < n; i++ { + parts = append(parts, s) + } + return strings.Join(parts, ",") +} diff --git a/free/ci/bench/testdata/medium/internal/theta/theta_test.go b/free/ci/bench/testdata/medium/internal/theta/theta_test.go new file mode 100644 index 000000000..4329c62d1 --- /dev/null +++ b/free/ci/bench/testdata/medium/internal/theta/theta_test.go @@ -0,0 +1,21 @@ +package theta + +import "testing" + +func TestName(t *testing.T) { + if Name() == "" { + t.Fatal("empty name") + } +} + +func TestSum(t *testing.T) { + if got := Sum(1, 2, 3); got != 6 { + t.Fatalf("Sum = %d", got) + } +} + +func TestRepeat(t *testing.T) { + if got := Repeat("a", 3); got != "a,a,a" { + t.Fatalf("Repeat = %q", got) + } +} diff --git a/free/ci/bench/testdata/medium/internal/zeta/zeta.go b/free/ci/bench/testdata/medium/internal/zeta/zeta.go new file mode 100644 index 000000000..8153ba7bf --- /dev/null +++ b/free/ci/bench/testdata/medium/internal/zeta/zeta.go @@ -0,0 +1,24 @@ +package zeta + +import "strings" + +// Name returns the package name in upper case. +func Name() string { return strings.ToUpper("zeta") } + +// Sum adds the values. +func Sum(values ...int) int { + total := 0 + for _, v := range values { + total += v + } + return total +} + +// Repeat joins n copies of s with a comma. +func Repeat(s string, n int) string { + parts := make([]string, 0, n) + for i := 0; i < n; i++ { + parts = append(parts, s) + } + return strings.Join(parts, ",") +} diff --git a/free/ci/bench/testdata/medium/internal/zeta/zeta_test.go b/free/ci/bench/testdata/medium/internal/zeta/zeta_test.go new file mode 100644 index 000000000..b11059256 --- /dev/null +++ b/free/ci/bench/testdata/medium/internal/zeta/zeta_test.go @@ -0,0 +1,21 @@ +package zeta + +import "testing" + +func TestName(t *testing.T) { + if Name() == "" { + t.Fatal("empty name") + } +} + +func TestSum(t *testing.T) { + if got := Sum(1, 2, 3); got != 6 { + t.Fatalf("Sum = %d", got) + } +} + +func TestRepeat(t *testing.T) { + if got := Repeat("a", 3); got != "a,a,a" { + t.Fatalf("Repeat = %q", got) + } +} diff --git a/free/ci/bench/testdata/medium/package.json b/free/ci/bench/testdata/medium/package.json new file mode 100644 index 000000000..38599e1fa --- /dev/null +++ b/free/ci/bench/testdata/medium/package.json @@ -0,0 +1,10 @@ +{ + "name": "medium", + "version": "0.0.0", + "private": true, + "type": "module", + "scripts": { + "lint": "node src/check.mjs", + "test": "node --test" + } +} diff --git a/free/ci/bench/testdata/medium/src/check.mjs b/free/ci/bench/testdata/medium/src/check.mjs new file mode 100644 index 000000000..ebabf2c8f --- /dev/null +++ b/free/ci/bench/testdata/medium/src/check.mjs @@ -0,0 +1,5 @@ +import { greet } from './greet.mjs'; + +if (greet('ci') !== 'hello ci') { + throw new Error('greet is broken'); +} diff --git a/free/ci/bench/testdata/medium/src/greet.mjs b/free/ci/bench/testdata/medium/src/greet.mjs new file mode 100644 index 000000000..15e5e8dd5 --- /dev/null +++ b/free/ci/bench/testdata/medium/src/greet.mjs @@ -0,0 +1 @@ +export const greet = (name) => `hello ${name}`; diff --git a/free/ci/bench/testdata/medium/test/greet.test.mjs b/free/ci/bench/testdata/medium/test/greet.test.mjs new file mode 100644 index 000000000..13a3742de --- /dev/null +++ b/free/ci/bench/testdata/medium/test/greet.test.mjs @@ -0,0 +1,7 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { greet } from '../src/greet.mjs'; + +test('greet', () => { + assert.equal(greet('ci'), 'hello ci'); +}); diff --git a/free/ci/bench/testdata/tiny/.nself/ci.yaml.fixture b/free/ci/bench/testdata/tiny/.nself/ci.yaml.fixture new file mode 100644 index 000000000..ca3d9b0b9 --- /dev/null +++ b/free/ci/bench/testdata/tiny/.nself/ci.yaml.fixture @@ -0,0 +1,17 @@ +# The same three checks the v1.4 gate runs on a Go repo (gofmt, go vet, go test), +# so the V15 and v1.4 wall times compare equal work. Run with --pipeline gate. +version: 1 +jobs: + fmt: + kind: static + run: [gofmt, -l, .] + vet: + kind: static + run: [go, vet, ./...] + test: + kind: test + run: [go, test, -count=1, ./...] +pipelines: + gate: + triggers: [manual] + jobs: [fmt, vet, test] diff --git a/free/ci/bench/testdata/tiny/go.mod.fixture b/free/ci/bench/testdata/tiny/go.mod.fixture new file mode 100644 index 000000000..b317940b9 --- /dev/null +++ b/free/ci/bench/testdata/tiny/go.mod.fixture @@ -0,0 +1,3 @@ +module example.com/tiny + +go 1.22 diff --git a/free/ci/bench/testdata/tiny/tiny.go b/free/ci/bench/testdata/tiny/tiny.go new file mode 100644 index 000000000..abd7f87f8 --- /dev/null +++ b/free/ci/bench/testdata/tiny/tiny.go @@ -0,0 +1,4 @@ +package tiny + +// Add returns the sum of a and b. +func Add(a, b int) int { return a + b } diff --git a/free/ci/bench/testdata/tiny/tiny_test.go b/free/ci/bench/testdata/tiny/tiny_test.go new file mode 100644 index 000000000..14efadc47 --- /dev/null +++ b/free/ci/bench/testdata/tiny/tiny_test.go @@ -0,0 +1,9 @@ +package tiny + +import "testing" + +func TestAdd(t *testing.T) { + if Add(1, 2) != 3 { + t.Fatal("Add(1, 2) != 3") + } +} diff --git a/free/ci/bench/timeit.go b/free/ci/bench/timeit.go new file mode 100644 index 000000000..ed80af664 --- /dev/null +++ b/free/ci/bench/timeit.go @@ -0,0 +1,10 @@ +package bench + +import "time" + +// timeIt runs fn once and returns its wall time in milliseconds. +func timeIt(fn func() error) (float64, error) { + start := time.Now() + err := fn() + return float64(time.Since(start).Microseconds()) / 1000, err +} diff --git a/free/ci/internal/nodes/provision/single_exec_test.go b/free/ci/internal/nodes/provision/single_exec_test.go index a804a0012..ac3551d61 100644 --- a/free/ci/internal/nodes/provision/single_exec_test.go +++ b/free/ci/internal/nodes/provision/single_exec_test.go @@ -56,6 +56,8 @@ var dynamicProgram = map[string]bool{ // nself-ci-agent job-shim replaces itself (syscall.Exec) with the step argv the agent sent on // stdin, as a job user chosen by sudo; never ssh (P7-NODE-06). "cmd/agent/job_shim.go:execShimRequest": true, + // The cibench timing harness starts the binary it measures (P7-CI-46). + "bench/spawn.go:spawn": true, // The container runtime binary is checked against runtimeBinary (docker or // podman by name or path) before any of these runs it (P7-TRUST-04). "internal/exec/container.go:containerStopper": true, From 0e23964e8f8ac6877ab3115efd949098eca90fc3 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 21:21:39 -0400 Subject: [PATCH 02/19] feat(ci): per-platform perf budgets, ci-bench workflow and controls (P7-CI-46) --- .github/workflows/ci-bench.yml | 111 ++++++++++++++++++ free/ci/bench/perf-budget-darwin-arm64.json | 16 +++ free/ci/bench/perf-budget.json | 16 +++ .../testdata/medium/.nself/ci.yaml.fixture | 7 ++ free/ci/bench/testdata/ratio-ok-result.json | 99 ++++++++++++++++ free/ci/bench/testdata/slow-result.json | 99 ++++++++++++++++ .../testdata/tiny/.nself/ci.yaml.fixture | 3 + 7 files changed, 351 insertions(+) create mode 100644 .github/workflows/ci-bench.yml create mode 100644 free/ci/bench/perf-budget-darwin-arm64.json create mode 100644 free/ci/bench/perf-budget.json create mode 100644 free/ci/bench/testdata/ratio-ok-result.json create mode 100644 free/ci/bench/testdata/slow-result.json diff --git a/.github/workflows/ci-bench.yml b/.github/workflows/ci-bench.yml new file mode 100644 index 000000000..65895b768 --- /dev/null +++ b/.github/workflows/ci-bench.yml @@ -0,0 +1,111 @@ +name: ci-bench + +# Local overhead of the nself ci V15 engine against the v1.4 gate (P7-CI-46). +# Runs cibench on a hosted runner, then the cli's own `perfbench check` against the +# per-platform budget (absolute p95) and `cibench -ratio-check` (V15 median against +# the v1.4 median). Two controls prove the gates can fire: a result ten times over +# every budget must fail both checks, and a result inside the relative budget must +# pass. Budgets and baselines: hq plan/p7/evidence/CI-46-perf.md. + +on: + pull_request: + paths: + - 'free/ci/**' + - '.github/workflows/ci-bench.yml' + workflow_dispatch: + +permissions: + contents: read + +concurrency: + group: ci-bench-${{ github.ref }} + cancel-in-progress: true + +env: + CGO_ENABLED: '0' + # nself-org/cli revision whose tools/perfbench reads the result and budget files. + CLI_SHA: '529e8917244bffe4ab1aa9d42b081dd3efe5ee73' + +jobs: + bench: + name: bench (${{ matrix.name }}) + runs-on: ${{ matrix.os }} + timeout-minutes: 45 + strategy: + fail-fast: false + matrix: + include: + - name: linux-amd64 + os: ubuntu-latest + budget: perf-budget.json + services: true + - name: macos-arm64 + os: macos-14 + budget: perf-budget-darwin-arm64.json + services: false + env: + BUDGET: ${{ github.workspace }}/free/ci/bench/${{ matrix.budget }} + SLOW: ${{ github.workspace }}/free/ci/bench/testdata/slow-result.json + OK: ${{ github.workspace }}/free/ci/bench/testdata/ratio-ok-result.json + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + repository: nself-org/cli + ref: ${{ env.CLI_SHA }} + path: cli + persist-credentials: false + + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7 + with: + go-version-file: free/ci/go.mod + cache-dependency-path: free/ci/go.sum + + - name: Harness tests + working-directory: free/ci + run: go test -mod=vendor -count=1 ./bench/... + + - name: Measure + working-directory: free/ci + run: go run -mod=vendor ./bench/cmd/cibench -runs 20 -warmup 2 -out "$RUNNER_TEMP/ci-bench.json" + + - name: Absolute p95 budget (cli perfbench check) + working-directory: cli + run: go run -mod=vendor ./tools/perfbench check -budget "$BUDGET" -in "$RUNNER_TEMP/ci-bench.json" + + - name: Relative budget (V15 against v1.4) + working-directory: free/ci + run: go run -mod=vendor ./bench/cmd/cibench -ratio-check -in "$RUNNER_TEMP/ci-bench.json" + + - name: Control - the gates fail an injected slowdown and pass a good result + run: | + set -u + if (cd cli && go run -mod=vendor ./tools/perfbench check -budget "$BUDGET" -in "$SLOW"); then + echo "::error::perfbench check passed a result ten times over budget"; exit 1 + fi + cd free/ci + if go run -mod=vendor ./bench/cmd/cibench -ratio-check -in "$SLOW"; then + echo "::error::the ratio check passed a result ten times over budget"; exit 1 + fi + go run -mod=vendor ./bench/cmd/cibench -ratio-check -in "$OK" + + - name: Service start (warm and cold) + if: matrix.services + run: | + set -eu + (cd cli && go build -mod=vendor -o "$RUNNER_TEMP/nself" ./cmd/nself) + cd free/ci + go run -mod=vendor ./bench/cmd/cibench -runs 10 -warmup 0 -skip-medium -services -cold \ + -nself "$RUNNER_TEMP/nself" -out "$RUNNER_TEMP/ci-bench-services.json" + + - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: always() + with: + name: ci-bench-${{ matrix.name }} + path: | + ${{ runner.temp }}/ci-bench.json + ${{ runner.temp }}/ci-bench-services.json + if-no-files-found: ignore diff --git a/free/ci/bench/perf-budget-darwin-arm64.json b/free/ci/bench/perf-budget-darwin-arm64.json new file mode 100644 index 000000000..6ba0e2689 --- /dev/null +++ b/free/ci/bench/perf-budget-darwin-arm64.json @@ -0,0 +1,16 @@ +{ + "_note": "Absolute p95 ceilings in ms: the recorded baseline p95 plus 25 percent (P7-CI-46), checked by cli tools/perfbench check. Baseline and update rule: hq plan/p7/evidence/CI-46-perf.md. The relative V15/v1.4 budget is checked by cibench -ratio-check.", + "schema": "perfbench-budget/v1", + "platform": "darwin/arm64", + "p95_ms": { + "ci.detect": 1, + "ci.e2e_medium.v14": 1453, + "ci.e2e_medium.v15": 2876, + "ci.e2e_tiny.v14": 555, + "ci.e2e_tiny.v15": 2279, + "ci.exec_overhead": 64, + "ci.plan": 1, + "ci.startup": 21, + "ci.store_open": 16 + } +} diff --git a/free/ci/bench/perf-budget.json b/free/ci/bench/perf-budget.json new file mode 100644 index 000000000..68a57c1b8 --- /dev/null +++ b/free/ci/bench/perf-budget.json @@ -0,0 +1,16 @@ +{ + "_note": "Absolute p95 ceilings in ms: the recorded baseline p95 plus 25 percent (P7-CI-46), checked by cli tools/perfbench check. Baseline and update rule: hq plan/p7/evidence/CI-46-perf.md. The relative V15/v1.4 budget is checked by cibench -ratio-check.", + "schema": "perfbench-budget/v1", + "platform": "linux/amd64", + "p95_ms": { + "ci.detect": 1, + "ci.e2e_medium.v14": 929, + "ci.e2e_medium.v15": 1980, + "ci.e2e_tiny.v14": 400, + "ci.e2e_tiny.v15": 1800, + "ci.exec_overhead": 53, + "ci.plan": 1, + "ci.startup": 13, + "ci.store_open": 35 + } +} diff --git a/free/ci/bench/testdata/medium/.nself/ci.yaml.fixture b/free/ci/bench/testdata/medium/.nself/ci.yaml.fixture index da2ce7763..07d60c333 100644 --- a/free/ci/bench/testdata/medium/.nself/ci.yaml.fixture +++ b/free/ci/bench/testdata/medium/.nself/ci.yaml.fixture @@ -1,23 +1,30 @@ # The checks the v1.4 gate runs on a Go plus Node repo (gofmt, go vet, go test, # and the package's lint and test scripts), so the V15 and v1.4 wall times # compare equal work. Run with --pipeline gate. +# requires.mem_mb: on Linux the engine runs every job under `ulimit -v` set from its +# memory demand (default 256 MB), which kills the Go and Node toolchains at start-up. version: 1 jobs: fmt: kind: static run: [gofmt, -l, .] + requires: {mem_mb: 4096} vet: kind: static run: [go, vet, ./...] + requires: {mem_mb: 4096} test: kind: test run: [go, test, -count=1, ./...] + requires: {mem_mb: 4096} web-lint: kind: static run: [npm, run, lint] + requires: {mem_mb: 4096} web-test: kind: test run: [npm, run, test] + requires: {mem_mb: 4096} pipelines: gate: triggers: [manual] diff --git a/free/ci/bench/testdata/ratio-ok-result.json b/free/ci/bench/testdata/ratio-ok-result.json new file mode 100644 index 000000000..d1f1be2cd --- /dev/null +++ b/free/ci/bench/testdata/ratio-ok-result.json @@ -0,0 +1,99 @@ +{ + "schema": "perfbench/v1", + "scenario": "ci-local-overhead", + "sha": "fixture", + "goos": "linux", + "goarch": "amd64", + "runs": 20, + "injected": false, + "metrics": [ + { + "name": "ci.startup", + "unit": "ms", + "p50": 7.449, + "p95": 10.281, + "max": 13.633, + "n": 20 + }, + { + "name": "ci.detect", + "unit": "ms", + "p50": 0.118, + "p95": 0.437, + "max": 1.153, + "n": 20 + }, + { + "name": "ci.plan", + "unit": "ms", + "p50": 0.043, + "p95": 0.071, + "max": 0.089, + "n": 20 + }, + { + "name": "ci.store_open", + "unit": "ms", + "p50": 24.477, + "p95": 27.867, + "max": 29.267, + "n": 20 + }, + { + "name": "ci.exec_overhead", + "unit": "ms", + "p50": 31.104, + "p95": 42.224, + "max": 45.888, + "n": 20 + }, + { + "name": "ci.exec_baseline", + "unit": "ms", + "p50": 1.045, + "p95": 1.193, + "max": 1.507, + "n": 20 + }, + { + "name": "ci.capacity_probe", + "unit": "ms", + "p50": 0.131, + "p95": 0.22, + "max": 0.236, + "n": 20 + }, + { + "name": "ci.e2e_tiny.v14", + "unit": "ms", + "p50": 294.071, + "p95": 319.802, + "max": 331.428, + "n": 20 + }, + { + "name": "ci.e2e_tiny.v15", + "unit": "ms", + "p50": 294.071, + "p95": 319.802, + "max": 331.428, + "n": 20 + }, + { + "name": "ci.e2e_medium.v14", + "unit": "ms", + "p50": 694.113, + "p95": 743.185, + "max": 820.547, + "n": 20 + }, + { + "name": "ci.e2e_medium.v15", + "unit": "ms", + "p50": 1463.806, + "p95": 1583.616, + "max": 1701.75, + "n": 20 + } + ] +} diff --git a/free/ci/bench/testdata/slow-result.json b/free/ci/bench/testdata/slow-result.json new file mode 100644 index 000000000..9529a03a7 --- /dev/null +++ b/free/ci/bench/testdata/slow-result.json @@ -0,0 +1,99 @@ +{ + "schema": "perfbench/v1", + "scenario": "ci-local-overhead", + "sha": "fixture", + "goos": "linux", + "goarch": "amd64", + "runs": 20, + "injected": false, + "metrics": [ + { + "name": "ci.startup", + "unit": "ms", + "p50": 74.49, + "p95": 102.81, + "max": 136.33, + "n": 20 + }, + { + "name": "ci.detect", + "unit": "ms", + "p50": 1.18, + "p95": 4.37, + "max": 11.53, + "n": 20 + }, + { + "name": "ci.plan", + "unit": "ms", + "p50": 0.43, + "p95": 0.71, + "max": 0.89, + "n": 20 + }, + { + "name": "ci.store_open", + "unit": "ms", + "p50": 244.77, + "p95": 278.67, + "max": 292.67, + "n": 20 + }, + { + "name": "ci.exec_overhead", + "unit": "ms", + "p50": 311.04, + "p95": 422.24, + "max": 458.88, + "n": 20 + }, + { + "name": "ci.exec_baseline", + "unit": "ms", + "p50": 10.45, + "p95": 11.93, + "max": 15.07, + "n": 20 + }, + { + "name": "ci.capacity_probe", + "unit": "ms", + "p50": 1.31, + "p95": 2.2, + "max": 2.36, + "n": 20 + }, + { + "name": "ci.e2e_tiny.v14", + "unit": "ms", + "p50": 2940.71, + "p95": 3198.02, + "max": 3314.28, + "n": 20 + }, + { + "name": "ci.e2e_tiny.v15", + "unit": "ms", + "p50": 13369.37, + "p95": 14392.83, + "max": 14969.77, + "n": 20 + }, + { + "name": "ci.e2e_medium.v14", + "unit": "ms", + "p50": 6941.13, + "p95": 7431.85, + "max": 8205.47, + "n": 20 + }, + { + "name": "ci.e2e_medium.v15", + "unit": "ms", + "p50": 14638.06, + "p95": 15836.16, + "max": 17017.5, + "n": 20 + } + ] +} diff --git a/free/ci/bench/testdata/tiny/.nself/ci.yaml.fixture b/free/ci/bench/testdata/tiny/.nself/ci.yaml.fixture index ca3d9b0b9..7a90604fc 100644 --- a/free/ci/bench/testdata/tiny/.nself/ci.yaml.fixture +++ b/free/ci/bench/testdata/tiny/.nself/ci.yaml.fixture @@ -5,12 +5,15 @@ jobs: fmt: kind: static run: [gofmt, -l, .] + requires: {mem_mb: 4096} vet: kind: static run: [go, vet, ./...] + requires: {mem_mb: 4096} test: kind: test run: [go, test, -count=1, ./...] + requires: {mem_mb: 4096} pipelines: gate: triggers: [manual] From d666fab3640d862cda2f0740650eee296a161b19 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 21:28:20 -0400 Subject: [PATCH 03/19] feat(ci): baseline budgets from the worst of three runs (P7-CI-46) --- free/ci/bench/budget.go | 28 +++++++++++++++++++++ free/ci/bench/budget_test.go | 28 +++++++++++++++++++++ free/ci/bench/cmd/cibench/main.go | 15 ++++++++--- free/ci/bench/perf-budget-darwin-arm64.json | 16 ++++++------ free/ci/bench/perf-budget.json | 16 ++++++------ 5 files changed, 84 insertions(+), 19 deletions(-) create mode 100644 free/ci/bench/budget_test.go diff --git a/free/ci/bench/budget.go b/free/ci/bench/budget.go index ed218ebda..cafccc591 100644 --- a/free/ci/bench/budget.go +++ b/free/ci/bench/budget.go @@ -29,6 +29,34 @@ type Budget struct { P95MS map[string]float64 `json:"p95_ms"` } +// WorstOf folds independent baseline runs of one platform into one result whose +// p50, p95 and max are, per metric, the highest seen in any run. One run of 20 +// samples under-states the p95 of a noisy machine; the worst of several is the +// baseline of record. The runs must come from one goos/goarch. +func WorstOf(runs []Result) (Result, error) { + if len(runs) == 0 { + return Result{}, fmt.Errorf("no baseline runs") + } + out := runs[0] + out.Metrics = append([]Metric(nil), runs[0].Metrics...) + for _, r := range runs[1:] { + if r.GOOS != out.GOOS || r.GOARCH != out.GOARCH { + return Result{}, fmt.Errorf("baseline runs mix %s/%s and %s/%s", out.GOOS, out.GOARCH, r.GOOS, r.GOARCH) + } + for _, m := range r.Metrics { + for i := range out.Metrics { + if out.Metrics[i].Name == m.Name { + out.Metrics[i].P50 = math.Max(out.Metrics[i].P50, m.P50) + out.Metrics[i].P95 = math.Max(out.Metrics[i].P95, m.P95) + out.Metrics[i].Max = math.Max(out.Metrics[i].Max, m.Max) + out.Metrics[i].N = min(out.Metrics[i].N, m.N) + } + } + } + } + return out, nil +} + // BudgetFromResult derives a budget from a baseline. keep selects the metrics // that get a ceiling (nil keeps every metric measured in milliseconds). func BudgetFromResult(base Result, note string, keep func(name string) bool) (Budget, error) { diff --git a/free/ci/bench/budget_test.go b/free/ci/bench/budget_test.go new file mode 100644 index 000000000..269cf8dc8 --- /dev/null +++ b/free/ci/bench/budget_test.go @@ -0,0 +1,28 @@ +package bench + +import "testing" + +func TestWorstOf(t *testing.T) { + a := NewResult("", 20, []Metric{{Name: "x", Unit: "ms", P50: 1, P95: 5, Max: 6, N: 20}, {Name: "y", Unit: "ms", P95: 2, N: 20}}) + b := NewResult("", 20, []Metric{{Name: "x", Unit: "ms", P50: 2, P95: 4, Max: 9, N: 20}, {Name: "y", Unit: "ms", P95: 3, N: 10}}) + got, err := WorstOf([]Result{a, b}) + if err != nil { + t.Fatal(err) + } + x, _ := got.Find("x") + y, _ := got.Find("y") + if x.P50 != 2 || x.P95 != 5 || x.Max != 9 || y.P95 != 3 || y.N != 10 { + t.Fatalf("%+v %+v", x, y) + } + if a.Metrics[0].P50 != 1 { + t.Fatal("WorstOf changed its input") + } + other := b + other.GOARCH = "riscv64" + if _, err := WorstOf([]Result{a, other}); err == nil { + t.Fatal("runs from two platforms were folded") + } + if _, err := WorstOf(nil); err == nil { + t.Fatal("no runs accepted") + } +} diff --git a/free/ci/bench/cmd/cibench/main.go b/free/ci/bench/cmd/cibench/main.go index bfbfdf73b..8f3162e40 100644 --- a/free/ci/bench/cmd/cibench/main.go +++ b/free/ci/bench/cmd/cibench/main.go @@ -4,7 +4,7 @@ // // cibench [-runs 10] [-warmup 2] [-bin path] [-json] [-services [-cold]] // cibench -ratio-check -in result.json [-exceptions file] -// cibench -write-budget perf-budget.json -in result.json [-only-core] +// cibench -write-budget perf-budget.json -in run1.json,run2.json,run3.json [-only-core] // // Run it from the free/ci module root; without -bin it builds ./cmd first. // Results go to stdout with -json (everything else goes to stderr). Exit 0 on @@ -44,7 +44,7 @@ func realMain(args []string, stdout, stderr io.Writer) int { fs.StringVar(&o.bin, "bin", "", "nself-ci binary (default: build ./cmd)") fs.StringVar(&o.nself, "nself", "nself", "nself CLI for -services") fs.StringVar(&o.sha, "sha", "", "revision measured (default: git HEAD of the working directory)") - fs.StringVar(&o.in, "in", "", "perfbench/v1 result file (-ratio-check, -write-budget)") + fs.StringVar(&o.in, "in", "", "perfbench/v1 result file (-ratio-check); with -write-budget a comma list of baseline runs, worst p95 per metric") fs.StringVar(&o.out, "out", "", "write the result JSON here instead of stdout") fs.StringVar(&o.exceptions, "exceptions", "bench/ratio-exceptions.json", "owner-accepted ratio exceptions") fs.StringVar(&o.writeBudget, "write-budget", "", "derive a budget file from -in and write it here") @@ -102,7 +102,16 @@ func budgetCmd(o options, stderr io.Writer) int { say(stderr, "cibench: -write-budget needs -in") return 2 } - res, err := bench.ReadResult(o.in) + var runs []bench.Result + for _, path := range strings.Split(o.in, ",") { + r, err := bench.ReadResult(path) + if err != nil { + say(stderr, "cibench: %v", err) + return 2 + } + runs = append(runs, r) + } + res, err := bench.WorstOf(runs) if err != nil { say(stderr, "cibench: %v", err) return 2 diff --git a/free/ci/bench/perf-budget-darwin-arm64.json b/free/ci/bench/perf-budget-darwin-arm64.json index 6ba0e2689..ba53fd539 100644 --- a/free/ci/bench/perf-budget-darwin-arm64.json +++ b/free/ci/bench/perf-budget-darwin-arm64.json @@ -1,16 +1,16 @@ { - "_note": "Absolute p95 ceilings in ms: the recorded baseline p95 plus 25 percent (P7-CI-46), checked by cli tools/perfbench check. Baseline and update rule: hq plan/p7/evidence/CI-46-perf.md. The relative V15/v1.4 budget is checked by cibench -ratio-check.", + "_note": "Absolute p95 ceilings in ms: the highest p95 of three independent 20-sample baseline runs plus 25 percent (P7-CI-46), checked by cli tools/perfbench check. Baselines and update rule: hq plan/p7/evidence/CI-46-perf.md. The relative V15/v1.4 budget is checked by cibench -ratio-check.", "schema": "perfbench-budget/v1", "platform": "darwin/arm64", "p95_ms": { "ci.detect": 1, - "ci.e2e_medium.v14": 1453, - "ci.e2e_medium.v15": 2876, - "ci.e2e_tiny.v14": 555, - "ci.e2e_tiny.v15": 2279, - "ci.exec_overhead": 64, + "ci.e2e_medium.v14": 1915, + "ci.e2e_medium.v15": 5124, + "ci.e2e_tiny.v14": 949, + "ci.e2e_tiny.v15": 4308, + "ci.exec_overhead": 207, "ci.plan": 1, - "ci.startup": 21, - "ci.store_open": 16 + "ci.startup": 37, + "ci.store_open": 230 } } diff --git a/free/ci/bench/perf-budget.json b/free/ci/bench/perf-budget.json index 68a57c1b8..76d20aa6f 100644 --- a/free/ci/bench/perf-budget.json +++ b/free/ci/bench/perf-budget.json @@ -1,16 +1,16 @@ { - "_note": "Absolute p95 ceilings in ms: the recorded baseline p95 plus 25 percent (P7-CI-46), checked by cli tools/perfbench check. Baseline and update rule: hq plan/p7/evidence/CI-46-perf.md. The relative V15/v1.4 budget is checked by cibench -ratio-check.", + "_note": "Absolute p95 ceilings in ms: the highest p95 of three independent 20-sample baseline runs plus 25 percent (P7-CI-46), checked by cli tools/perfbench check. Baselines and update rule: hq plan/p7/evidence/CI-46-perf.md. The relative V15/v1.4 budget is checked by cibench -ratio-check.", "schema": "perfbench-budget/v1", "platform": "linux/amd64", "p95_ms": { "ci.detect": 1, - "ci.e2e_medium.v14": 929, - "ci.e2e_medium.v15": 1980, - "ci.e2e_tiny.v14": 400, - "ci.e2e_tiny.v15": 1800, - "ci.exec_overhead": 53, + "ci.e2e_medium.v14": 946, + "ci.e2e_medium.v15": 2008, + "ci.e2e_tiny.v14": 431, + "ci.e2e_tiny.v15": 1816, + "ci.exec_overhead": 59, "ci.plan": 1, - "ci.startup": 13, - "ci.store_open": 35 + "ci.startup": 14, + "ci.store_open": 58 } } From a29848c98eb901b8d27db23f6362ecebbd7381db Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 21:41:12 -0400 Subject: [PATCH 04/19] fix(ci): review fixes for the bench harness, hosted baselines, one Go toolchain (P7-CI-46) --- .github/workflows/ci-bench.yml | 18 +++++++++----- free/ci/bench/budget.go | 26 +++++++++++++++------ free/ci/bench/budget_test.go | 5 ++++ free/ci/bench/cmd/cibench/main.go | 4 +++- free/ci/bench/perf-budget-darwin-arm64.json | 6 ++--- free/ci/bench/perf-budget.json | 10 ++++---- free/ci/bench/result_test.go | 6 ++++- free/ci/bench/services.go | 19 +++++++++++---- free/ci/bench/timeit.go | 5 ++++ 9 files changed, 71 insertions(+), 28 deletions(-) diff --git a/.github/workflows/ci-bench.yml b/.github/workflows/ci-bench.yml index 65895b768..d087cce57 100644 --- a/.github/workflows/ci-bench.yml +++ b/.github/workflows/ci-bench.yml @@ -59,10 +59,12 @@ jobs: path: cli persist-credentials: false + # The cli module needs a newer Go than the plugin pins, and setup-go sets + # GOTOOLCHAIN=local, so one toolchain (the newer) builds both. - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7 with: - go-version-file: free/ci/go.mod - cache-dependency-path: free/ci/go.sum + go-version-file: cli/go.mod + cache: false - name: Harness tests working-directory: free/ci @@ -83,12 +85,16 @@ jobs: - name: Control - the gates fail an injected slowdown and pass a good result run: | set -u - if (cd cli && go run -mod=vendor ./tools/perfbench check -budget "$BUDGET" -in "$SLOW"); then - echo "::error::perfbench check passed a result ten times over budget"; exit 1 + rc=0 + (cd cli && go run -mod=vendor ./tools/perfbench check -budget "$BUDGET" -in "$SLOW") || rc=$? + if [ "$rc" -ne 1 ]; then + echo "::error::perfbench check exited $rc on a result ten times over budget; want 1 (a violation)"; exit 1 fi cd free/ci - if go run -mod=vendor ./bench/cmd/cibench -ratio-check -in "$SLOW"; then - echo "::error::the ratio check passed a result ten times over budget"; exit 1 + rc=0 + go run -mod=vendor ./bench/cmd/cibench -ratio-check -in "$SLOW" || rc=$? + if [ "$rc" -ne 1 ]; then + echo "::error::the ratio check exited $rc on a result ten times over budget; want 1 (a violation)"; exit 1 fi go run -mod=vendor ./bench/cmd/cibench -ratio-check -in "$OK" diff --git a/free/ci/bench/budget.go b/free/ci/bench/budget.go index cafccc591..993f23ddb 100644 --- a/free/ci/bench/budget.go +++ b/free/ci/bench/budget.go @@ -4,7 +4,7 @@ package bench // derivation from a recorded baseline. // Inputs: a baseline Result, a headroom factor, a metric filter. // Outputs: a Budget whose absolute p95 ceilings are the baseline p95 plus the -// headroom, rounded up to a whole millisecond. +// headroom, rounded up to a whole millisecond, never below MinCeilingMS. // Constraints: the field names are the ones `perfbench check` decodes with // DisallowUnknownFields; a metric that is not in the baseline is never given a // ceiling (an invented number is the one thing this Ticket must not ship). @@ -21,6 +21,11 @@ const BudgetSchemaV1 = "perfbench-budget/v1" // Headroom is the allowed growth over the baseline p95 (Ticket constraint: +25%). const Headroom = 1.25 +// MinCeilingMS is the smallest ceiling a budget gets. Below it a timing is +// scheduler noise on a shared runner (a 0.3 ms planner run can take 3 ms), so a +// 25 percent rule on such a baseline would fail on weather. +const MinCeilingMS = 5.0 + // Budget is the budget document. P95MS maps a metric name to its p95 ceiling in ms. type Budget struct { Note string `json:"_note"` @@ -43,13 +48,20 @@ func WorstOf(runs []Result) (Result, error) { if r.GOOS != out.GOOS || r.GOARCH != out.GOARCH { return Result{}, fmt.Errorf("baseline runs mix %s/%s and %s/%s", out.GOOS, out.GOARCH, r.GOOS, r.GOARCH) } + if len(r.Metrics) != len(out.Metrics) { + return Result{}, fmt.Errorf("baseline runs carry different metrics (%d and %d)", len(out.Metrics), len(r.Metrics)) + } for _, m := range r.Metrics { + base, ok := out.Find(m.Name) + if !ok { + return Result{}, fmt.Errorf("metric %s is missing from the first baseline run", m.Name) + } for i := range out.Metrics { - if out.Metrics[i].Name == m.Name { - out.Metrics[i].P50 = math.Max(out.Metrics[i].P50, m.P50) - out.Metrics[i].P95 = math.Max(out.Metrics[i].P95, m.P95) - out.Metrics[i].Max = math.Max(out.Metrics[i].Max, m.Max) - out.Metrics[i].N = min(out.Metrics[i].N, m.N) + if out.Metrics[i].Name == base.Name { + out.Metrics[i].P50 = math.Max(base.P50, m.P50) + out.Metrics[i].P95 = math.Max(base.P95, m.P95) + out.Metrics[i].Max = math.Max(base.Max, m.Max) + out.Metrics[i].N = min(base.N, m.N) } } } @@ -69,7 +81,7 @@ func BudgetFromResult(base Result, note string, keep func(name string) bool) (Bu if math.IsNaN(m.P95) || m.P95 <= 0 { return b, fmt.Errorf("metric %s: baseline p95 %v is not a usable timing", m.Name, m.P95) } - b.P95MS[m.Name] = math.Ceil(m.P95 * Headroom) + b.P95MS[m.Name] = math.Max(MinCeilingMS, math.Ceil(m.P95*Headroom)) } if len(b.P95MS) == 0 { return b, fmt.Errorf("baseline has no metric to budget") diff --git a/free/ci/bench/budget_test.go b/free/ci/bench/budget_test.go index 269cf8dc8..241ac9d94 100644 --- a/free/ci/bench/budget_test.go +++ b/free/ci/bench/budget_test.go @@ -22,6 +22,11 @@ func TestWorstOf(t *testing.T) { if _, err := WorstOf([]Result{a, other}); err == nil { t.Fatal("runs from two platforms were folded") } + short := NewResult("", 20, []Metric{{Name: "x", Unit: "ms", P95: 1, N: 20}}) + short.GOOS, short.GOARCH = a.GOOS, a.GOARCH + if _, err := WorstOf([]Result{a, short}); err == nil { + t.Fatal("a run with a missing metric was folded") + } if _, err := WorstOf(nil); err == nil { t.Fatal("no runs accepted") } diff --git a/free/ci/bench/cmd/cibench/main.go b/free/ci/bench/cmd/cibench/main.go index 8f3162e40..61c7aeaad 100644 --- a/free/ci/bench/cmd/cibench/main.go +++ b/free/ci/bench/cmd/cibench/main.go @@ -17,8 +17,10 @@ import ( "fmt" "io" "os" + "os/signal" "path/filepath" "strings" + "syscall" "time" "github.com/nself-org/plugins/free/ci/bench" @@ -140,7 +142,7 @@ var informational = map[string]bool{bench.MetricExecBaseline: true, bench.Metric // runCmd measures and prints the result. func runCmd(o options, stdout, stderr io.Writer) int { - ctx, stop := context.WithCancel(context.Background()) + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer stop() dir, err := os.MkdirTemp(os.Getenv("TMPDIR"), "cibench-") if err != nil { diff --git a/free/ci/bench/perf-budget-darwin-arm64.json b/free/ci/bench/perf-budget-darwin-arm64.json index ba53fd539..c7a413bdf 100644 --- a/free/ci/bench/perf-budget-darwin-arm64.json +++ b/free/ci/bench/perf-budget-darwin-arm64.json @@ -1,15 +1,15 @@ { - "_note": "Absolute p95 ceilings in ms: the highest p95 of three independent 20-sample baseline runs plus 25 percent (P7-CI-46), checked by cli tools/perfbench check. Baselines and update rule: hq plan/p7/evidence/CI-46-perf.md. The relative V15/v1.4 budget is checked by cibench -ratio-check.", + "_note": "Absolute p95 ceilings in ms: the highest p95 of the recorded 20-sample baseline runs (three on the local host, one on the hosted runner) plus 25 percent, never below 5 ms (P7-CI-46), checked by cli tools/perfbench check. Baselines and update rule: hq plan/p7/evidence/CI-46-perf.md. The relative V15/v1.4 budget is checked by cibench -ratio-check.", "schema": "perfbench-budget/v1", "platform": "darwin/arm64", "p95_ms": { - "ci.detect": 1, + "ci.detect": 5, "ci.e2e_medium.v14": 1915, "ci.e2e_medium.v15": 5124, "ci.e2e_tiny.v14": 949, "ci.e2e_tiny.v15": 4308, "ci.exec_overhead": 207, - "ci.plan": 1, + "ci.plan": 5, "ci.startup": 37, "ci.store_open": 230 } diff --git a/free/ci/bench/perf-budget.json b/free/ci/bench/perf-budget.json index 76d20aa6f..0ac393caa 100644 --- a/free/ci/bench/perf-budget.json +++ b/free/ci/bench/perf-budget.json @@ -1,15 +1,15 @@ { - "_note": "Absolute p95 ceilings in ms: the highest p95 of three independent 20-sample baseline runs plus 25 percent (P7-CI-46), checked by cli tools/perfbench check. Baselines and update rule: hq plan/p7/evidence/CI-46-perf.md. The relative V15/v1.4 budget is checked by cibench -ratio-check.", + "_note": "Absolute p95 ceilings in ms: the highest p95 of the recorded 20-sample baseline runs (three on the local host, one on the hosted runner) plus 25 percent, never below 5 ms (P7-CI-46), checked by cli tools/perfbench check. Baselines and update rule: hq plan/p7/evidence/CI-46-perf.md. The relative V15/v1.4 budget is checked by cibench -ratio-check.", "schema": "perfbench-budget/v1", "platform": "linux/amd64", "p95_ms": { - "ci.detect": 1, + "ci.detect": 5, "ci.e2e_medium.v14": 946, - "ci.e2e_medium.v15": 2008, + "ci.e2e_medium.v15": 3616, "ci.e2e_tiny.v14": 431, - "ci.e2e_tiny.v15": 1816, + "ci.e2e_tiny.v15": 2440, "ci.exec_overhead": 59, - "ci.plan": 1, + "ci.plan": 5, "ci.startup": 14, "ci.store_open": 58 } diff --git a/free/ci/bench/result_test.go b/free/ci/bench/result_test.go index 56684c675..c4b852643 100644 --- a/free/ci/bench/result_test.go +++ b/free/ci/bench/result_test.go @@ -102,7 +102,7 @@ func TestBudgetFromResult(t *testing.T) { if err != nil { t.Fatal(err) } - if b.Schema != BudgetSchemaV1 || b.P95MS["ci.startup"] != 13 || b.P95MS["ci.plan"] != 1 { + if b.Schema != BudgetSchemaV1 || b.P95MS["ci.startup"] != 13 || b.P95MS["ci.plan"] != MinCeilingMS { t.Fatalf("%+v", b) } if _, ok := b.P95MS["ci.bytes"]; ok { @@ -114,6 +114,10 @@ func TestBudgetFromResult(t *testing.T) { if got := strings.Join(b.Names(), ","); got != "ci.plan,ci.startup" { t.Fatalf("names %s", got) } + big := NewResult("", 1, []Metric{Summarize("ci.big", "ms", []float64{100})}) + if bb, err := BudgetFromResult(big, "", nil); err != nil || bb.P95MS["ci.big"] != 125 { + t.Fatalf("100 ms baseline + 25%%: %+v %v", bb, err) + } zero := NewResult("", 1, []Metric{{Name: "ci.x", Unit: "ms"}}) if _, err := BudgetFromResult(zero, "", nil); err == nil { t.Fatal("a zero p95 was given a ceiling") diff --git a/free/ci/bench/services.go b/free/ci/bench/services.go index 88fdaf2a5..92bee0896 100644 --- a/free/ci/bench/services.go +++ b/free/ci/bench/services.go @@ -41,7 +41,11 @@ func measureServices(ctx context.Context, cfg Config, home string, col *Collecto if err != nil { return 0, "", fmt.Errorf("service %s: %w", comp, err) } - images := imagesOf(ctx, h.Project) + images, err := imagesOf(ctx, h.Project) + if err != nil { + _ = prov.Down(ctx, h) + return 0, "", err + } if err := prov.Down(ctx, h); err != nil { return 0, "", fmt.Errorf("service %s teardown: %w", comp, err) } @@ -80,12 +84,14 @@ func measureServices(ctx context.Context, cfg Config, home string, col *Collecto return nil } -// imagesOf lists the images of the containers of compose project name. -func imagesOf(ctx context.Context, project string) string { +// imagesOf lists the images of the containers of compose project name. A +// project with no container is an error: the cold scenario would otherwise +// remove nothing and record a warm start as a cold one. +func imagesOf(ctx context.Context, project string) (string, error) { r, err := must(ctx, []string{"docker", "ps", "-a", "--filter", "label=com.docker.compose.project=" + project, "--format", "{{.Image}}"}, "", baseEnv()) if err != nil { - return "" + return "", fmt.Errorf("list the images of project %s: %w", project, err) } seen := map[string]bool{} var out []string @@ -95,5 +101,8 @@ func imagesOf(ctx context.Context, project string) string { out = append(out, img) } } - return strings.Join(out, " ") + if len(out) == 0 { + return "", fmt.Errorf("project %s has no container, so its image is unknown", project) + } + return strings.Join(out, " "), nil } diff --git a/free/ci/bench/timeit.go b/free/ci/bench/timeit.go index ed80af664..6b421e3d5 100644 --- a/free/ci/bench/timeit.go +++ b/free/ci/bench/timeit.go @@ -1,5 +1,10 @@ package bench +// Purpose: time one iteration of a scenario. +// Inputs: the scenario function. +// Outputs: its wall time in milliseconds and its error. +// Constraints: wall clock only; no warm-up logic (the caller discards warm-up runs). + import "time" // timeIt runs fn once and returns its wall time in milliseconds. From a4642eaa9c5e2c17de7305ec7b1d2ff690d6cfc7 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 21:50:52 -0400 Subject: [PATCH 05/19] ci: run controls and service scenarios even when a gate step fails (P7-CI-46) --- .github/workflows/ci-bench.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci-bench.yml b/.github/workflows/ci-bench.yml index d087cce57..cfb64fb02 100644 --- a/.github/workflows/ci-bench.yml +++ b/.github/workflows/ci-bench.yml @@ -83,6 +83,7 @@ jobs: run: go run -mod=vendor ./bench/cmd/cibench -ratio-check -in "$RUNNER_TEMP/ci-bench.json" - name: Control - the gates fail an injected slowdown and pass a good result + if: ${{ !cancelled() }} run: | set -u rc=0 @@ -99,7 +100,7 @@ jobs: go run -mod=vendor ./bench/cmd/cibench -ratio-check -in "$OK" - name: Service start (warm and cold) - if: matrix.services + if: ${{ !cancelled() && matrix.services }} run: | set -eu (cd cli && go build -mod=vendor -o "$RUNNER_TEMP/nself" ./cmd/nself) From 7ba046ad1aac600f84d76dd61ec545c6ddaa1648 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 22:05:03 -0400 Subject: [PATCH 06/19] fix(ci): an image that is already gone counts as cold in the service scenario (P7-CI-46) --- free/ci/bench/services.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/free/ci/bench/services.go b/free/ci/bench/services.go index 92bee0896..cb5c6c589 100644 --- a/free/ci/bench/services.go +++ b/free/ci/bench/services.go @@ -71,9 +71,14 @@ func measureServices(ctx context.Context, cfg Config, home string, col *Collecto } for i := 0; i < n; i++ { for _, img := range strings.Fields(images) { - if _, err := must(ctx, []string{"docker", "image", "rm", img}, cfg.Dir, baseEnv()); err != nil { + r, err := spawn(ctx, []string{"docker", "image", "rm", img}, cfg.Dir, baseEnv()) + if err != nil { return fmt.Errorf("cold start needs the image removed: %w", err) } + // An image that is already gone is the cold state this scenario wants. + if r.ExitCode != 0 && !strings.Contains(r.Tail, "No such image") { + return fmt.Errorf("cold start needs the image removed: docker image rm %s: exit %d: %s", img, r.ExitCode, r.Tail) + } } ms, _, err := up() if err != nil { From bf559f7db0ce415ebff91517900027f08dbcd6e4 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 22:18:43 -0400 Subject: [PATCH 07/19] fix(ci): classify service samples warm or cold by the observed image state (P7-CI-46) --- free/ci/bench/services.go | 73 +++++++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 22 deletions(-) diff --git a/free/ci/bench/services.go b/free/ci/bench/services.go index cb5c6c589..67f69f2f7 100644 --- a/free/ci/bench/services.go +++ b/free/ci/bench/services.go @@ -55,40 +55,69 @@ func measureServices(ctx context.Context, cfg Config, home string, col *Collecto if err != nil { return err } - for i := 0; i < cfg.Runs; i++ { - ms, _, err := up() - if err != nil { - return err + coldWant := 0 + if sc.Cold { + if coldWant = sc.ColdRuns; coldWant < 1 { + coldWant = cfg.Runs } - col.Add(MetricServiceWarm, "ms", ms) - } - if !sc.Cold { - return nil - } - n := sc.ColdRuns - if n < 1 { - n = cfg.Runs } - for i := 0; i < n; i++ { - for _, img := range strings.Fields(images) { - r, err := spawn(ctx, []string{"docker", "image", "rm", img}, cfg.Dir, baseEnv()) - if err != nil { - return fmt.Errorf("cold start needs the image removed: %w", err) - } - // An image that is already gone is the cold state this scenario wants. - if r.ExitCode != 0 && !strings.Contains(r.Tail, "No such image") { - return fmt.Errorf("cold start needs the image removed: docker image rm %s: exit %d: %s", img, r.ExitCode, r.Tail) + // A sample is warm or cold by what docker says about the images right before + // the start, not by what this loop intended: if the teardown removes the + // images, every start is cold and no warm sample is recorded (and that is + // reported below), instead of a cold start being filed as warm. + warm, cold := 0, 0 + for try := 0; try < 3*(cfg.Runs+coldWant) && (warm < cfg.Runs || cold < coldWant); try++ { + if coldWant > 0 && cold < coldWant && (warm >= cfg.Runs || try%2 == 1) { + if err := removeImages(ctx, cfg.Dir, images); err != nil { + return err } } + present := imagesPresent(ctx, cfg.Dir, images) ms, _, err := up() if err != nil { return err } - col.Add(MetricServiceCold, "ms", ms) + switch { + case present && warm < cfg.Runs: + col.Add(MetricServiceWarm, "ms", ms) + warm++ + case !present && cold < coldWant: + col.Add(MetricServiceCold, "ms", ms) + cold++ + } + } + if warm < cfg.Runs || cold < coldWant { + return fmt.Errorf("service scenarios recorded %d warm and %d cold samples, wanted %d and %d (images %s)", + warm, cold, cfg.Runs, coldWant, images) } return nil } +// removeImages removes the images; one that is already gone is the cold state. +func removeImages(ctx context.Context, dir, images string) error { + for _, img := range strings.Fields(images) { + r, err := spawn(ctx, []string{"docker", "image", "rm", img}, dir, baseEnv()) + if err != nil { + return fmt.Errorf("cold start needs the image removed: %w", err) + } + if r.ExitCode != 0 && !strings.Contains(r.Tail, "No such image") { + return fmt.Errorf("cold start needs the image removed: docker image rm %s: exit %d: %s", img, r.ExitCode, r.Tail) + } + } + return nil +} + +// imagesPresent reports whether every image is in the local store. +func imagesPresent(ctx context.Context, dir, images string) bool { + for _, img := range strings.Fields(images) { + r, err := spawn(ctx, []string{"docker", "image", "inspect", "--format", "{{.Id}}", img}, dir, baseEnv()) + if err != nil || r.ExitCode != 0 { + return false + } + } + return true +} + // imagesOf lists the images of the containers of compose project name. A // project with no container is an error: the cold scenario would otherwise // remove nothing and record a warm start as a cold one. From 73dc8bee370762300be34d3f31b553cb72716250 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 22:40:12 -0400 Subject: [PATCH 08/19] ci: log each service sample and bound the service scenario (P7-CI-46) --- .github/workflows/ci-bench.yml | 2 +- free/ci/bench/services.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci-bench.yml b/.github/workflows/ci-bench.yml index cfb64fb02..9db8d6477 100644 --- a/.github/workflows/ci-bench.yml +++ b/.github/workflows/ci-bench.yml @@ -105,7 +105,7 @@ jobs: set -eu (cd cli && go build -mod=vendor -o "$RUNNER_TEMP/nself" ./cmd/nself) cd free/ci - go run -mod=vendor ./bench/cmd/cibench -runs 10 -warmup 0 -skip-medium -services -cold \ + go run -mod=vendor ./bench/cmd/cibench -runs 5 -warmup 0 -skip-medium -services -cold -cold-runs 3 \ -nself "$RUNNER_TEMP/nself" -out "$RUNNER_TEMP/ci-bench-services.json" - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 diff --git a/free/ci/bench/services.go b/free/ci/bench/services.go index 67f69f2f7..6b3b83fb4 100644 --- a/free/ci/bench/services.go +++ b/free/ci/bench/services.go @@ -66,7 +66,7 @@ func measureServices(ctx context.Context, cfg Config, home string, col *Collecto // images, every start is cold and no warm sample is recorded (and that is // reported below), instead of a cold start being filed as warm. warm, cold := 0, 0 - for try := 0; try < 3*(cfg.Runs+coldWant) && (warm < cfg.Runs || cold < coldWant); try++ { + for try := 0; try < 2*(cfg.Runs+coldWant) && (warm < cfg.Runs || cold < coldWant); try++ { if coldWant > 0 && cold < coldWant && (warm >= cfg.Runs || try%2 == 1) { if err := removeImages(ctx, cfg.Dir, images); err != nil { return err @@ -77,6 +77,7 @@ func measureServices(ctx context.Context, cfg Config, home string, col *Collecto if err != nil { return err } + cfg.logf("service try %d: images present before start: %v, %.0f ms", try+1, present, ms) switch { case present && warm < cfg.Runs: col.Add(MetricServiceWarm, "ms", ms) From dc6e9fa4e3f539ebc8c582bffca54c8674b6e77a Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 22:57:26 -0400 Subject: [PATCH 09/19] ci: log the local image list after the first service start (P7-CI-46) --- free/ci/bench/services.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/free/ci/bench/services.go b/free/ci/bench/services.go index 6b3b83fb4..a1314a526 100644 --- a/free/ci/bench/services.go +++ b/free/ci/bench/services.go @@ -55,6 +55,9 @@ func measureServices(ctx context.Context, cfg Config, home string, col *Collecto if err != nil { return err } + if r, e := must(ctx, []string{"docker", "image", "ls", "--no-trunc", "--format", "{{.Repository}}:{{.Tag}} {{.ID}}"}, cfg.Dir, baseEnv()); e == nil { + cfg.logf("images after the first start (project images: %s):\n%s", images, r.Tail) + } coldWant := 0 if sc.Cold { if coldWant = sc.ColdRuns; coldWant < 1 { From 8a02bf006a92d1d3fca0b315e29c41df25b08855 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 23:39:55 -0400 Subject: [PATCH 10/19] fix(ci): service scenario identifies images by ID, not by tag (P7-CI-46) --- free/ci/bench/services.go | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/free/ci/bench/services.go b/free/ci/bench/services.go index a1314a526..bc6ea8d6b 100644 --- a/free/ci/bench/services.go +++ b/free/ci/bench/services.go @@ -122,21 +122,27 @@ func imagesPresent(ctx context.Context, dir, images string) bool { return true } -// imagesOf lists the images of the containers of compose project name. A -// project with no container is an error: the cold scenario would otherwise -// remove nothing and record a warm start as a cold one. +// imagesOf lists the image IDs of the containers of compose project name. It +// returns IDs, not names: the pulled image can be untagged locally +// (pgvector/pgvector:) while the container still names the tag, so a name +// finds nothing to inspect or remove. A project with no container is an error: +// the cold scenario would otherwise remove nothing and record a warm start as +// a cold one. func imagesOf(ctx context.Context, project string) (string, error) { - r, err := must(ctx, []string{"docker", "ps", "-a", "--filter", "label=com.docker.compose.project=" + project, - "--format", "{{.Image}}"}, "", baseEnv()) + r, err := must(ctx, []string{"docker", "ps", "-a", "-q", "--filter", "label=com.docker.compose.project=" + project}, "", baseEnv()) if err != nil { - return "", fmt.Errorf("list the images of project %s: %w", project, err) + return "", fmt.Errorf("list the containers of project %s: %w", project, err) } seen := map[string]bool{} var out []string - for _, img := range strings.Fields(r.Tail) { - if !seen[img] { - seen[img] = true - out = append(out, img) + for _, c := range strings.Fields(r.Tail) { + ir, err := must(ctx, []string{"docker", "inspect", "--format", "{{.Image}}", c}, "", baseEnv()) + if err != nil { + return "", fmt.Errorf("find the image of container %s: %w", c, err) + } + if id := strings.TrimSpace(ir.Tail); id != "" && !seen[id] { + seen[id] = true + out = append(out, id) } } if len(out) == 0 { From 87d3feace5f8f272bd8faa3f65f344b4b8ff4939 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Mon, 19 Oct 2026 00:06:12 -0400 Subject: [PATCH 11/19] fix(ci): no absolute ceiling on the frozen v1.4 gate; budgets cover every hosted baseline (P7-CI-46) --- free/ci/bench/cmd/cibench/main.go | 8 +++++++- free/ci/bench/cmd/cibench/main_test.go | 4 ++-- free/ci/bench/perf-budget-darwin-arm64.json | 6 ++---- free/ci/bench/perf-budget.json | 8 +++----- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/free/ci/bench/cmd/cibench/main.go b/free/ci/bench/cmd/cibench/main.go index 61c7aeaad..c3cd311c3 100644 --- a/free/ci/bench/cmd/cibench/main.go +++ b/free/ci/bench/cmd/cibench/main.go @@ -119,7 +119,13 @@ func budgetCmd(o options, stderr io.Writer) int { return 2 } keep := func(name string) bool { - return !(o.onlyCore && strings.HasPrefix(name, "ci.service_")) && !informational[name] + // The v1.4 gate is frozen: its absolute time is the yardstick of the + // relative budget, not something to guard (a noisy runner would fail a + // budget on code nobody changes). + if informational[name] || strings.HasSuffix(name, ".v14") { + return false + } + return !(o.onlyCore && strings.HasPrefix(name, "ci.service_")) } b, err := bench.BudgetFromResult(res, o.note, keep) if err != nil { diff --git a/free/ci/bench/cmd/cibench/main_test.go b/free/ci/bench/cmd/cibench/main_test.go index 2b95bb49b..eb467b6ae 100644 --- a/free/ci/bench/cmd/cibench/main_test.go +++ b/free/ci/bench/cmd/cibench/main_test.go @@ -72,8 +72,8 @@ func TestWriteBudget(t *testing.T) { if !strings.Contains(s, `"perfbench-budget/v1"`) || !strings.Contains(s, `"ci.e2e_tiny.v15": 625`) { t.Fatalf("budget:\n%s", s) } - if strings.Contains(s, "service_start") || strings.Contains(s, "exec_baseline") { - t.Fatalf("service and informational metrics must get no ceiling:\n%s", s) + if strings.Contains(s, "service_start") || strings.Contains(s, "exec_baseline") || strings.Contains(s, ".v14") { + t.Fatalf("service, informational and v1.4 metrics must get no ceiling:\n%s", s) } if got := realMain([]string{"-write-budget", out}, &so, &se); got != 2 { t.Fatalf("missing -in must be a usage error, got %d", got) diff --git a/free/ci/bench/perf-budget-darwin-arm64.json b/free/ci/bench/perf-budget-darwin-arm64.json index c7a413bdf..0cf41998f 100644 --- a/free/ci/bench/perf-budget-darwin-arm64.json +++ b/free/ci/bench/perf-budget-darwin-arm64.json @@ -1,12 +1,10 @@ { - "_note": "Absolute p95 ceilings in ms: the highest p95 of the recorded 20-sample baseline runs (three on the local host, one on the hosted runner) plus 25 percent, never below 5 ms (P7-CI-46), checked by cli tools/perfbench check. Baselines and update rule: hq plan/p7/evidence/CI-46-perf.md. The relative V15/v1.4 budget is checked by cibench -ratio-check.", + "_note": "Absolute p95 ceilings in ms for the V15 engine: the highest p95 of the recorded 20-sample baseline runs (three on the local host, three on the hosted runner) plus 25 percent, never below 5 ms (P7-CI-46), checked by cli tools/perfbench check. The frozen v1.4 gate has no ceiling; it is the yardstick of the relative budget. Baselines and update rule: hq plan/p7/evidence/CI-46-perf.md. The relative V15/v1.4 budget is checked by cibench -ratio-check.", "schema": "perfbench-budget/v1", "platform": "darwin/arm64", "p95_ms": { "ci.detect": 5, - "ci.e2e_medium.v14": 1915, - "ci.e2e_medium.v15": 5124, - "ci.e2e_tiny.v14": 949, + "ci.e2e_medium.v15": 5170, "ci.e2e_tiny.v15": 4308, "ci.exec_overhead": 207, "ci.plan": 5, diff --git a/free/ci/bench/perf-budget.json b/free/ci/bench/perf-budget.json index 0ac393caa..67620e48e 100644 --- a/free/ci/bench/perf-budget.json +++ b/free/ci/bench/perf-budget.json @@ -1,13 +1,11 @@ { - "_note": "Absolute p95 ceilings in ms: the highest p95 of the recorded 20-sample baseline runs (three on the local host, one on the hosted runner) plus 25 percent, never below 5 ms (P7-CI-46), checked by cli tools/perfbench check. Baselines and update rule: hq plan/p7/evidence/CI-46-perf.md. The relative V15/v1.4 budget is checked by cibench -ratio-check.", + "_note": "Absolute p95 ceilings in ms for the V15 engine: the highest p95 of the recorded 20-sample baseline runs (three on the local host, three on the hosted runner) plus 25 percent, never below 5 ms (P7-CI-46), checked by cli tools/perfbench check. The frozen v1.4 gate has no ceiling; it is the yardstick of the relative budget. Baselines and update rule: hq plan/p7/evidence/CI-46-perf.md. The relative V15/v1.4 budget is checked by cibench -ratio-check.", "schema": "perfbench-budget/v1", "platform": "linux/amd64", "p95_ms": { "ci.detect": 5, - "ci.e2e_medium.v14": 946, - "ci.e2e_medium.v15": 3616, - "ci.e2e_tiny.v14": 431, - "ci.e2e_tiny.v15": 2440, + "ci.e2e_medium.v15": 3733, + "ci.e2e_tiny.v15": 2541, "ci.exec_overhead": 59, "ci.plan": 5, "ci.startup": 14, From 0cc4cb518758ed1c67e01ea6efe35bdaeed1956c Mon Sep 17 00:00:00 2001 From: admin Date: Tue, 20 Oct 2026 07:58:39 -0400 Subject: [PATCH 12/19] fix(ci): repair stale branch admission, Android and file-size gates --- free/ci/cmd/build_v15_test.go | 1 + free/ci/internal/cache/specs/engine.go | 44 --------- free/ci/internal/cache/specs/engine_pack.go | 56 +++++++++++ free/ci/internal/cache/specs/remove.go | 77 +++++++++++++++ free/ci/internal/cache/specs/unpack.go | 68 ------------- .../dispatch/chokepoint_roles_test.go | 99 +++++++++++++++++++ free/ci/internal/dispatch/chokepoint_test.go | 14 ++- free/ci/internal/run/cache_hook.go | 16 --- free/ci/internal/run/cache_hook_image.go | 23 +++++ free/ci/internal/sbom/tool.go | 43 -------- free/ci/internal/sbom/tool_digest.go | 49 +++++++++ 11 files changed, 316 insertions(+), 174 deletions(-) create mode 100644 free/ci/internal/cache/specs/engine_pack.go create mode 100644 free/ci/internal/cache/specs/remove.go create mode 100644 free/ci/internal/dispatch/chokepoint_roles_test.go create mode 100644 free/ci/internal/run/cache_hook_image.go create mode 100644 free/ci/internal/sbom/tool_digest.go diff --git a/free/ci/cmd/build_v15_test.go b/free/ci/cmd/build_v15_test.go index 38d22f099..25be99c54 100644 --- a/free/ci/cmd/build_v15_test.go +++ b/free/ci/cmd/build_v15_test.go @@ -73,6 +73,7 @@ const requiredUnit = "version: 1\njobs:\n unit:\n kind: test\n run: [sh, func TestBuildV15Synthesized(t *testing.T) { repo := androidRepo(t, requiredUnit) t.Setenv("ANDROID_HOME", "/opt/sdk-fixture") + t.Setenv("ANDROID_SDK_ROOT", "") t.Setenv("JAVA_HOME", "") lane, err := planBuildLane(context.Background(), repo, "android", 15*time.Minute) if err != nil { diff --git a/free/ci/internal/cache/specs/engine.go b/free/ci/internal/cache/specs/engine.go index 12af07fca..1f937c305 100644 --- a/free/ci/internal/cache/specs/engine.go +++ b/free/ci/internal/cache/specs/engine.go @@ -285,47 +285,3 @@ func (e *Engine) refuse(ctx context.Context, r SaveRequest, err error) error { } return err } - -var errSizeCap = errors.New("specs: the packed cache exceeds the entry byte cap") - -// capWriter counts stored bytes and fails past the cap. -type capWriter struct { - w io.Writer - n int64 - cap int64 -} - -func (c *capWriter) Write(p []byte) (int, error) { - if c.n += int64(len(p)); c.n > c.cap { - return 0, errSizeCap - } - return c.w.Write(p) -} - -// putDir streams dir through tar and scan into the blob store. The local blob -// is a plain tar: disk is cheap and a restore is on a job's critical path, so a -// copy beats an inflate (D-1008). Compress only when a blob leaves the host. -// With inflate the scan also looks inside the gzip payloads of the files -// (Spec.InflateScan, D-1037). -func (e *Engine) putDir(ctx context.Context, base *os.Root, realm cas.Realm, rel string, budget int64, inflate bool) (string, int64, PackStats, error) { - pr, pw := io.Pipe() - var st PackStats - done := make(chan struct{}) - go func() { - defer close(done) - var err error - defer func() { _ = pw.CloseWithError(err) }() - sw := newScanWriter(&capWriter{w: pw, cap: budget}, e.Scanner) - var inf *inflater - if inflate { - inf = newInflater(e.Scanner) - } - if st, err = pack(ctx, base, rel, sw, cacheLimits(), inf); err == nil { - err = sw.Close() - } - }() - digest, size, err := e.Blobs.Put(ctx, realm, pr) - _ = pr.CloseWithError(err) - <-done - return digest, size, st, err -} diff --git a/free/ci/internal/cache/specs/engine_pack.go b/free/ci/internal/cache/specs/engine_pack.go new file mode 100644 index 000000000..394f2bb07 --- /dev/null +++ b/free/ci/internal/cache/specs/engine_pack.go @@ -0,0 +1,56 @@ +package specs + +// Stream bounded cache archives through value scanning into blob storage. + +import ( + "context" + "errors" + "io" + "os" + + "github.com/nself-org/plugins/free/ci/internal/blob/cas" +) + +var errSizeCap = errors.New("specs: the packed cache exceeds the entry byte cap") + +// capWriter counts stored bytes and fails past the cap. +type capWriter struct { + w io.Writer + n int64 + cap int64 +} + +func (c *capWriter) Write(p []byte) (int, error) { + if c.n += int64(len(p)); c.n > c.cap { + return 0, errSizeCap + } + return c.w.Write(p) +} + +// putDir streams dir through tar and scan into the blob store. The local blob +// is a plain tar: disk is cheap and a restore is on a job's critical path, so a +// copy beats an inflate (D-1008). Compress only when a blob leaves the host. +// With inflate the scan also looks inside the gzip payloads of the files +// (Spec.InflateScan, D-1037). +func (e *Engine) putDir(ctx context.Context, base *os.Root, realm cas.Realm, rel string, budget int64, inflate bool) (string, int64, PackStats, error) { + pr, pw := io.Pipe() + var st PackStats + done := make(chan struct{}) + go func() { + defer close(done) + var err error + defer func() { _ = pw.CloseWithError(err) }() + sw := newScanWriter(&capWriter{w: pw, cap: budget}, e.Scanner) + var inf *inflater + if inflate { + inf = newInflater(e.Scanner) + } + if st, err = pack(ctx, base, rel, sw, cacheLimits(), inf); err == nil { + err = sw.Close() + } + }() + digest, size, err := e.Blobs.Put(ctx, realm, pr) + _ = pr.CloseWithError(err) + <-done + return digest, size, st, err +} diff --git a/free/ci/internal/cache/specs/remove.go b/free/ci/internal/cache/specs/remove.go new file mode 100644 index 000000000..3ca5a174a --- /dev/null +++ b/free/ci/internal/cache/specs/remove.go @@ -0,0 +1,77 @@ +package specs + +// Remove cache trees without following links outside their pinned root. + +import ( + "errors" + "os" + "path" +) + +// RemoveTree deletes dir and everything in it, first making every directory +// owner-writable (a Go module cache holds read-only directories). +func RemoveTree(dir string) error { + if info, err := os.Lstat(dir); err == nil && info.Mode()&os.ModeSymlink != 0 { + return os.Remove(dir) // a link is unlinked, its target never walked + } + root, err := os.OpenRoot(dir) + if errors.Is(err, os.ErrNotExist) { + return nil + } + if err != nil { + return err + } + _ = chmodDirs(root, ".") + _ = root.Close() + return os.RemoveAll(dir) +} + +// RemoveIn deletes the entry rel of base, making directories owner-writable +// first, without ever leaving base: a rel that is a symlink out of base is +// refused (its link is removed, its target never touched). +func RemoveIn(base *os.Root, rel string) error { return removeIn(base, rel) } + +// removeIn deletes the directory rel of base (same permission rule as RemoveTree) +// without ever leaving base. +func removeIn(base *os.Root, rel string) error { + info, err := base.Lstat(rel) + if err == nil && info.Mode()&os.ModeSymlink != 0 { + return base.Remove(rel) // unlink it; never walk or chmod what it points at + } + if err == nil && info.IsDir() { + if afterLstat != nil { + afterLstat() + } + if sub, err := base.OpenRoot(rel); err == nil { + // OpenRoot follows an in-root symlink, so a directory swapped for one since the + // Lstat would be chmodded in its target's place: only the inode seen above is touched. + if got, err := sub.Stat("."); err == nil && os.SameFile(info, got) { + _ = chmodDirs(sub, ".") + } + _ = sub.Close() + } + } + return removeAll(base, rel) +} + +// afterLstat is a test seam between the Lstat and the OpenRoot of removeIn. +var afterLstat func() + +func chmodDirs(root *os.Root, dir string) error { + _ = root.Chmod(dir, 0o700) + f, err := root.Open(dir) + if err != nil { + return err + } + ents, err := f.ReadDir(-1) + _ = f.Close() + if err != nil { + return err + } + for _, e := range ents { + if e.IsDir() { + _ = chmodDirs(root, path.Join(dir, e.Name())) + } + } + return nil +} diff --git a/free/ci/internal/cache/specs/unpack.go b/free/ci/internal/cache/specs/unpack.go index f6bf0aa53..56f97c2fd 100644 --- a/free/ci/internal/cache/specs/unpack.go +++ b/free/ci/internal/cache/specs/unpack.go @@ -259,71 +259,3 @@ func settleFile(root *os.Root, name string, mode os.FileMode, at time.Time, opt _ = root.Chtimes(name, at, at) return nil } - -// RemoveTree deletes dir and everything in it, first making every directory -// owner-writable (a Go module cache holds read-only directories). -func RemoveTree(dir string) error { - if info, err := os.Lstat(dir); err == nil && info.Mode()&os.ModeSymlink != 0 { - return os.Remove(dir) // a link is unlinked, its target never walked - } - root, err := os.OpenRoot(dir) - if errors.Is(err, os.ErrNotExist) { - return nil - } - if err != nil { - return err - } - _ = chmodDirs(root, ".") - _ = root.Close() - return os.RemoveAll(dir) -} - -// RemoveIn deletes the entry rel of base, making directories owner-writable -// first, without ever leaving base: a rel that is a symlink out of base is -// refused (its link is removed, its target never touched). -func RemoveIn(base *os.Root, rel string) error { return removeIn(base, rel) } - -// removeIn deletes the directory rel of base (same permission rule as RemoveTree) -// without ever leaving base. -func removeIn(base *os.Root, rel string) error { - info, err := base.Lstat(rel) - if err == nil && info.Mode()&os.ModeSymlink != 0 { - return base.Remove(rel) // unlink it; never walk or chmod what it points at - } - if err == nil && info.IsDir() { - if afterLstat != nil { - afterLstat() - } - if sub, err := base.OpenRoot(rel); err == nil { - // OpenRoot follows an in-root symlink, so a directory swapped for one since the - // Lstat would be chmodded in its target's place: only the inode seen above is touched. - if got, err := sub.Stat("."); err == nil && os.SameFile(info, got) { - _ = chmodDirs(sub, ".") - } - _ = sub.Close() - } - } - return removeAll(base, rel) -} - -// afterLstat is a test seam between the Lstat and the OpenRoot of removeIn. -var afterLstat func() - -func chmodDirs(root *os.Root, dir string) error { - _ = root.Chmod(dir, 0o700) - f, err := root.Open(dir) - if err != nil { - return err - } - ents, err := f.ReadDir(-1) - _ = f.Close() - if err != nil { - return err - } - for _, e := range ents { - if e.IsDir() { - _ = chmodDirs(root, path.Join(dir, e.Name())) - } - } - return nil -} diff --git a/free/ci/internal/dispatch/chokepoint_roles_test.go b/free/ci/internal/dispatch/chokepoint_roles_test.go new file mode 100644 index 000000000..3c19fcd0d --- /dev/null +++ b/free/ci/internal/dispatch/chokepoint_roles_test.go @@ -0,0 +1,99 @@ +package dispatch + +import ( + "go/ast" + "go/token" + "go/types" + "path/filepath" + "strings" + "testing" + + "golang.org/x/tools/go/packages" +) + +// A role Grant is unrelated to lease admission. Resolve its object rather than +// allowlisting files or functions, which could also contain a lease bypass. +func isRoleGrant(obj types.Object) bool { + if obj == nil || obj.Name() != "Grant" || obj.Pkg() == nil { + return false + } + const base = "github.com/nself-org/plugins/free/ci/internal/" + switch obj.Pkg().Path() { + case base + "store": + _, ok := obj.(*types.TypeName) + return ok + case base + "roles": + fn, ok := obj.(*types.Func) + if !ok { + return false + } + sig, ok := fn.Type().(*types.Signature) + return ok && sig.Recv() != nil + } + return false +} + +func roleGrantPositions(t *testing.T, root string) map[string]map[token.Pos]bool { + t.Helper() + result := map[string]map[token.Pos]bool{} + // Planted modules deliberately contain unresolved identifiers. They retain + // the conservative syntactic scan, including their lease Grant violations. + if strings.Contains(filepath.ToSlash(root), "testdata/") { + return result + } + pkgs, err := packages.Load(&packages.Config{Dir: root, + Mode: packages.NeedName | packages.NeedFiles | packages.NeedSyntax | packages.NeedTypes | packages.NeedTypesInfo | packages.NeedImports | packages.NeedDeps}, "./...") + if err != nil { + t.Fatal(err) + } + for _, pkg := range pkgs { + if len(pkg.Errors) != 0 { + t.Fatalf("load %s: %v", pkg.PkgPath, pkg.Errors) + } + for _, objects := range []map[*ast.Ident]types.Object{pkg.TypesInfo.Uses, pkg.TypesInfo.Defs} { + for id, obj := range objects { + if !isRoleGrant(obj) { + continue + } + pos := pkg.Fset.Position(id.Pos()) + rel, err := filepath.Rel(root, pos.Filename) + if err != nil { + t.Fatal(err) + } + path := filepath.Clean(filepath.Join(root, rel)) + if result[path] == nil { + result[path] = map[token.Pos]bool{} + } + // scanChokepoint parses each file with a fresh FileSet (base position 1). + result[path][token.Pos(pos.Offset+1)] = true + } + } + } + return result +} + +func TestRoleGrantObjects(t *testing.T) { + const base = "github.com/nself-org/plugins/free/ci/internal/" + for _, tc := range []struct { + name, pkg string + method, want bool + }{ + {"role type", "store", false, true}, + {"role method", "roles", true, true}, + {"lease method", "lease", true, false}, + {"store method", "store", true, false}, + {"unrelated type", "other", false, false}, + } { + t.Run(tc.name, func(t *testing.T) { + pkg := types.NewPackage(base+tc.pkg, tc.pkg) + var obj types.Object = types.NewTypeName(token.NoPos, pkg, "Grant", nil) + if tc.method { + recv := types.NewVar(token.NoPos, pkg, "s", types.Typ[types.Int]) + obj = types.NewFunc(token.NoPos, pkg, "Grant", types.NewSignatureType(recv, nil, nil, nil, nil, false)) + } + if got := isRoleGrant(obj); got != tc.want { + t.Fatalf("isRoleGrant = %v, want %v", got, tc.want) + } + }) + } +} diff --git a/free/ci/internal/dispatch/chokepoint_test.go b/free/ci/internal/dispatch/chokepoint_test.go index bbc813f02..218f230f9 100644 --- a/free/ci/internal/dispatch/chokepoint_test.go +++ b/free/ci/internal/dispatch/chokepoint_test.go @@ -80,7 +80,7 @@ func ownerOf(decl ast.Decl) (string, *ast.Ident) { } // scanFile reports the uses in one parsed file that the allowlist does not cover. -func scanFile(f *ast.File, rel string, add func(string)) { +func scanFile(f *ast.File, rel string, grants map[token.Pos]bool, add func(string)) { dir := filepath.ToSlash(filepath.Dir(rel)) usesReflect := false for _, imp := range f.Imports { @@ -101,6 +101,9 @@ func scanFile(f *ast.File, rel string, add func(string)) { switch x := n.(type) { case *ast.SelectorExpr: name := x.Sel.Name + if grants[x.Sel.Pos()] { + return true + } switch { case reflective[name], name == "Method" && usesReflect: report(name) @@ -112,7 +115,7 @@ func scanFile(f *ast.File, rel string, add func(string)) { report(name) } case *ast.Ident: - if banned[x.Name] && x != declName && !(x.Name == "LocalAdmit" && localFiles[rel]) && (x.Name == "LocalAdmit" || !chokeFuncs[key]) { + if banned[x.Name] && !grants[x.Pos()] && x != declName && !(x.Name == "LocalAdmit" && localFiles[rel]) && (x.Name == "LocalAdmit" || !chokeFuncs[key]) { report(x.Name) } case *ast.BasicLit: @@ -129,6 +132,11 @@ func scanFile(f *ast.File, rel string, add func(string)) { // allowlist, and how many files it read. func scanChokepoint(t *testing.T, root string) (violations []string, files int) { t.Helper() + root, absErr := filepath.Abs(root) + if absErr != nil { + t.Fatal(absErr) + } + roleGrants := roleGrantPositions(t, root) seen := map[string]bool{} add := func(v string) { // a selector and its identifier are two nodes if !seen[v] { @@ -156,7 +164,7 @@ func scanChokepoint(t *testing.T, root string) (violations []string, files int) t.Fatalf("parse %s: %v", path, perr) } files++ - scanFile(f, filepath.ToSlash(rel), add) + scanFile(f, filepath.ToSlash(rel), roleGrants[filepath.Clean(path)], add) return nil }) if err != nil { diff --git a/free/ci/internal/run/cache_hook.go b/free/ci/internal/run/cache_hook.go index 11781b5bf..ea7576204 100644 --- a/free/ci/internal/run/cache_hook.go +++ b/free/ci/internal/run/cache_hook.go @@ -26,7 +26,6 @@ import ( "os" "path/filepath" "runtime" - "strings" "sync" "time" @@ -298,18 +297,3 @@ func (h *cacheHook) evict(ctx context.Context, st *store.Store, now time.Time) { return true }) } - -// imageDigest is the pinned digest of the job's container image, "" if unknown. -func (h *cacheHook) imageDigest(ctx context.Context, a AttemptInfo) string { - img := "" - if a.Job.Container != nil { - img = a.Job.Container.Image - } - if img == "" && Registered.Image != nil { - img, _ = Registered.Image.Image(ctx, a) - } - if _, d, ok := strings.Cut(img, "@"); ok && strings.HasPrefix(d, "sha256:") { - return d - } - return "" -} diff --git a/free/ci/internal/run/cache_hook_image.go b/free/ci/internal/run/cache_hook_image.go new file mode 100644 index 000000000..8057fd969 --- /dev/null +++ b/free/ci/internal/run/cache_hook_image.go @@ -0,0 +1,23 @@ +package run + +// Resolve the pinned container digest used in cache identity. + +import ( + "context" + "strings" +) + +// imageDigest is the pinned digest of the job's container image, "" if unknown. +func (h *cacheHook) imageDigest(ctx context.Context, a AttemptInfo) string { + img := "" + if a.Job.Container != nil { + img = a.Job.Container.Image + } + if img == "" && Registered.Image != nil { + img, _ = Registered.Image.Image(ctx, a) + } + if _, d, ok := strings.Cut(img, "@"); ok && strings.HasPrefix(d, "sha256:") { + return d + } + return "" +} diff --git a/free/ci/internal/sbom/tool.go b/free/ci/internal/sbom/tool.go index 23b7c27ad..0bff1fbbd 100644 --- a/free/ci/internal/sbom/tool.go +++ b/free/ci/internal/sbom/tool.go @@ -17,10 +17,7 @@ package sbom import ( "context" - "crypto/sha256" - "encoding/hex" "errors" - "io" "io/fs" "os" "path/filepath" @@ -217,46 +214,6 @@ func trusted(p string) error { return nil } -// wantBinaryDigest is the pin's sha256 of the extracted executable for an os/arch. -func (p Pin) wantBinaryDigest(goos, goarch string) (string, error) { - src, ok := p.Source(goos, goarch) - if !ok || src.BinarySHA256 == "" { - return "", Failf(ReasonToolChecksum, "the %s pin has no binary_sha256 for %s/%s, so an installed binary cannot be authenticated", ToolName, goos, goarch) - } - return src.BinarySHA256, nil -} - -// verifyBinary hashes the file at p and compares it with the pin's binary_sha256. -// It hashes the descriptor it opened and refuses a path that changed under it. -func verifyBinary(p string, pin Pin, goos, goarch string) error { - want, err := pin.wantBinaryDigest(goos, goarch) - if err != nil { - return err - } - pre, err := os.Lstat(p) - if err != nil { - return Failf(ReasonToolMissing, "cannot inspect %s: %v", p, err) - } - f, err := os.Open(p) - if err != nil { - return Failf(ReasonToolMissing, "cannot read %s: %v", p, err) - } - defer func() { _ = f.Close() }() - st, err := f.Stat() - if err != nil || !os.SameFile(pre, st) { - return Failf(ReasonToolChecksum, "%s changed while it was being checked; run `%s`", p, InstallCommand) - } - h := sha256.New() - n, err := io.Copy(h, io.LimitReader(f, maxBinary+1)) - if err != nil || n > maxBinary { - return Failf(ReasonToolChecksum, "cannot hash %s (%d bytes read, error %v); run `%s`", p, n, err, InstallCommand) - } - if got := hex.EncodeToString(h.Sum(nil)); got != want { - return Failf(ReasonToolChecksum, "%s has sha256 %s, the pin says %s; it is not the pinned %s %s; run `%s` to replace it", p, got, want, ToolName, pin.Version, InstallCommand) - } - return nil -} - // plain refuses an entry another user owns or can write. func plain(st os.FileInfo, what string) error { switch { diff --git a/free/ci/internal/sbom/tool_digest.go b/free/ci/internal/sbom/tool_digest.go new file mode 100644 index 000000000..00185d411 --- /dev/null +++ b/free/ci/internal/sbom/tool_digest.go @@ -0,0 +1,49 @@ +package sbom + +// Verify the pinned executable before it is used. +import ( + "crypto/sha256" + "encoding/hex" + "io" + "os" +) + +// wantBinaryDigest is the pin's sha256 of the extracted executable for an os/arch. +func (p Pin) wantBinaryDigest(goos, goarch string) (string, error) { + src, ok := p.Source(goos, goarch) + if !ok || src.BinarySHA256 == "" { + return "", Failf(ReasonToolChecksum, "the %s pin has no binary_sha256 for %s/%s, so an installed binary cannot be authenticated", ToolName, goos, goarch) + } + return src.BinarySHA256, nil +} + +// verifyBinary hashes the file at p and compares it with the pin's binary_sha256. +// It hashes the descriptor it opened and refuses a path that changed under it. +func verifyBinary(p string, pin Pin, goos, goarch string) error { + want, err := pin.wantBinaryDigest(goos, goarch) + if err != nil { + return err + } + pre, err := os.Lstat(p) + if err != nil { + return Failf(ReasonToolMissing, "cannot inspect %s: %v", p, err) + } + f, err := os.Open(p) + if err != nil { + return Failf(ReasonToolMissing, "cannot read %s: %v", p, err) + } + defer func() { _ = f.Close() }() + st, err := f.Stat() + if err != nil || !os.SameFile(pre, st) { + return Failf(ReasonToolChecksum, "%s changed while it was being checked; run `%s`", p, InstallCommand) + } + h := sha256.New() + n, err := io.Copy(h, io.LimitReader(f, maxBinary+1)) + if err != nil || n > maxBinary { + return Failf(ReasonToolChecksum, "cannot hash %s (%d bytes read, error %v); run `%s`", p, n, err, InstallCommand) + } + if got := hex.EncodeToString(h.Sum(nil)); got != want { + return Failf(ReasonToolChecksum, "%s has sha256 %s, the pin says %s; it is not the pinned %s %s; run `%s` to replace it", p, got, want, ToolName, pin.Version, InstallCommand) + } + return nil +} From 385984efe1ce9e9dfdaa1df673c49e8912e96d62 Mon Sep 17 00:00:00 2001 From: admin Date: Tue, 20 Oct 2026 08:01:00 -0400 Subject: [PATCH 13/19] test(ci): make production-marker fixture independent of host Docker --- free/ci/cmd/agent/prodguard_test.go | 6 ++++++ free/ci/internal/dispatch/chokepoint_roles_test.go | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/free/ci/cmd/agent/prodguard_test.go b/free/ci/cmd/agent/prodguard_test.go index 3db9e2e26..7ac260ed8 100644 --- a/free/ci/cmd/agent/prodguard_test.go +++ b/free/ci/cmd/agent/prodguard_test.go @@ -63,6 +63,12 @@ func TestServeStdioExitsNonZeroWithE663OnADeployHost(t *testing.T) { func TestServeStdioExitsNonZeroWithE663OnAProdStackMarker(t *testing.T) { useState(t) dir := t.TempDir() + // The marker guard checks for docker on PATH before calling the injected runner. + bin := t.TempDir() + if err := os.WriteFile(filepath.Join(bin, "docker"), []byte("#!/bin/sh\nexit 1\n"), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) if err := os.WriteFile(filepath.Join(dir, ".env"), []byte("ENV=prod\n"), 0o600); err != nil { t.Fatal(err) } diff --git a/free/ci/internal/dispatch/chokepoint_roles_test.go b/free/ci/internal/dispatch/chokepoint_roles_test.go index 3c19fcd0d..e92636ed9 100644 --- a/free/ci/internal/dispatch/chokepoint_roles_test.go +++ b/free/ci/internal/dispatch/chokepoint_roles_test.go @@ -42,7 +42,7 @@ func roleGrantPositions(t *testing.T, root string) map[string]map[token.Pos]bool return result } pkgs, err := packages.Load(&packages.Config{Dir: root, - Mode: packages.NeedName | packages.NeedFiles | packages.NeedSyntax | packages.NeedTypes | packages.NeedTypesInfo | packages.NeedImports | packages.NeedDeps}, "./...") + Mode: packages.LoadSyntax}, "./...") if err != nil { t.Fatal(err) } From 1dd4c917119ad6c6c507a89a80e712feec6e5456 Mon Sep 17 00:00:00 2001 From: admin Date: Tue, 20 Oct 2026 08:04:29 -0400 Subject: [PATCH 14/19] test(ci): canonicalize Node fixture home on macOS --- free/ci/internal/gates/node_prepare_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/free/ci/internal/gates/node_prepare_test.go b/free/ci/internal/gates/node_prepare_test.go index e374b4690..50a4fe806 100644 --- a/free/ci/internal/gates/node_prepare_test.go +++ b/free/ci/internal/gates/node_prepare_test.go @@ -100,9 +100,15 @@ type result struct { func (h *nodeHost) run(id, workdir string) result { h.t.Helper() p := h.plan(id, workdir) + // macOS temp roots can include /var -> /private/var. The executor rejects + // symlinked workdirs, so use the real fixture home just as a configured CI home. + home, err := filepath.EvalSymlinks(h.t.TempDir()) + if err != nil { + h.t.Fatal(err) + } var out strings.Builder res := (&ciexec.Executor{}).Run(context.Background(), ciexec.JobSpec{AttemptID: "node-" + strings.ReplaceAll(id, ".", "-"), CoordinatorID: "coord", - Job: model.Job{Kind: "quality", Workdir: workdir, Isolation: "process"}, Command: p.Command(), SourceDir: h.root, Home: h.t.TempDir(), + Job: model.Job{Kind: "quality", Workdir: workdir, Isolation: "process"}, Command: p.Command(), SourceDir: h.root, Home: home, Output: func(b []byte) { out.Write(b) }}) r := result{exit: res.ExitCode, out: strings.TrimSpace(out.String())} if res.Err != nil { From f216c165b2fb0080db9a19fe9e5e338e3dad24bd Mon Sep 17 00:00:00 2001 From: admin Date: Tue, 20 Oct 2026 08:11:34 -0400 Subject: [PATCH 15/19] fix(ci): align process-start guards with current exec funnel --- .../internal/exec/container_security_test.go | 10 ++++++ free/ci/internal/exec/sweep.go | 3 ++ .../internal/legacy/gate_env_live_key_test.go | 2 +- .../nodes/provision/funnel_program_test.go | 32 +++++++++++++++++++ .../nodes/provision/single_exec_test.go | 2 ++ 5 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 free/ci/internal/nodes/provision/funnel_program_test.go diff --git a/free/ci/internal/exec/container_security_test.go b/free/ci/internal/exec/container_security_test.go index 8b529678e..cc8ad738f 100644 --- a/free/ci/internal/exec/container_security_test.go +++ b/free/ci/internal/exec/container_security_test.go @@ -111,3 +111,13 @@ func TestOrphanContainerVanishedDuringSweep(t *testing.T) { t.Fatal(err) } } + +func TestOrphanRemovalRefusesNonRuntime(t *testing.T) { + for _, binary := range []string{"", "ssh", "/usr/bin/ssh", "sh"} { + t.Run(binary, func(t *testing.T) { + if err := removeStaleContainer(context.Background(), binary, "gone"); err == nil { + t.Fatal("non-runtime program accepted") + } + }) + } +} diff --git a/free/ci/internal/exec/sweep.go b/free/ci/internal/exec/sweep.go index 6a0abc493..a3e171c3e 100644 --- a/free/ci/internal/exec/sweep.go +++ b/free/ci/internal/exec/sweep.go @@ -141,6 +141,9 @@ func (e *Executor) sweepRuntime(ctx context.Context, current, binary string) err } func removeStaleContainer(ctx context.Context, binary, id string) error { + if !runtimeBinary.MatchString(binary) { + return coded("E613", "container runtime removal refused") + } out, err := osexec.CommandContext(ctx, binary, "rm", "-f", id).CombinedOutput() if err != nil && strings.Contains(strings.ToLower(string(out)), "no such container") { return nil diff --git a/free/ci/internal/legacy/gate_env_live_key_test.go b/free/ci/internal/legacy/gate_env_live_key_test.go index 02bd3a283..ba983de0c 100644 --- a/free/ci/internal/legacy/gate_env_live_key_test.go +++ b/free/ci/internal/legacy/gate_env_live_key_test.go @@ -55,7 +55,7 @@ func TestSecretsEnvLiveKeyGateTrackedValueOnly(t *testing.T) { if err := os.WriteFile(path, []byte("fixture-value-without-assignment\n"), 0o600); err != nil { t.Fatal(err) } - cmd := exec.Command("git", "add", ".environments/prod/.env") + cmd := exec.Command("git", "add", "-f", ".environments/prod/.env") cmd.Dir = root if out, err := cmd.CombinedOutput(); err != nil { t.Fatalf("track env fixture: %v: %s", err, out) diff --git a/free/ci/internal/nodes/provision/funnel_program_test.go b/free/ci/internal/nodes/provision/funnel_program_test.go new file mode 100644 index 000000000..71c24f175 --- /dev/null +++ b/free/ci/internal/nodes/provision/funnel_program_test.go @@ -0,0 +1,32 @@ +package provision + +import ( + "go/parser" + "go/token" + "testing" +) + +func TestTrustedDynamicProgramsStillRejectSSH(t *testing.T) { + for _, tc := range []struct { + name, path, fn, program string + bad bool + }{ + {"executor argv", "internal/exec/process.go", "runCommandFiles", "program", false}, + {"executor ssh", "internal/exec/process.go", "runCommandFiles", `"ssh"`, true}, + {"untrusted argv", "internal/exec/process.go", "other", "program", true}, + {"sweep runtime", "internal/exec/sweep.go", "removeStaleContainer", "binary", false}, + {"sweep ssh", "internal/exec/sweep.go", "removeStaleContainer", `"ssh"`, true}, + } { + t.Run(tc.name, func(t *testing.T) { + src := "package exec\nimport e \"os/exec\"\nfunc " + tc.fn + "(){ e.Command(" + tc.program + ") }" + f, err := parser.ParseFile(token.NewFileSet(), tc.path, src, 0) + if err != nil { + t.Fatal(err) + } + _, bad := inspectFile(f, tc.path, nil) + if (len(bad) > 0) != tc.bad { + t.Fatalf("violations %v, want bad %v", bad, tc.bad) + } + }) + } +} diff --git a/free/ci/internal/nodes/provision/single_exec_test.go b/free/ci/internal/nodes/provision/single_exec_test.go index ac3551d61..0641e1905 100644 --- a/free/ci/internal/nodes/provision/single_exec_test.go +++ b/free/ci/internal/nodes/provision/single_exec_test.go @@ -46,6 +46,7 @@ var dynamicProgram = map[string]bool{ "internal/exec/process.go:runCommand": true, "internal/exec/process.go:runCommandIn": true, "internal/exec/process.go:runCommandSig": true, + "internal/exec/process.go:runCommandFiles": true, // `nself ci --detach` re-executes the plugin binary (os.Executable) in a new // session; the program is the caller's own binary, never ssh (P7-CI-63). "internal/exec/detach.go:StartDetached": true, @@ -63,6 +64,7 @@ var dynamicProgram = map[string]bool{ "internal/exec/container.go:containerStopper": true, "internal/exec/runtime.go:runtimeQuery": true, "internal/exec/runtime.go:RuntimeRemove": true, + "internal/exec/sweep.go:removeStaleContainer": true, // The sandboxed-container probe (P7-TRUST-05): the runtime binary is checked against // runtimeBinary, the runtime name and image against fixed patterns; runsc is run only by its // absolute path with base name runsc, with the one argument --version. From cf71be4e21feb8a2e53fe5bd7ea6b9339fe84a95 Mon Sep 17 00:00:00 2001 From: admin Date: Tue, 20 Oct 2026 08:15:33 -0400 Subject: [PATCH 16/19] fix(ci): make cancellation test deterministic and resolve package lint --- free/ci/cmd/agent/cache_hook_test.go | 10 ++++++---- free/ci/cmd/agent/join.go | 2 +- free/ci/cmd/agent/join_test.go | 4 ++-- free/ci/cmd/agent/uninstall.go | 8 ++++---- free/ci/cmd/netproxy_test.go | 2 +- free/ci/cmd/nodes_pair_test.go | 10 +++++----- free/ci/cmd/nodes_provision_test.go | 4 ++-- free/ci/cmd/nodes_remote.go | 2 +- free/ci/cmd/providers_probe_test.go | 12 ++++++------ free/ci/cmd/runs_reproduce.go | 6 +++--- free/ci/cmd/serve.go | 4 +--- free/ci/cmd/serve_admin_checkpoint_test.go | 8 ++++---- free/ci/cmd/serve_agent_listener_test.go | 8 ++++---- free/ci/cmd/serve_enroll_listener_test.go | 2 +- free/ci/internal/cache/specs/unpack.go | 2 +- free/ci/internal/dispatch/backpressure_test.go | 2 +- free/ci/internal/dispatch/chokepoint_test.go | 2 +- free/ci/internal/dispatch/deployaudit_test.go | 4 ++-- free/ci/internal/dispatch/floor_test.go | 2 +- free/ci/internal/gates/truth/cli.go | 12 ++++++------ free/ci/internal/gates/truth/evidence.go | 6 +++--- free/ci/internal/nodes/lifecycle/cli.go | 2 +- free/ci/internal/nodes/lifecycle/review_test.go | 4 ++-- free/ci/internal/nodes/lifecycle/revoke.go | 2 +- free/ci/internal/nodes/provision/cli.go | 4 ++-- free/ci/internal/nodes/provision/token_argv_test.go | 2 +- free/ci/internal/run/hygiene_hook.go | 2 +- free/ci/internal/run/jobrun.go | 6 +++--- free/ci/internal/run/remote_await.go | 2 +- free/ci/internal/run/remote_cache_hook.go | 2 +- free/ci/internal/run/safety_test.go | 8 ++++++-- free/ci/internal/sbom/fixtures_test.go | 2 ++ 32 files changed, 77 insertions(+), 71 deletions(-) diff --git a/free/ci/cmd/agent/cache_hook_test.go b/free/ci/cmd/agent/cache_hook_test.go index dd90ecf6d..c0b41f7c4 100644 --- a/free/ci/cmd/agent/cache_hook_test.go +++ b/free/ci/cmd/agent/cache_hook_test.go @@ -38,7 +38,9 @@ func TestCacheHookReadOnlyMounts(t *testing.T) { if err != nil { t.Fatal(err) } - defer cacheAfter(context.Background(), l, session.JobRoot{}, session.Outcome{State: "cancelled"}, nil) + defer func() { + _ = cacheAfter(context.Background(), l, session.JobRoot{}, session.Outcome{State: "cancelled"}, nil) + }() if len(setup.Mounts) != 2 { t.Fatalf("mounts=%+v", setup.Mounts) } @@ -115,7 +117,7 @@ func TestCacheHookPartialRestoreBindsNothing(t *testing.T) { if err != nil { t.Fatal(err) } - defer cacheAfter(context.Background(), l, root, session.Outcome{}, nil) + defer func() { _ = cacheAfter(context.Background(), l, root, session.Outcome{}, nil) }() if len(setup.Mounts) != 0 || len(setup.Env) != 0 { t.Fatalf("partial restore bound: %+v", setup) } @@ -166,7 +168,7 @@ func TestCacheBeforeJobFailureDropsEnvironment(t *testing.T) { if err != nil { t.Fatal(err) } - defer ws.Close() + defer func() { _ = ws.Close() }() if err := ws.Mkdir(specs.WorkspaceCacheName, 0755); err != nil { t.Fatal(err) } @@ -199,7 +201,7 @@ func TestCacheStageFailureKeepsUsableBindings(t *testing.T) { if err != nil { t.Fatal(err) } - defer cacheAfter(context.Background(), l, root, session.Outcome{}, nil) + defer func() { _ = cacheAfter(context.Background(), l, root, session.Outcome{}, nil) }() if setup.Env["GOCACHE"] == "" || setup.Env["GOMODCACHE"] == "" { t.Fatalf("usable binding lost: %+v", setup) } diff --git a/free/ci/cmd/agent/join.go b/free/ci/cmd/agent/join.go index 7d401071f..9413e0472 100644 --- a/free/ci/cmd/agent/join.go +++ b/free/ci/cmd/agent/join.go @@ -80,7 +80,7 @@ func runJoin(ctx context.Context, args []string, sio IO) int { } in := bufio.NewReader(sio.Stdin) line := func(q string) (string, error) { - fmt.Fprint(sio.Stderr, q) + _, _ = fmt.Fprint(sio.Stderr, q) s, err := in.ReadString('\n') if err != nil && (err != io.EOF || s == "") { return "", err diff --git a/free/ci/cmd/agent/join_test.go b/free/ci/cmd/agent/join_test.go index 2ad4bed18..9fa69e1f4 100644 --- a/free/ci/cmd/agent/join_test.go +++ b/free/ci/cmd/agent/join_test.go @@ -164,7 +164,7 @@ func TestJoinEnrollsAndSavesTheCredential(t *testing.T) { t.Fatalf("approver saw %+v", got) } // The sealed client key, the KEK next to it, nothing else secret in plain. - filepath.WalkDir(state, func(p string, d fs.DirEntry, err error) error { + _ = filepath.WalkDir(state, func(p string, d fs.DirEntry, err error) error { if err == nil && !d.IsDir() { raw, _ := os.ReadFile(p) if bytes.Contains(raw, []byte(c.code)) { @@ -213,7 +213,7 @@ func TestJoinWithoutTTYAndWithoutFlagRefuses(t *testing.T) { if err != nil { t.Fatal(err) } - defer null.Close() + defer func() { _ = null.Close() }() code, _, stderr, _ = joinRun(t, null, "", "--name", "node-j", c.url) if code != 1 || !strings.Contains(stderr, "Type yes to trust it") || !strings.Contains(stderr, "fingerprint_unconfirmed") || !strings.Contains(stderr, c.fp) { t.Fatalf("exit %d stderr %q", code, stderr) diff --git a/free/ci/cmd/agent/uninstall.go b/free/ci/cmd/agent/uninstall.go index e291e40eb..ea5573179 100644 --- a/free/ci/cmd/agent/uninstall.go +++ b/free/ci/cmd/agent/uninstall.go @@ -45,13 +45,13 @@ func localRun(ctx context.Context, argv ...string) (string, error) { if err != nil { return "", err } - defer r.Close() + defer func() { _ = r.Close() }() null, err := os.Open(os.DevNull) if err != nil { - w.Close() + _ = w.Close() return "", err } - defer null.Close() + defer func() { _ = null.Close() }() attr := &os.ProcAttr{Files: []*os.File{null, w, w}, Env: []string{"PATH=/usr/bin:/bin:/usr/sbin:/sbin"}} var proc *os.Process switch argv[0] { @@ -62,7 +62,7 @@ func localRun(ctx context.Context, argv ...string) (string, error) { default: err = errors.New("uninstall runs no program named " + argv[0]) } - w.Close() + _ = w.Close() if err != nil { return "", err } diff --git a/free/ci/cmd/netproxy_test.go b/free/ci/cmd/netproxy_test.go index 8ea4b285f..930f3cc6c 100644 --- a/free/ci/cmd/netproxy_test.go +++ b/free/ci/cmd/netproxy_test.go @@ -33,7 +33,7 @@ func TestNetproxyProbeModePrintsWhatConnected(t *testing.T) { if err != nil { t.Fatal(err) } - defer ln.Close() + defer func() { _ = ln.Close() }() go func() { for { c, err := ln.Accept() diff --git a/free/ci/cmd/nodes_pair_test.go b/free/ci/cmd/nodes_pair_test.go index 0f26e3958..a7c1aec0e 100644 --- a/free/ci/cmd/nodes_pair_test.go +++ b/free/ci/cmd/nodes_pair_test.go @@ -198,7 +198,7 @@ func TestNodesPairEndToEnd(t *testing.T) { if err != nil { t.Fatal(err) } - defer st.Close() + defer func() { _ = st.Close() }() n, err := registry.New(st).Get(context.Background(), "node-pair") if err != nil { t.Fatal(err) @@ -505,7 +505,7 @@ func (r *pullRig) handshake(c *pairedCert) error { if err != nil { r.t.Fatal(err) } - defer ln.Close() + defer func() { _ = ln.Close() }() go func() { for { conn, err := ln.Accept() @@ -513,7 +513,7 @@ func (r *pullRig) handshake(c *pairedCert) error { return } go func() { - defer conn.Close() + defer func() { _ = conn.Close() }() if conn.(*tls.Conn).Handshake() == nil { _, _ = conn.Write([]byte("ok")) } @@ -529,7 +529,7 @@ func (r *pullRig) handshake(c *pairedCert) error { if err != nil { return err } - defer conn.Close() + defer func() { _ = conn.Close() }() _ = conn.SetReadDeadline(time.Now().Add(3 * time.Second)) _, err = io.ReadFull(conn, make([]byte, 2)) return err @@ -541,7 +541,7 @@ func (r *pullRig) count(q string, args ...any) int { if err != nil { r.t.Fatal(err) } - defer db.Close() + defer func() { _ = db.Close() }() var n int if err = db.QueryRow(q, args...).Scan(&n); err != nil { r.t.Fatal(err) diff --git a/free/ci/cmd/nodes_provision_test.go b/free/ci/cmd/nodes_provision_test.go index 71b293008..19791ea0e 100644 --- a/free/ci/cmd/nodes_provision_test.go +++ b/free/ci/cmd/nodes_provision_test.go @@ -20,8 +20,8 @@ func capture(t *testing.T, fn func() int) (code int, out, errs string) { os.Stdout, os.Stderr = wo, we defer func() { os.Stdout, os.Stderr = so, se }() code = fn() - wo.Close() - we.Close() + _ = wo.Close() + _ = we.Close() bo, _ := io.ReadAll(ro) be, _ := io.ReadAll(re) return code, string(bo), string(be) diff --git a/free/ci/cmd/nodes_remote.go b/free/ci/cmd/nodes_remote.go index 6eea58491..e54cfc51b 100644 --- a/free/ci/cmd/nodes_remote.go +++ b/free/ci/cmd/nodes_remote.go @@ -114,7 +114,7 @@ var nodeBusy = struct { // claimNode marks cmd's node busy for an upgrade (or a rotate in upgrade mode) and returns the release. func claimNode(cmd clientapi.NodeCommand) (func(), error) { - if cmd.Verb != "upgrade" && !(cmd.Verb == "rotate" && slices.Contains(cmd.Args, "--mode=upgrade")) { + if cmd.Verb != "upgrade" && (cmd.Verb != "rotate" || !slices.Contains(cmd.Args, "--mode=upgrade")) { return func() {}, nil } key := cmd.Node diff --git a/free/ci/cmd/providers_probe_test.go b/free/ci/cmd/providers_probe_test.go index e22b03dcd..a8269a778 100644 --- a/free/ci/cmd/providers_probe_test.go +++ b/free/ci/cmd/providers_probe_test.go @@ -69,17 +69,17 @@ func renderedWorkflow(t *testing.T) string { func githubFixture(t *testing.T, untrustedPresent bool) *httptest.Server { doc := renderedWorkflow(t) srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - switch { - case r.URL.Path == "/user": + switch r.URL.Path { + case "/user": if bearer(r) == "classic" { w.Header().Set("X-OAuth-Scopes", "repo, workflow") } serveJSON(w, 200, map[string]string{"login": "octo"}) - case r.URL.Path == "/repos/octo/widgets/actions/permissions/fork-pr-contributor-approval": + case "/repos/octo/widgets/actions/permissions/fork-pr-contributor-approval": serveJSON(w, 200, map[string]string{"approval_policy": "first_time_contributors"}) - case r.URL.Path == "/repos/octo/widgets": + case "/repos/octo/widgets": serveJSON(w, 200, map[string]any{"default_branch": "main", "permissions": map[string]bool{"admin": true, "push": true, "pull": true}}) - case r.URL.Path == "/repos/octo/widgets/contents/"+githubtmpl.Path: + case "/repos/octo/widgets/contents/" + githubtmpl.Path: if ref := r.URL.Query().Get("ref"); ref == "main" || untrustedPresent && ref == "nself-ci/untrusted" { serveJSON(w, 200, map[string]string{"encoding": "base64", "content": base64.StdEncoding.EncodeToString([]byte(doc))}) return @@ -278,7 +278,7 @@ func TestNoSecretOutput(t *testing.T) { } env := cidoc.Env{Root: r.work, Capture: report.Capture, Getenv: func(k string) string { return planted[k] }} for _, c := range pdoctor.RunWith(context.Background(), env, r.store()) { - all.WriteString(fmt.Sprintf("%+v\n", c)) + fmt.Fprintf(&all, "%+v\n", c) } fg := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { if req.URL.Path == "/-/health" { diff --git a/free/ci/cmd/runs_reproduce.go b/free/ci/cmd/runs_reproduce.go index 971ba1e7a..eb2e14589 100644 --- a/free/ci/cmd/runs_reproduce.go +++ b/free/ci/cmd/runs_reproduce.go @@ -141,10 +141,10 @@ func writeReproduceError(code string, err error, exit int) int { func printReproduceReport(w io.Writer, r reproduce.Report) { fmt.Fprintf(w, "reproduce %s at %.12s (%s, recorded on %s, now %s)\n", r.Job, r.Revision, r.Isolation, r.Platform, r.HostPlatform) for _, d := range r.Drift { - switch { - case d.Kind == reproduce.DriftToolGone: + switch d.Kind { + case reproduce.DriftToolGone: fmt.Fprintf(w, " drift: %s is not available here (recorded %q)\n", d.Name, d.Recorded) - case d.Kind == reproduce.DriftIsolation: + case reproduce.DriftIsolation: fmt.Fprintf(w, " drift: recorded isolation %s, running as %s\n", d.Recorded, d.Current) default: fmt.Fprintf(w, " drift: %s %s: recorded %q, now %q\n", d.Kind, d.Name, d.Recorded, d.Current) diff --git a/free/ci/cmd/serve.go b/free/ci/cmd/serve.go index 72598b08a..666796144 100644 --- a/free/ci/cmd/serve.go +++ b/free/ci/cmd/serve.go @@ -33,9 +33,7 @@ func (l *listFlag) Set(v string) error { if v == "" { return nil } - for _, p := range strings.Split(v, ",") { - l.vals = append(l.vals, p) - } + l.vals = append(l.vals, strings.Split(v, ",")...) return nil } diff --git a/free/ci/cmd/serve_admin_checkpoint_test.go b/free/ci/cmd/serve_admin_checkpoint_test.go index dda548950..f9b5b08e8 100644 --- a/free/ci/cmd/serve_admin_checkpoint_test.go +++ b/free/ci/cmd/serve_admin_checkpoint_test.go @@ -114,7 +114,7 @@ func rewriteChain(t *testing.T, path string, seq int64, detail string) { if err != nil { t.Fatal(err) } - defer db.Close() + defer func() { _ = db.Close() }() if _, err = db.Exec(`UPDATE audit SET detail=? WHERE seq=?`, detail, seq); err != nil { t.Fatal(err) } @@ -176,7 +176,7 @@ func TestCheckpointRemovedRowsFail(t *testing.T) { if err != nil { t.Fatal(err) } - defer db.Close() + defer func() { _ = db.Close() }() if _, err = db.Exec(`DELETE FROM audit WHERE seq>=3`); err != nil { t.Fatal(err) } @@ -197,7 +197,7 @@ func TestCheckpointQuarantineSeamReadsTheStore(t *testing.T) { if err != nil { t.Fatal(err) } - defer st.Close() + defer func() { _ = st.Close() }() led := store.AttestLedger{S: st} if q, err := storeQuarantined(ctx, ev); err != nil || q { t.Fatalf("empty store: %v %v", q, err) @@ -206,7 +206,7 @@ func TestCheckpointQuarantineSeamReadsTheStore(t *testing.T) { if err != nil { t.Fatal(err) } - defer db.Close() + defer func() { _ = db.Close() }() if _, err = db.Exec(`INSERT INTO attest_quarantine(run_id,key_id,quarantined_at) VALUES('run-q','ci-node-0123456789abcdef',1)`); err != nil { t.Fatal(err) } diff --git a/free/ci/cmd/serve_agent_listener_test.go b/free/ci/cmd/serve_agent_listener_test.go index 70d8edb83..3a8ef68cf 100644 --- a/free/ci/cmd/serve_agent_listener_test.go +++ b/free/ci/cmd/serve_agent_listener_test.go @@ -141,7 +141,7 @@ func TestAgentListenerStartRevokeStop(t *testing.T) { } for name, c := range map[string]*tls.Certificate{"no certificate": nil, "user certificate": &userPair} { if resp, rerr := client(c).Get(base + "/v1/agent/leases"); rerr == nil { - resp.Body.Close() + _ = resp.Body.Close() t.Fatalf("%s was served with status %d", name, resp.StatusCode) } } @@ -159,7 +159,7 @@ func TestAgentListenerStartRevokeStop(t *testing.T) { t.Fatalf("hello: %v %v", resp, err) } session := resp.Header.Get(agentapi.HeaderSession) - resp.Body.Close() + _ = resp.Body.Close() if session == "" { t.Fatal("no session") } @@ -171,7 +171,7 @@ func TestAgentListenerStartRevokeStop(t *testing.T) { rq.Header.Set(agentapi.HeaderWait, "30") if r, derr := cl.Do(rq); derr == nil { _, _ = io.Copy(io.Discard, r.Body) - r.Body.Close() + _ = r.Body.Close() } polled <- time.Now() }() @@ -205,7 +205,7 @@ func TestAgentListenerStartRevokeStop(t *testing.T) { t.Fatal("the stopped server is still the active one") } if c, derr := net.DialTimeout("tcp", addr.String(), 500*time.Millisecond); derr == nil { - c.Close() + _ = c.Close() t.Fatal("the agent port still accepts connections after stop") } } diff --git a/free/ci/cmd/serve_enroll_listener_test.go b/free/ci/cmd/serve_enroll_listener_test.go index 2b0fdb184..ac0171de2 100644 --- a/free/ci/cmd/serve_enroll_listener_test.go +++ b/free/ci/cmd/serve_enroll_listener_test.go @@ -94,7 +94,7 @@ func enrollFreeAddr(t *testing.T) string { if err != nil { t.Fatal(err) } - defer ln.Close() + defer func() { _ = ln.Close() }() return ln.Addr().String() } diff --git a/free/ci/internal/cache/specs/unpack.go b/free/ci/internal/cache/specs/unpack.go index 56f97c2fd..589fc7a37 100644 --- a/free/ci/internal/cache/specs/unpack.go +++ b/free/ci/internal/cache/specs/unpack.go @@ -94,7 +94,7 @@ func extract(r io.Reader, root *os.Root, lim authz.Limits, opt UnpackOptions) (U if !dry { fw = newFileWriter(root, opt) } - defer fw.close() // an early return still waits for the writers + defer func() { _ = fw.close() }() // an early return still waits for the writers for { h, err := tr.Next() if errors.Is(err, io.EOF) { diff --git a/free/ci/internal/dispatch/backpressure_test.go b/free/ci/internal/dispatch/backpressure_test.go index 1010cd3e6..1f0f5ae53 100644 --- a/free/ci/internal/dispatch/backpressure_test.go +++ b/free/ci/internal/dispatch/backpressure_test.go @@ -127,7 +127,7 @@ func TestBackpressureLimits(t *testing.T) { if _, err = d2.enqueue(jobIn("alpha", "x2")); err == nil { t.Fatal("over the cap") } - d2.drop(e, errors.New("gone")) + _ = d2.drop(e, errors.New("gone")) if _, err = d2.enqueue(jobIn("alpha", "x2")); err != nil { t.Fatalf("a drained queue accepts again: %v", err) } diff --git a/free/ci/internal/dispatch/chokepoint_test.go b/free/ci/internal/dispatch/chokepoint_test.go index 218f230f9..39b47c71a 100644 --- a/free/ci/internal/dispatch/chokepoint_test.go +++ b/free/ci/internal/dispatch/chokepoint_test.go @@ -115,7 +115,7 @@ func scanFile(f *ast.File, rel string, grants map[token.Pos]bool, add func(strin report(name) } case *ast.Ident: - if banned[x.Name] && !grants[x.Pos()] && x != declName && !(x.Name == "LocalAdmit" && localFiles[rel]) && (x.Name == "LocalAdmit" || !chokeFuncs[key]) { + if banned[x.Name] && !grants[x.Pos()] && x != declName && (x.Name != "LocalAdmit" || !localFiles[rel]) && (x.Name == "LocalAdmit" || !chokeFuncs[key]) { report(x.Name) } case *ast.BasicLit: diff --git a/free/ci/internal/dispatch/deployaudit_test.go b/free/ci/internal/dispatch/deployaudit_test.go index c20455670..8b842c72c 100644 --- a/free/ci/internal/dispatch/deployaudit_test.go +++ b/free/ci/internal/dispatch/deployaudit_test.go @@ -33,12 +33,12 @@ func auditActions(t *testing.T, path, action string) []string { if err != nil { t.Fatal(err) } - defer db.Close() + defer func() { _ = db.Close() }() rows, err := db.Query("SELECT target FROM audit WHERE action=? ORDER BY seq", action) if err != nil { t.Fatal(err) } - defer rows.Close() + defer func() { _ = rows.Close() }() var out []string for rows.Next() { var target string diff --git a/free/ci/internal/dispatch/floor_test.go b/free/ci/internal/dispatch/floor_test.go index 975d0e6a4..47f320cfd 100644 --- a/free/ci/internal/dispatch/floor_test.go +++ b/free/ci/internal/dispatch/floor_test.go @@ -71,7 +71,7 @@ func TestRaiseIsolationNeverLowers(t *testing.T) { for _, level := range []model.Isolation{"process", "container", "sandboxed-container", "vm", "ephemeral-vm"} { for _, floor := range []model.Isolation{"", "process", "container", "sandboxed-container"} { got := raiseIsolation(model.Job{Isolation: level}, floor).Isolation - if got != level && !(floor != "" && got == floor) { + if got != level && (floor == "" || got != floor) { t.Fatalf("level %q floor %q gave %q", level, floor, got) } if rank := map[model.Isolation]int{"process": 0, "container": 1, "sandboxed-container": 2, "vm": 3, "ephemeral-vm": 4}; rank[got] < rank[level] { diff --git a/free/ci/internal/gates/truth/cli.go b/free/ci/internal/gates/truth/cli.go index d5c0fb1bd..f96dbedb1 100644 --- a/free/ci/internal/gates/truth/cli.go +++ b/free/ci/internal/gates/truth/cli.go @@ -31,7 +31,7 @@ var revisionRe = regexp.MustCompile(`^[0-9a-f]{40}$`) // Main runs one command and returns the process exit code. func Main(args []string, in io.Reader, out, errw io.Writer) int { if len(args) == 0 { - fmt.Fprintln(errw, "usage: run.go check|sweep|hash [flags] [rule files...]") + _, _ = fmt.Fprintln(errw, "usage: run.go check|sweep|hash [flags] [rule files...]") return 2 } switch args[0] { @@ -42,7 +42,7 @@ func Main(args []string, in io.Reader, out, errw io.Writer) int { case "hash": return cmdHash(in, out, errw) } - fmt.Fprintf(errw, "truth: unknown command %q (check, sweep, hash)\n", args[0]) + _, _ = fmt.Fprintf(errw, "truth: unknown command %q (check, sweep, hash)\n", args[0]) return 2 } @@ -54,7 +54,7 @@ func cmdCheck(args []string, out, errw io.Writer) int { kind := fs.String("kind", "", "report one rule kind only (banned-literal, retired-term, dead-host, numeric-claim, version-claim), or `rules` to validate the rule files without scanning") asJSON := fs.Bool("json", false, "print one JSON document: {data:{result, checks, findings}}") if err := fs.Parse(args); err != nil || fs.NArg() == 0 { - fmt.Fprintln(errw, "usage: check --root DIR [--rules-root DIR] [--kind K] RULEFILE...") + _, _ = fmt.Fprintln(errw, "usage: check --root DIR [--rules-root DIR] [--kind K] RULEFILE...") return 2 } base := *rulesRoot @@ -64,10 +64,10 @@ func cmdCheck(args []string, out, errw io.Writer) int { if *kind == "rules" { rs, err := Load(base, fs.Args()) if err != nil { - fmt.Fprintln(out, "ERROR truth.rules\n "+err.Error()) + _, _ = fmt.Fprintln(out, "ERROR truth.rules\n "+err.Error()) return 2 } - fmt.Fprintf(out, "PASS truth.rules (%d rule(s) in %d file(s))\n", len(rs.Rules), len(rs.Files)) + _, _ = fmt.Fprintf(out, "PASS truth.rules (%d rule(s) in %d file(s))\n", len(rs.Rules), len(rs.Files)) return 0 } checks, found := RunFull(*root, fs.Args(), *rulesRoot) @@ -82,7 +82,7 @@ func cmdCheck(args []string, out, errw io.Writer) int { only = checks // the rule files did not load: show that error, not "unknown --kind" } if len(only) == 0 { - fmt.Fprintf(errw, "truth: unknown --kind %q (rules, %s)\n", *kind, strings.Join(append(append([]string(nil), Kinds...), ClaimKinds...), ", ")) + _, _ = fmt.Fprintf(errw, "truth: unknown --kind %q (rules, %s)\n", *kind, strings.Join(append(append([]string(nil), Kinds...), ClaimKinds...), ", ")) return 2 } checks = only diff --git a/free/ci/internal/gates/truth/evidence.go b/free/ci/internal/gates/truth/evidence.go index c866174ff..13cdc3e3b 100644 --- a/free/ci/internal/gates/truth/evidence.go +++ b/free/ci/internal/gates/truth/evidence.go @@ -38,11 +38,11 @@ func Fold(repo string, checks []model.Check, errText string) model.Check { var notes []string for _, k := range checks { c.Findings += k.Findings - switch { - case k.Result == "error": + switch k.Result { + case "error": c.Result, c.FailureClass = "error", k.FailureClass notes = append(notes, k.Excerpt) - case k.Result == "fail": + case "fail": if c.Result != "error" { c.Result, c.FailureClass = "fail", "code" } diff --git a/free/ci/internal/nodes/lifecycle/cli.go b/free/ci/internal/nodes/lifecycle/cli.go index d916096b8..ce8db5c4f 100644 --- a/free/ci/internal/nodes/lifecycle/cli.go +++ b/free/ci/internal/nodes/lifecycle/cli.go @@ -31,7 +31,7 @@ type Env struct { } type flags struct { - json, acceptsSet bool + json bool project, name, hostKey string agentBinary, installer string mode string diff --git a/free/ci/internal/nodes/lifecycle/review_test.go b/free/ci/internal/nodes/lifecycle/review_test.go index 95d251c9c..2dc6b3bb1 100644 --- a/free/ci/internal/nodes/lifecycle/review_test.go +++ b/free/ci/internal/nodes/lifecycle/review_test.go @@ -166,8 +166,8 @@ func TestPinFileChangesAreSerializedAcrossProcesses(t *testing.T) { done := make(chan error, 1) go func() { done <- k.d.pin("pl1", []remote.HostKey{keyA}) }() select { - case <-done: - t.Fatal("pin wrote the pin file while another process held the pin lock") + case err := <-done: + t.Fatalf("pin returned %v while another process held the pin lock", err) case <-time.After(300 * time.Millisecond): } unlock() diff --git a/free/ci/internal/nodes/lifecycle/revoke.go b/free/ci/internal/nodes/lifecycle/revoke.go index 22c6adc62..8ac414bf5 100644 --- a/free/ci/internal/nodes/lifecycle/revoke.go +++ b/free/ci/internal/nodes/lifecycle/revoke.go @@ -88,7 +88,7 @@ func (d *Deps) dropOrphanPin(ctx context.Context, rep RevokeReport, id string, n if lerr != nil { return rep, newErr(CodeNoRoute, "lock", "node %s is busy: %s", id, oneLine(lerr)) } - defer unlock() + defer func() { unlock() }() switch _, gerr := d.Registry.Get(ctx, id); { case gerr == nil: unlock() diff --git a/free/ci/internal/nodes/provision/cli.go b/free/ci/internal/nodes/provision/cli.go index e4b0ef189..302c342bc 100644 --- a/free/ci/internal/nodes/provision/cli.go +++ b/free/ci/internal/nodes/provision/cli.go @@ -94,7 +94,7 @@ func parseCommand(fs *flagSet, name string, args []string, d Deps) (done bool, c } if fs.help.on { txt, _ := Help(name) - fmt.Fprint(d.Out, txt) + _, _ = fmt.Fprint(d.Out, txt) return true, 0 } return false, 0 @@ -233,7 +233,7 @@ func RunVerify(args []string, d Deps) int { return finish(d, err) } } else { - fmt.Fprint(d.Out, RenderMatrix(reports)) + _, _ = fmt.Fprint(d.Out, RenderMatrix(reports)) } if FoundProblems(reports) { return 1 // output is already written, as core's silent errs.Exit(1) diff --git a/free/ci/internal/nodes/provision/token_argv_test.go b/free/ci/internal/nodes/provision/token_argv_test.go index 66ecf758a..61266faa9 100644 --- a/free/ci/internal/nodes/provision/token_argv_test.go +++ b/free/ci/internal/nodes/provision/token_argv_test.go @@ -141,7 +141,7 @@ func TestInstallRunnerInstance_EndToEndArgvAndEnv(t *testing.T) { t.Fatalf("config.sh saw ACTIONS_RUNNER_INPUT_TOKEN=%q", env) } // no token on stdin: the script refuses before any host work - os.Remove(argvLog) + _ = os.Remove(argvLog) out, err := LocalExecutor{}.RunInput(context.Background(), renderedScript(t, root), "") if err == nil || !strings.Contains(out, "no runner registration token on stdin") { t.Fatalf("empty stdin: out=%q err=%v", out, err) diff --git a/free/ci/internal/run/hygiene_hook.go b/free/ci/internal/run/hygiene_hook.go index 384bf13fb..7819903af 100644 --- a/free/ci/internal/run/hygiene_hook.go +++ b/free/ci/internal/run/hygiene_hook.go @@ -146,7 +146,7 @@ func markerLive(path string) bool { if err != nil { return false } - defer proc.Release() + defer func() { _ = proc.Release() }() err = proc.Signal(syscall.Signal(0)) return err == nil || errors.Is(err, syscall.EPERM) } diff --git a/free/ci/internal/run/jobrun.go b/free/ci/internal/run/jobrun.go index 89b80502d..7a93df42e 100644 --- a/free/ci/internal/run/jobrun.go +++ b/free/ci/internal/run/jobrun.go @@ -192,13 +192,13 @@ func (rs *runState) runLeased(ctx context.Context, pj plan.PlannedJob, job model _ = guard("cache after hook", func() error { return rs.hooks.Cache.After(bg, info, oc) }) } }() - switch { - case res.Attempt.State == "cancelled": + switch res.Attempt.State { + case "cancelled": if _, err := rs.settle(id, "running", "cancelled", "cancelled"); err != nil { return failed(o, err) } return rs.stopped(o, pj.ID) - case res.Attempt.State == "passed": + case "passed": return rs.pass(id, o, info) } o.FailureClass = res.Attempt.FailureClass diff --git a/free/ci/internal/run/remote_await.go b/free/ci/internal/run/remote_await.go index 8bc6be1a7..2777c19b0 100644 --- a/free/ci/internal/run/remote_await.go +++ b/free/ci/internal/run/remote_await.go @@ -75,7 +75,7 @@ func (rs *runState) runRemote(ctx context.Context, pj plan.PlannedJob, job model ra = rd.sink.attempt(id) } if ra == nil { - rs.driveToCancelled(id) + _ = rs.driveToCancelled(id) return failed(o, fail(CodeOptions, "attempt %s was placed on a remote runner; this run has no way to await it", id)) } info := rs.info(pj, job, id, n) diff --git a/free/ci/internal/run/remote_cache_hook.go b/free/ci/internal/run/remote_cache_hook.go index 202237fb8..76df13d56 100644 --- a/free/ci/internal/run/remote_cache_hook.go +++ b/free/ci/internal/run/remote_cache_hook.go @@ -79,7 +79,7 @@ func (remoteCacheFinalize) AfterRun(ctx context.Context, r RunInfo) error { if _, statErr := os.Stat(path); statErr == nil { fresh, err = store.Open(path, store.Options{}) if err == nil { - defer fresh.Close() + defer func() { _ = fresh.Close() }() state, _, err = fresh.AttemptStateEpoch(ctx, e.attempt) } } diff --git a/free/ci/internal/run/safety_test.go b/free/ci/internal/run/safety_test.go index 4ee9cef23..584f544e4 100644 --- a/free/ci/internal/run/safety_test.go +++ b/free/ci/internal/run/safety_test.go @@ -46,8 +46,12 @@ func TestStatusNotPostedForPartialCancelledOrOutsideConfig(t *testing.T) { } // Ctrl-C: the run context ends while a job runs; nothing is posted r := newRig(t, map[string]model.Job{"a": sh("sleep 60")}, gitFake{}) - ctx, cancel := context.WithCancel(context.Background()) - go func() { time.Sleep(1500 * time.Millisecond); cancel() }() + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + r.C.Hooks.AttemptStarts = []AttemptStartHook{startFn(func(AttemptInfo) error { + cancel() + return nil + })} res, err := r.C.Run(ctx) if err != nil || res.Exit == 0 || len(r.Rep.all()) != 0 || !strings.Contains(res.Summary.Status, report.ReasonCancelled) { t.Fatalf("exit %d err %v status %q posted %+v", res.Exit, err, res.Summary.Status, r.Rep.all()) diff --git a/free/ci/internal/sbom/fixtures_test.go b/free/ci/internal/sbom/fixtures_test.go index ea9b555d1..b300f388f 100644 --- a/free/ci/internal/sbom/fixtures_test.go +++ b/free/ci/internal/sbom/fixtures_test.go @@ -1,3 +1,5 @@ +//go:build sbomtool + package sbom_test import ( From 5d734a8f57e0ab4b83834c272be9c186f09609ec Mon Sep 17 00:00:00 2001 From: admin Date: Tue, 20 Oct 2026 08:19:19 -0400 Subject: [PATCH 17/19] fix(ci): audit operator trust inside disposable hosted workflow VMs --- .github/workflows/ci-scenarios.yml | 7 +++- .github/workflows/nself-ci-source.yml | 5 ++- .../ci/internal/dispatch/backpressure_test.go | 2 +- free/ci/internal/protect/template_test.go | 39 +++++++++++++++++++ 4 files changed, 49 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci-scenarios.yml b/.github/workflows/ci-scenarios.yml index bb5bc2295..69b88484e 100644 --- a/.github/workflows/ci-scenarios.yml +++ b/.github/workflows/ci-scenarios.yml @@ -10,6 +10,9 @@ # Constraints: actions pinned by SHA; scenario 1 cuts the network for its own processes with `sudo -n` (nft on Linux, pf on # macOS) and removes the cut on every exit path. Runs on pull requests that touch these paths and on demand. +# The disposable GitHub-hosted VM isolates each scenario. The source engine +# audits the explicit operator trust override; the workflow has a read-only token +# and receives no repository secrets. name: ci-scenarios on: @@ -80,7 +83,7 @@ jobs: run: | set -euo pipefail mkdir -p "$RUNNER_TEMP/evidence" - .drill-bin/nself-ci --no-status --pipeline drills --job drill-scenario-12-external-consumer --json > "$RUNNER_TEMP/evidence/scenario-12.json" || true # exit 1 is the partial selection; the job check below decides + .drill-bin/nself-ci --trust owner --no-status --pipeline drills --job drill-scenario-12-external-consumer --json > "$RUNNER_TEMP/evidence/scenario-12.json" || true # exit 1 is the partial selection; the job check below decides # A single-job run is a partial selection, so its overall result is never pass: read the job's own check. jq -r '"scenario-12: " + ([.data.checks[] | select(.id == "drill-scenario-12-external-consumer") | .result][0] // "no result")' "$RUNNER_TEMP/evidence/scenario-12.json" | tee -a "$GITHUB_STEP_SUMMARY" jq -e '[.data.checks[] | select(.id == "drill-scenario-12-external-consumer") | .result] == ["pass"]' "$RUNNER_TEMP/evidence/scenario-12.json" >/dev/null @@ -90,7 +93,7 @@ jobs: run: | set -euo pipefail mkdir -p "$RUNNER_TEMP/evidence" - .drill-bin/nself-ci --no-status --pipeline drills --job drill-scenario-01-local-only --json > "$RUNNER_TEMP/evidence/scenario-01.json" || true # exit 1 is the partial selection; the job check below decides + .drill-bin/nself-ci --trust owner --no-status --pipeline drills --job drill-scenario-01-local-only --json > "$RUNNER_TEMP/evidence/scenario-01.json" || true # exit 1 is the partial selection; the job check below decides # A single-job run is a partial selection, so its overall result is never pass: read the job's own check. jq -r '"scenario-01: " + ([.data.checks[] | select(.id == "drill-scenario-01-local-only") | .result][0] // "no result")' "$RUNNER_TEMP/evidence/scenario-01.json" | tee -a "$GITHUB_STEP_SUMMARY" jq -e '[.data.checks[] | select(.id == "drill-scenario-01-local-only") | .result] == ["pass"]' "$RUNNER_TEMP/evidence/scenario-01.json" >/dev/null diff --git a/.github/workflows/nself-ci-source.yml b/.github/workflows/nself-ci-source.yml index a9cc3e984..4d148a4a7 100644 --- a/.github/workflows/nself-ci-source.yml +++ b/.github/workflows/nself-ci-source.yml @@ -71,7 +71,10 @@ jobs: set -o pipefail mkdir -p "$RUNNER_TEMP/evidence" status=0 - "$NSELF_CI_BIN" --no-status --json > "$RUNNER_TEMP/evidence/nself-ci-source.json" || status=$? + # This disposable GitHub-hosted VM is the outer isolation boundary. + # The checkout-built engine audits this explicit local-operator override; + # no repository secrets or writable token are available to its jobs. + "$NSELF_CI_BIN" --trust owner --no-status --json > "$RUNNER_TEMP/evidence/nself-ci-source.json" || status=$? { echo "### nself-ci/source (built from $GITHUB_SHA)" echo "exit code: $status" diff --git a/free/ci/internal/dispatch/backpressure_test.go b/free/ci/internal/dispatch/backpressure_test.go index 1f0f5ae53..1a1e9a840 100644 --- a/free/ci/internal/dispatch/backpressure_test.go +++ b/free/ci/internal/dispatch/backpressure_test.go @@ -127,7 +127,7 @@ func TestBackpressureLimits(t *testing.T) { if _, err = d2.enqueue(jobIn("alpha", "x2")); err == nil { t.Fatal("over the cap") } - _ = d2.drop(e, errors.New("gone")) + _, _ = d2.drop(e, errors.New("gone")) if _, err = d2.enqueue(jobIn("alpha", "x2")); err != nil { t.Fatalf("a drained queue accepts again: %v", err) } diff --git a/free/ci/internal/protect/template_test.go b/free/ci/internal/protect/template_test.go index cd4d70878..fbda575aa 100644 --- a/free/ci/internal/protect/template_test.go +++ b/free/ci/internal/protect/template_test.go @@ -17,6 +17,7 @@ type workflowDoc struct { Env map[string]string `yaml:"env"` Jobs map[string]struct { Name string `yaml:"name"` + RunsOn string `yaml:"runs-on"` Strategy any `yaml:"strategy"` Steps []struct { Uses string `yaml:"uses"` @@ -117,3 +118,41 @@ func TestMutationPatchesApplyShape(t *testing.T) { } } } + +// A checkout-built engine cannot infer operator provenance from a fetched PR. +// Its override is scoped to disposable hosted runners, without secrets or writes. +func TestHostedSourceWorkflowTrust(t *testing.T) { + for _, tc := range []struct { + file, job, command, runner string + calls int + }{ + {"../../.github/workflows/nself-ci-source.yml", "nself-ci-source", `"$NSELF_CI_BIN"`, "ubuntu-latest", 1}, + {"../../.github/workflows/ci-scenarios.yml", "scenarios", ".drill-bin/nself-ci", "${{ matrix.os }}", 2}, + } { + t.Run(tc.job, func(t *testing.T) { + d, raw := readWorkflow(t, tc.file) + job, ok := d.Jobs[tc.job] + if !ok || job.RunsOn != tc.runner { + t.Fatalf("unexpected runner: %+v", job) + } + if len(d.Perm) != 1 || d.Perm["contents"] != "read" || strings.Contains(strings.ReplaceAll(raw, " ", ""), "${{secrets.") || strings.Contains(raw, "self-hosted") { + t.Fatal("operator override requires disposable hosted execution, no secrets and a read-only token") + } + calls := 0 + for _, step := range job.Steps { + for _, line := range strings.Split(step.Run, "\n") { + if !strings.HasPrefix(strings.TrimSpace(line), tc.command+" ") { + continue + } + calls++ + if !strings.Contains(line, " --trust owner ") { + t.Fatalf("missing audited operator override: %s", line) + } + } + } + if calls != tc.calls { + t.Fatalf("gate commands %d, want %d", calls, tc.calls) + } + }) + } +} From c387b46bf979016a68f79f5972f5f61d64c43452 Mon Sep 17 00:00:00 2001 From: admin Date: Tue, 20 Oct 2026 08:23:12 -0400 Subject: [PATCH 18/19] refactor(ci): finish diagnostic lint and split approval and hook sources --- free/ci/bench/cmd/cibench/main.go | 2 +- free/ci/cmd/agent/cache_hook_test.go | 6 +- free/ci/cmd/agent/cred_wrapper.go | 10 +-- free/ci/cmd/agent/exec.go | 12 +-- free/ci/cmd/agent/job_shim.go | 14 ++-- free/ci/cmd/agent/join.go | 18 ++-- free/ci/cmd/agent/main.go | 6 +- free/ci/cmd/agent/pull.go | 28 ++++--- free/ci/cmd/agent/pull_e2e_test.go | 2 +- free/ci/cmd/agent/serve.go | 28 +++---- free/ci/cmd/agent/uninstall.go | 12 +-- free/ci/cmd/agent/upgrade.go | 16 ++-- free/ci/cmd/agent/verify_release.go | 16 ++-- free/ci/cmd/agent/version.go | 4 +- free/ci/cmd/approve.go | 70 +--------------- free/ci/cmd/approve_pending.go | 79 ++++++++++++++++++ free/ci/cmd/evidence.go | 4 +- free/ci/cmd/explain.go | 2 +- free/ci/cmd/nodes_pair.go | 24 +++--- free/ci/cmd/nodes_secrets.go | 8 +- free/ci/cmd/policy_protect.go | 14 ++-- free/ci/cmd/policy_protect_ops.go | 12 +-- free/ci/cmd/providers.go | 2 +- free/ci/cmd/runs_reproduce.go | 10 +-- free/ci/cmd/serve_admin.go | 2 +- free/ci/cmd/v15_coordinator.go | 8 +- free/ci/cmd/v15_detach.go | 2 +- free/ci/internal/exec/detach.go | 4 - free/ci/internal/exec/detach_windows.go | 8 +- free/ci/internal/gates/truth/cli.go | 20 ++--- free/ci/internal/gates/truth/report.go | 8 +- .../ci/internal/nodes/lifecycle/separation.go | 2 +- free/ci/internal/nodes/provision/cli.go | 8 +- free/ci/internal/run/hooks.go | 76 ----------------- free/ci/internal/run/hooks_registry.go | 82 +++++++++++++++++++ free/ci/internal/run/render.go | 18 ++-- 36 files changed, 330 insertions(+), 307 deletions(-) create mode 100644 free/ci/cmd/approve_pending.go create mode 100644 free/ci/internal/run/hooks_registry.go diff --git a/free/ci/bench/cmd/cibench/main.go b/free/ci/bench/cmd/cibench/main.go index c3cd311c3..8d743f744 100644 --- a/free/ci/bench/cmd/cibench/main.go +++ b/free/ci/bench/cmd/cibench/main.go @@ -125,7 +125,7 @@ func budgetCmd(o options, stderr io.Writer) int { if informational[name] || strings.HasSuffix(name, ".v14") { return false } - return !(o.onlyCore && strings.HasPrefix(name, "ci.service_")) + return !o.onlyCore || !strings.HasPrefix(name, "ci.service_") } b, err := bench.BudgetFromResult(res, o.note, keep) if err != nil { diff --git a/free/ci/cmd/agent/cache_hook_test.go b/free/ci/cmd/agent/cache_hook_test.go index c0b41f7c4..03f9f80ef 100644 --- a/free/ci/cmd/agent/cache_hook_test.go +++ b/free/ci/cmd/agent/cache_hook_test.go @@ -163,7 +163,7 @@ func TestCacheBeforeJobFailureDropsEnvironment(t *testing.T) { if err != nil { t.Fatal(err) } - defer cacheAfter(context.Background(), l, session.JobRoot{}, session.Outcome{}, nil) + defer func() { _ = cacheAfter(context.Background(), l, session.JobRoot{}, session.Outcome{}, nil) }() ws, err := os.OpenRoot(t.TempDir()) if err != nil { t.Fatal(err) @@ -223,7 +223,7 @@ func TestCacheHookReplacementClosesRoots(t *testing.T) { if _, err := cacheBefore(context.Background(), l, session.JobRoot{Dir: t.TempDir()}); err != nil { t.Fatal(err) } - defer cacheAfter(context.Background(), l, session.JobRoot{}, session.Outcome{}, nil) + defer func() { _ = cacheAfter(context.Background(), l, session.JobRoot{}, session.Outcome{}, nil) }() if _, err := old.base.Stat("."); err == nil { t.Fatal("replaced root left open") } @@ -250,7 +250,7 @@ func TestCacheCollectionRefusalReachesJobLog(t *testing.T) { } l := cacheHookLease("rw", "process") nodeCaches.Store(l.AttemptID, &nodeCache{base: base, stage: &refusedCacheStage{}}) - defer cacheAfter(context.Background(), l, session.JobRoot{}, session.Outcome{}, nil) + defer func() { _ = cacheAfter(context.Background(), l, session.JobRoot{}, session.Outcome{}, nil) }() var output bytes.Buffer hook := agentAfterJob(execSpec{AttemptID: l.AttemptID}, jobInputs{Output: func(b []byte) { _, _ = output.Write(b) }}) if err := hook(context.Background(), nil); err != nil { diff --git a/free/ci/cmd/agent/cred_wrapper.go b/free/ci/cmd/agent/cred_wrapper.go index d86486356..9e4401759 100644 --- a/free/ci/cmd/agent/cred_wrapper.go +++ b/free/ci/cmd/agent/cred_wrapper.go @@ -39,28 +39,28 @@ func init() { func runCredWrapper(ctx context.Context, args []string, sio IO) int { if len(args) < 3 || args[1] != "--" || credEnv[args[0]] == "" { - fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent cred-wrapper -- [args...]") + _, _ = fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent cred-wrapper -- [args...]") return 2 } line, err := readCredential(sio.Stdin) if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } defer zeroBytes(line) path, err := lookCommand(args[2]) if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } env, err := ciexec.AllowlistEnv{}.Environment(ctx, model.Job{}) if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } env = append(env, credEnv[args[0]]+"="+string(line)) err = syscall.Exec(path, args[2:], env) // the process becomes the runner command; only returns on failure - fmt.Fprintln(sio.Stderr, "error: exec:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error: exec:", err) return 1 } diff --git a/free/ci/cmd/agent/exec.go b/free/ci/cmd/agent/exec.go index 5b12e44cb..e78f9bca7 100644 --- a/free/ci/cmd/agent/exec.go +++ b/free/ci/cmd/agent/exec.go @@ -77,32 +77,32 @@ func runExecCmd(ctx context.Context, args []string, sio IO) int { specPath := fs.String("spec", "", "job spec JSON file") evPath := fs.String("evidence", "", "where to write the evidence document") if err := fs.Parse(args); err != nil || *specPath == "" || *evPath == "" || fs.NArg() != 0 { - fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent exec --spec --evidence ") + _, _ = fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent exec --spec --evidence ") return 2 } if err := RunPreStart(ctx, "exec"); err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } raw, err := os.ReadFile(*specPath) if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 2 } var spec execSpec dec := json.NewDecoder(bytes.NewReader(raw)) dec.DisallowUnknownFields() if err := dec.Decode(&spec); err != nil { - fmt.Fprintln(sio.Stderr, "error: spec:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error: spec:", err) return 2 } res, err := runSpec(ctx, spec, jobInputs{Output: func(b []byte) { _, _ = sio.Stdout.Write(b) }}) if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } if err := writeEvidence(*evPath, res.Evidence); err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } if res.Outcome.State != "passed" { diff --git a/free/ci/cmd/agent/job_shim.go b/free/ci/cmd/agent/job_shim.go index 84b8fa654..728b8a0c1 100644 --- a/free/ci/cmd/agent/job_shim.go +++ b/free/ci/cmd/agent/job_shim.go @@ -95,7 +95,7 @@ func shimUserOK(euid int, name string) bool { func runJobShim(_ context.Context, args []string, sio IO) int { if len(args) != 0 { - fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent job-shim (one JSON request line on stdin)") + _, _ = fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent job-shim (one JSON request line on stdin)") return 2 } name := "" @@ -103,17 +103,17 @@ func runJobShim(_ context.Context, args []string, sio IO) int { name = u.Username } if !shimUserOK(os.Geteuid(), name) { - fmt.Fprintln(sio.Stderr, "error: job-shim runs only as a job user (nself-ci-job-), through the agent's sudo rule") + _, _ = fmt.Fprintln(sio.Stderr, "error: job-shim runs only as a job user (nself-ci-job-), through the agent's sudo rule") return 1 } raw, err := readShimRequest(sio.Stdin, maxShimRequest) if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 2 } req, err := parseShimRequest(raw) if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 2 } return execShimRequest(req, sio) @@ -123,7 +123,7 @@ func runJobShim(_ context.Context, args []string, sio IO) int { func execShimRequest(req shimRequest, sio IO) int { path, err := lookCommand(req.Argv[0]) if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 127 } dir := req.Dir @@ -131,10 +131,10 @@ func execShimRequest(req shimRequest, sio IO) int { dir = "/" // never the agent's inherited cwd (its 0700 state directory) } if err = os.Chdir(dir); err != nil { - fmt.Fprintln(sio.Stderr, "error: cannot enter the job directory:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error: cannot enter the job directory:", err) return 126 } err = syscall.Exec(path, req.Argv, req.Env) // only returns on failure - fmt.Fprintln(sio.Stderr, "error: exec:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error: exec:", err) return 126 } diff --git a/free/ci/cmd/agent/join.go b/free/ci/cmd/agent/join.go index 9413e0472..71670db72 100644 --- a/free/ci/cmd/agent/join.go +++ b/free/ci/cmd/agent/join.go @@ -52,21 +52,21 @@ func runJoin(ctx context.Context, args []string, sio IO) int { return 2 } if fs.NArg() != 1 { - fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent join [--expect-fingerprint SHA256:] [--name N] ") + _, _ = fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent join [--expect-fingerprint SHA256:] [--name N] ") return 2 } if err := RunPreStart(ctx, "join"); err != nil { - fmt.Fprintf(sio.Stderr, "nself-ci-agent join: %v\n", err) + _, _ = fmt.Fprintf(sio.Stderr, "nself-ci-agent join: %v\n", err) return 1 } state, err := stateDir() if err != nil { - fmt.Fprintf(sio.Stderr, "nself-ci-agent join: %v\n", err) + _, _ = fmt.Fprintf(sio.Stderr, "nself-ci-agent join: %v\n", err) return 1 } key, err := nodekey.LoadOrCreate(state) if err != nil { - fmt.Fprintf(sio.Stderr, "nself-ci-agent join: node key: %v\n", err) + _, _ = fmt.Fprintf(sio.Stderr, "nself-ci-agent join: node key: %v\n", err) return 1 } host, _ := os.Hostname() @@ -94,23 +94,23 @@ func runJoin(ctx context.Context, args []string, sio IO) int { } if tty { opts.Confirm = func(fp string) (bool, error) { - fmt.Fprintf(sio.Stderr, "The coordinator's root fingerprint is\n %s\nCompare it with the one `nself ci nodes pair` printed.\n", fp) + _, _ = fmt.Fprintf(sio.Stderr, "The coordinator's root fingerprint is\n %s\nCompare it with the one `nself ci nodes pair` printed.\n", fp) a, err := line("Type yes to trust it: ") return strings.EqualFold(a, "yes"), err } } res, err := enroll.Join(ctx, opts) if err != nil { - fmt.Fprintf(sio.Stderr, "nself-ci-agent join: %v\n", err) + _, _ = fmt.Fprintf(sio.Stderr, "nself-ci-agent join: %v\n", err) return 1 } if err = enroll.SaveCredential(state, res, time.Now()); err != nil { - fmt.Fprintf(sio.Stderr, "nself-ci-agent join: the certificate was issued but could not be saved: %v\n", err) + _, _ = fmt.Fprintf(sio.Stderr, "nself-ci-agent join: the certificate was issued but could not be saved: %v\n", err) return 1 } - fmt.Fprintf(sio.Stdout, "Joined as %s. Certificate valid until %s (renews at two thirds of its life).\n", res.NodeID, res.NotAfter.Local().Format(time.DateTime)) + _, _ = fmt.Fprintf(sio.Stdout, "Joined as %s. Certificate valid until %s (renews at two thirds of its life).\n", res.NodeID, res.NotAfter.Local().Format(time.DateTime)) if err = pairing.RunJoinHooks(ctx, res, state); err != nil { - fmt.Fprintf(sio.Stderr, "nself-ci-agent join: enrolled, but %v\n", err) + _, _ = fmt.Fprintf(sio.Stderr, "nself-ci-agent join: enrolled, but %v\n", err) return 1 } return 0 diff --git a/free/ci/cmd/agent/main.go b/free/ci/cmd/agent/main.go index 6a09eb6a3..6cfd9facb 100644 --- a/free/ci/cmd/agent/main.go +++ b/free/ci/cmd/agent/main.go @@ -41,9 +41,9 @@ func main() { func run(ctx context.Context, args []string, sio IO) int { if len(args) == 0 || args[0] == "-h" || args[0] == "--help" || args[0] == "help" { - fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent [args]") + _, _ = fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent [args]") for _, l := range usageLines() { - fmt.Fprintln(sio.Stderr, l) + _, _ = fmt.Fprintln(sio.Stderr, l) } if len(args) == 0 { return 2 @@ -52,7 +52,7 @@ func run(ctx context.Context, args []string, sio IO) int { } fn, ok := lookup(args[0]) if !ok { - fmt.Fprintf(sio.Stderr, "nself-ci-agent: unknown command %q\n", args[0]) + _, _ = fmt.Fprintf(sio.Stderr, "nself-ci-agent: unknown command %q\n", args[0]) return 2 } return fn(ctx, args[1:], sio) diff --git a/free/ci/cmd/agent/pull.go b/free/ci/cmd/agent/pull.go index 7bf5bc5c4..400a8d5a7 100644 --- a/free/ci/cmd/agent/pull.go +++ b/free/ci/cmd/agent/pull.go @@ -68,7 +68,7 @@ func parsePull(args []string, sio IO) (pullFlags, bool) { fs.BoolVar(&f.secretsOK, "secrets-eligible", false, "this node may receive project and environment secrets") fs.BoolVar(&f.dedicated, "dedicated", false, "this node is dedicated to the project (also release and deploy secrets)") if err := fs.Parse(args); err != nil || fs.NArg() != 0 || f.slots < 0 || (f.coordinator != "" && !idRE.MatchString(f.coordinator)) { - fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent pull [--slots N] [--coordinator ID] [--project DIR] [--secrets-eligible] [--dedicated]") + _, _ = fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent pull [--slots N] [--coordinator ID] [--project DIR] [--secrets-eligible] [--dedicated]") return f, false } return f, true @@ -111,31 +111,31 @@ func runPull(ctx context.Context, args []string, sio IO) int { return 2 } if err := RunPreStart(ctx, "pull"); err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } state, err := stateDir() if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } cred, err := enroll.LoadCredential(state) if errors.Is(err, os.ErrNotExist) { - fmt.Fprintln(sio.Stderr, "error: this node has not joined a coordinator; run `nself-ci-agent join ` first") + _, _ = fmt.Fprintln(sio.Stderr, "error: this node has not joined a coordinator; run `nself-ci-agent join ` first") return 1 } if err != nil { - fmt.Fprintln(sio.Stderr, "error: enrollment:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error: enrollment:", err) return 1 } base, err := url.Parse(cred.Enrollment.AgentURL) if err != nil || base.Scheme != "https" || base.Hostname() == "" { - fmt.Fprintln(sio.Stderr, "error: the enrollment holds no https agent URL; join again") + _, _ = fmt.Fprintln(sio.Stderr, "error: the enrollment holds no https agent URL; join again") return 1 } tlsc, err := pullTLS(state, cred, base.Hostname()) if err != nil { - fmt.Fprintln(sio.Stderr, "error: enrollment:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error: enrollment:", err) return 1 } if f.coordinator == "" { @@ -148,21 +148,23 @@ func runPull(ctx context.Context, args []string, sio IO) int { func pullLoop(ctx context.Context, f pullFlags, state string, cred enroll.Credential, baseURL string, tlsc *tls.Config, sio IO) int { key, err := nodekey.LoadOrCreate(state) if err != nil { - fmt.Fprintln(sio.Stderr, "error: node key:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error: node key:", err) return 1 } age, err := agekey.LoadOrCreate(ctx, state) if err != nil { - fmt.Fprintln(sio.Stderr, "error: age identity:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error: age identity:", err) return 1 } table, err := slots.Open(state, max(1, ciexec.Probe().CPUs/2)) if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } table.Lower(f.slots) - logf := func(format string, a ...any) { fmt.Fprintf(sio.Stderr, "nself-ci-agent pull: "+format+"\n", a...) } + logf := func(format string, a ...any) { + _, _ = fmt.Fprintf(sio.Stderr, "nself-ci-agent pull: "+format+"\n", a...) + } resolver := pullResolver(state, f, cred.Enrollment.NodeID, age.Path) for ctx.Err() == nil { car := agent.NewCarrier(agent.CarrierConfig{ @@ -178,7 +180,7 @@ func pullLoop(ctx context.Context, f pullFlags, state string, cred enroll.Creden Status: hostStatus, }) if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } began := time.Now() @@ -189,7 +191,7 @@ func pullLoop(ctx context.Context, f pullFlags, state string, cred enroll.Creden var rej *session.RejectError switch { case errors.As(err, &rej): - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 case watch.drained(): return 0 diff --git a/free/ci/cmd/agent/pull_e2e_test.go b/free/ci/cmd/agent/pull_e2e_test.go index 49e0db2d1..2a257e48f 100644 --- a/free/ci/cmd/agent/pull_e2e_test.go +++ b/free/ci/cmd/agent/pull_e2e_test.go @@ -133,7 +133,7 @@ func newPullRigWith(t *testing.T, o rigOpts) *pullRig { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() _ = srv.Shutdown(ctx) - <-done + _ = <-done }) return r } diff --git a/free/ci/cmd/agent/serve.go b/free/ci/cmd/agent/serve.go index 922e2b736..13d7e42ad 100644 --- a/free/ci/cmd/agent/serve.go +++ b/free/ci/cmd/agent/serve.go @@ -46,26 +46,26 @@ func runSlots(_ context.Context, args []string, sio IO) int { fs.SetOutput(sio.Stderr) set := fs.Int("set", 0, "set the host slot cap to N") if err := fs.Parse(args); err != nil || fs.NArg() != 0 || *set < 0 { - fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent slots [--set N]") + _, _ = fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent slots [--set N]") return 2 } state, err := stateDir() if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } if *set > 0 { if err := slots.SetCap(state, *set); err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } } capacity, used, err := slots.Status(state) if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } - fmt.Fprintf(sio.Stdout, "cap %d used %d\n", capacity, used) + _, _ = fmt.Fprintf(sio.Stdout, "cap %d used %d\n", capacity, used) return 0 } @@ -80,26 +80,26 @@ func runServe(ctx context.Context, args []string, sio IO) int { fingerprint := fs.String("root-fingerprint", "", "fingerprint of that coordinator's root") secretValues := fs.Bool("accept-secret-values", false, "accept value-bearing secrets frames (PERSONAL coordinator)") if err := fs.Parse(args); err != nil || !*stdio || fs.NArg() != 0 || !idRE.MatchString(*coordinator) { - fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent serve --stdio [--slots N] [--coordinator ID] [--root-fingerprint FP] [--accept-secret-values]") + _, _ = fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent serve --stdio [--slots N] [--coordinator ID] [--root-fingerprint FP] [--accept-secret-values]") return 2 } if err := RunPreStart(ctx, "serve"); err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } state, err := stateDir() if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } key, err := nodekey.LoadOrCreate(state) if err != nil { - fmt.Fprintln(sio.Stderr, "error: node key:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error: node key:", err) return 1 } age, err := agekey.LoadOrCreate(ctx, state) if err != nil { - fmt.Fprintln(sio.Stderr, "error: age identity:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error: age identity:", err) return 1 } capacity := ciexec.Probe() @@ -108,14 +108,14 @@ func runServe(ctx context.Context, args []string, sio IO) int { // only lower this process's share. table, err := slots.Open(state, max(1, capacity.CPUs/2)) if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } table.Lower(*nslots) policy := protocol.CarrierPolicy{Carrier: "A", Personal: *secretValues, OperatorOwned: *secretValues} car, err := session.NewStreamCarrier(sio.Stdin, sio.Stdout, policy, true) if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } car = withCheckout(car, state) @@ -127,11 +127,11 @@ func runServe(ctx context.Context, args []string, sio IO) int { Status: hostStatus, }) if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } if err := s.Run(ctx); err != nil && !errors.Is(err, context.Canceled) { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } return 0 diff --git a/free/ci/cmd/agent/uninstall.go b/free/ci/cmd/agent/uninstall.go index ea5573179..618c03bec 100644 --- a/free/ci/cmd/agent/uninstall.go +++ b/free/ci/cmd/agent/uninstall.go @@ -92,16 +92,16 @@ func runUninstall(ctx context.Context, args []string, sio IO) int { fs.SetOutput(sio.Stderr) system := fs.Bool("system", false, "remove the system install (users, sudoers rules, service, directories)") if err := fs.Parse(args); err != nil || fs.NArg() != 0 || !*system { - fmt.Fprintf(sio.Stderr, "usage: sudo %s uninstall --system (the root-owned copy of the agent)\n", darwin.TrustedPath) + _, _ = fmt.Fprintf(sio.Stderr, "usage: sudo %s uninstall --system (the root-owned copy of the agent)\n", darwin.TrustedPath) return 2 } f, ok := bootstrap.Lookup(runtime.GOOS) if !ok { - fmt.Fprintln(sio.Stderr, bootstrap.NewError("uninstall_unsupported", "no system install exists for %s; nothing to remove", runtime.GOOS)) + _, _ = fmt.Fprintln(sio.Stderr, bootstrap.NewError("uninstall_unsupported", "no system install exists for %s; nothing to remove", runtime.GOOS)) return 1 } if os.Geteuid() != 0 { - fmt.Fprintf(sio.Stderr, "error: uninstall --system must run as root: sudo %s uninstall --system\n", darwin.TrustedPath) + _, _ = fmt.Fprintf(sio.Stderr, "error: uninstall --system must run as root: sudo %s uninstall --system\n", darwin.TrustedPath) return 1 } if exe, err := os.Executable(); err != nil || trustedRootPath(exe) != nil { @@ -109,13 +109,13 @@ func runUninstall(ctx context.Context, args []string, sio IO) int { if err == nil { why = trustedRootPath(exe).Error() } - fmt.Fprintf(sio.Stderr, "error: refusing to run as root from a file the agent user can change (%s).\nRun the root-owned copy: sudo %s uninstall --system\n", why, darwin.TrustedPath) + _, _ = fmt.Fprintf(sio.Stderr, "error: refusing to run as root from a file the agent user can change (%s).\nRun the root-owned copy: sudo %s uninstall --system\n", why, darwin.TrustedPath) return 1 } o := bootstrap.Options{Run: localRun, Stdout: sio.Stdout, - Logf: func(format string, a ...any) { fmt.Fprintf(sio.Stderr, format+"\n", a...) }} + Logf: func(format string, a ...any) { _, _ = fmt.Fprintf(sio.Stderr, format+"\n", a...) }} if err := f.Uninstall(ctx, o); err != nil { - fmt.Fprintln(sio.Stderr, err) + _, _ = fmt.Fprintln(sio.Stderr, err) return 1 } return 0 diff --git a/free/ci/cmd/agent/upgrade.go b/free/ci/cmd/agent/upgrade.go index 5fc9e66c4..41b13547b 100644 --- a/free/ci/cmd/agent/upgrade.go +++ b/free/ci/cmd/agent/upgrade.go @@ -41,7 +41,7 @@ func runUpgrade(ctx context.Context, args []string, sio IO) int { manifestURL := fs.String("manifest-url", "", "https URL of this platform's .manifest.json (default: the GitHub release of --version)") allowDown := fs.Bool("allow-downgrade", false, "allow a version lower than this agent's") if err := fs.Parse(args); err != nil || fs.NArg() != 0 || !verify.ValidVersion(*version) { - fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent upgrade --version X.Y.Z [--manifest-url URL] [--allow-downgrade]") + _, _ = fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent upgrade --version X.Y.Z [--manifest-url URL] [--allow-downgrade]") return 2 } target := verify.Target(runtime.GOOS, runtime.GOARCH) @@ -49,7 +49,7 @@ func runUpgrade(ctx context.Context, args []string, sio IO) int { if *manifestURL != "" { u, err := url.Parse(*manifestURL) if err != nil || path.Base(u.Path) != install.BinaryName+"-"+target+".manifest.json" { - fmt.Fprintf(sio.Stderr, "E651: --manifest-url must name %s-%s.manifest.json\n", install.BinaryName, target) + _, _ = fmt.Fprintf(sio.Stderr, "E651: --manifest-url must name %s-%s.manifest.json\n", install.BinaryName, target) return 2 } u.Path = path.Dir(u.Path) @@ -57,7 +57,7 @@ func runUpgrade(ctx context.Context, args []string, sio IO) int { } state, err := stateDir() if err != nil { - fmt.Fprintln(sio.Stderr, "error:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error:", err) return 1 } exe, err := os.Executable() @@ -65,12 +65,12 @@ func runUpgrade(ctx context.Context, args []string, sio IO) int { exe, err = filepath.EvalSymlinks(exe) } if err != nil { - fmt.Fprintln(sio.Stderr, "error: agent binary:", err) + _, _ = fmt.Fprintln(sio.Stderr, "error: agent binary:", err) return 1 } v, err := upgradeVerifier() if err != nil { - fmt.Fprintln(sio.Stderr, err) + _, _ = fmt.Fprintln(sio.Stderr, err) return 1 } res, err := update.ApplyLocal(ctx, update.LocalOptions{ @@ -79,13 +79,13 @@ func runUpgrade(ctx context.Context, args []string, sio IO) int { Source: update.ReleaseSource{Release: install.Release{Version: *version, BaseURL: base, Verifier: v}}, }) if err != nil { - fmt.Fprintln(sio.Stderr, err) + _, _ = fmt.Fprintln(sio.Stderr, err) return 1 } if res.Noop { - fmt.Fprintf(sio.Stdout, "nself-ci-agent already runs %s\n", res.To) + _, _ = fmt.Fprintf(sio.Stdout, "nself-ci-agent already runs %s\n", res.To) return 0 } - fmt.Fprintf(sio.Stdout, "nself-ci-agent %s -> %s installed; the previous binary is kept as %s%s; restart the agent to run it\n", res.From, res.To, exe, update.PrevSuffix) + _, _ = fmt.Fprintf(sio.Stdout, "nself-ci-agent %s -> %s installed; the previous binary is kept as %s%s; restart the agent to run it\n", res.From, res.To, exe, update.PrevSuffix) return 0 } diff --git a/free/ci/cmd/agent/verify_release.go b/free/ci/cmd/agent/verify_release.go index 5e9733ba7..e1c2a9a69 100644 --- a/free/ci/cmd/agent/verify_release.go +++ b/free/ci/cmd/agent/verify_release.go @@ -46,12 +46,12 @@ func runVerifyRelease(_ context.Context, args []string, sio IO) int { return 2 } if *pubPath == "" || fs.NArg() != 1 { - fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent verify-release --pub FILE [--version V] [--tags-only] DIR") + _, _ = fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent verify-release --pub FILE [--version V] [--tags-only] DIR") return 2 } v, err := verifierFromPEM(*pubPath, *revokedFile) if err != nil { - fmt.Fprintf(sio.Stderr, "verify-release: %v\n", err) + _, _ = fmt.Fprintf(sio.Stderr, "verify-release: %v\n", err) return 1 } if *tagsOnly { @@ -117,7 +117,7 @@ func verifierFromPEM(path, revokedPath string) (*verify.Verifier, error) { func verifyReleaseDir(v *verify.Verifier, dir, version string, requireAll bool, stdout, stderr io.Writer) int { entries, err := os.ReadDir(dir) if err != nil { - fmt.Fprintln(stderr, "verify-release: cannot read the release directory") + _, _ = fmt.Fprintln(stderr, "verify-release: cannot read the release directory") return 1 } bins, manifests, sides := map[string]bool{}, map[string]bool{}, map[string]bool{} @@ -137,18 +137,18 @@ func verifyReleaseDir(v *verify.Verifier, dir, version string, requireAll bool, // (one with a manifest fails below as "manifest without a binary"). for name := range sides { if base := sideBase(name); !bins[base] && !manifests[base] { - fmt.Fprintf(stderr, "%s: %s: %s: %s has no binary in the directory\n", name, verify.CodeVerify, verify.ReasonProvenance, name) + _, _ = fmt.Fprintf(stderr, "%s: %s: %s: %s has no binary in the directory\n", name, verify.CodeVerify, verify.ReasonProvenance, name) return 1 } } if len(bins) == 0 { - fmt.Fprintln(stderr, "verify-release: no nself-ci-agent-- file in the directory") + _, _ = fmt.Fprintln(stderr, "verify-release: no nself-ci-agent-- file in the directory") return 1 } if requireAll { for _, t := range releaseTargets { if !bins["nself-ci-agent-"+t] { - fmt.Fprintf(stderr, "verify-release: %s: target %s is missing from the release\n", verify.CodeVerify, t) + _, _ = fmt.Fprintf(stderr, "verify-release: %s: target %s is missing from the release\n", verify.CodeVerify, t) return 1 } } @@ -171,7 +171,7 @@ func verifyReleaseDir(v *verify.Verifier, dir, version string, requireAll bool, failed := false for _, n := range names { if err := verifyOne(v, dir, n, bins[n], manifests[n], &version, stdout); err != nil { - fmt.Fprintf(stderr, "%s: %v\n", n, err) + _, _ = fmt.Fprintf(stderr, "%s: %v\n", n, err) failed = true } } @@ -213,7 +213,7 @@ func verifyOne(v *verify.Verifier, dir, name string, hasBin, hasManifest bool, v if err != nil { return err } - fmt.Fprintf(stdout, "ok %s %s %s\n", name, m.Version, m.KeyID) + _, _ = fmt.Fprintf(stdout, "ok %s %s %s\n", name, m.Version, m.KeyID) return nil } diff --git a/free/ci/cmd/agent/version.go b/free/ci/cmd/agent/version.go index 69387cb0f..1c57b287e 100644 --- a/free/ci/cmd/agent/version.go +++ b/free/ci/cmd/agent/version.go @@ -14,10 +14,10 @@ var Version = "1.5.0" func init() { must(RegisterSubcommand("version", "print the agent version", func(_ context.Context, args []string, sio IO) int { if len(args) != 0 { - fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent version") + _, _ = fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent version") return 2 } - fmt.Fprintf(sio.Stdout, "nself-ci-agent %s %s/%s\n", Version, runtime.GOOS, runtime.GOARCH) + _, _ = fmt.Fprintf(sio.Stdout, "nself-ci-agent %s %s/%s\n", Version, runtime.GOOS, runtime.GOARCH) return 0 })) } diff --git a/free/ci/cmd/approve.go b/free/ci/cmd/approve.go index 7955f897c..0e0fa961f 100644 --- a/free/ci/cmd/approve.go +++ b/free/ci/cmd/approve.go @@ -158,10 +158,10 @@ func runApprove(args []string, env approveEnv) int { return adminWriteData(env.adminEnv, "ci runs approve", out) } if !out.RunApproved { - fmt.Fprintf(env.Out, "spend approved for %s at %s by %s (the run needs no trust approval)\n", out.Run, out.HeadSHA, out.Approver) + _, _ = fmt.Fprintf(env.Out, "spend approved for %s at %s by %s (the run needs no trust approval)\n", out.Run, out.HeadSHA, out.Approver) return 0 } - fmt.Fprintf(env.Out, "approved %s at %s by %s (config %s)\n", out.Run, out.HeadSHA, out.Approver, out.ConfigDigest) + _, _ = fmt.Fprintf(env.Out, "approved %s at %s by %s (config %s)\n", out.Run, out.HeadSHA, out.Approver, out.ConfigDigest) return 0 } @@ -236,69 +236,3 @@ func authorToE688(err error) error { } return err } - -type pendingOut struct { - Run string `json:"run"` - Project string `json:"project"` - HeadSHA string `json:"head_sha"` - ConfigDigest string `json:"config_digest"` - HeadRepo string `json:"head_repo,omitempty"` - Author string `json:"author,omitempty"` - Reasons []string `json:"reasons"` -} - -func runPending(args []string, env approveEnv) int { - fs := flag.NewFlagSet("runs pending", flag.ContinueOnError) - fs.SetOutput(io.Discard) - asJSON := fs.Bool("json", false, "print one v1 JSON envelope") - pos, err := adminParse(fs, args) - if err == nil && len(pos) != 0 { - err = adminUsage("usage: nself ci runs pending [--json]") - } - if err != nil { - return approveFail(env, "ci runs pending", adminHasJSON(args), err) - } - ctx := context.Background() - svc, actor, done, err := env.Open(ctx) - if err == nil { - defer done() - err = actor.Can(roles.Member, "") - } - var list []pendingOut - if err == nil { - list, err = pendingRuns(ctx, svc.St) - } - if err != nil { - return approveFail(env, "ci runs pending", *asJSON, err) - } - if *asJSON { - return adminWriteData(env.adminEnv, "ci runs pending", list) - } - for _, p := range list { - fmt.Fprintf(env.Out, "%s %s %s %s\n", plainText(p.Run), plainText(p.HeadSHA), strings.Join(p.Reasons, ","), plainText(p.Project)) - } - if len(list) == 0 { - fmt.Fprintln(env.Out, "no run waits for approval") - } - return 0 -} - -// pendingRuns writes the voids of stale approvals, then lists the runs that still wait. -func pendingRuns(ctx context.Context, st *store.Store) ([]pendingOut, error) { - if _, err := st.SweepApprovals(ctx, "", voidCheck); err != nil { - return nil, err - } - held, err := st.PendingApprovals(ctx, approvalNeeded, voidCheck) - if err != nil { - return nil, err - } - list := make([]pendingOut, 0, len(held)) - for _, h := range held { - reasons := []string{source.ReasonApprovalRequired} - if h.HasApproval && h.Approval.VoidReason != "" { - reasons = append(reasons, strings.Split(h.Approval.VoidReason, ",")...) - } - list = append(list, pendingOut{Run: h.Run, Project: h.Project, HeadSHA: h.Revision, ConfigDigest: h.ConfigDigest, HeadRepo: h.HeadRepo, Author: h.Author, Reasons: reasons}) - } - return list, nil -} diff --git a/free/ci/cmd/approve_pending.go b/free/ci/cmd/approve_pending.go new file mode 100644 index 000000000..a02a6d9ce --- /dev/null +++ b/free/ci/cmd/approve_pending.go @@ -0,0 +1,79 @@ +package main + +// List pending approvals after voiding stale approval bindings. +import ( + "context" + "flag" + "fmt" + "github.com/nself-org/plugins/free/ci/internal/roles" + "github.com/nself-org/plugins/free/ci/internal/store" + "github.com/nself-org/plugins/free/ci/internal/trust/source" + "io" + "strings" +) + +type pendingOut struct { + Run string `json:"run"` + Project string `json:"project"` + HeadSHA string `json:"head_sha"` + ConfigDigest string `json:"config_digest"` + HeadRepo string `json:"head_repo,omitempty"` + Author string `json:"author,omitempty"` + Reasons []string `json:"reasons"` +} + +func runPending(args []string, env approveEnv) int { + fs := flag.NewFlagSet("runs pending", flag.ContinueOnError) + fs.SetOutput(io.Discard) + asJSON := fs.Bool("json", false, "print one v1 JSON envelope") + pos, err := adminParse(fs, args) + if err == nil && len(pos) != 0 { + err = adminUsage("usage: nself ci runs pending [--json]") + } + if err != nil { + return approveFail(env, "ci runs pending", adminHasJSON(args), err) + } + ctx := context.Background() + svc, actor, done, err := env.Open(ctx) + if err == nil { + defer done() + err = actor.Can(roles.Member, "") + } + var list []pendingOut + if err == nil { + list, err = pendingRuns(ctx, svc.St) + } + if err != nil { + return approveFail(env, "ci runs pending", *asJSON, err) + } + if *asJSON { + return adminWriteData(env.adminEnv, "ci runs pending", list) + } + for _, p := range list { + _, _ = fmt.Fprintf(env.Out, "%s %s %s %s\n", plainText(p.Run), plainText(p.HeadSHA), strings.Join(p.Reasons, ","), plainText(p.Project)) + } + if len(list) == 0 { + _, _ = fmt.Fprintln(env.Out, "no run waits for approval") + } + return 0 +} + +// pendingRuns writes the voids of stale approvals, then lists the runs that still wait. +func pendingRuns(ctx context.Context, st *store.Store) ([]pendingOut, error) { + if _, err := st.SweepApprovals(ctx, "", voidCheck); err != nil { + return nil, err + } + held, err := st.PendingApprovals(ctx, approvalNeeded, voidCheck) + if err != nil { + return nil, err + } + list := make([]pendingOut, 0, len(held)) + for _, h := range held { + reasons := []string{source.ReasonApprovalRequired} + if h.HasApproval && h.Approval.VoidReason != "" { + reasons = append(reasons, strings.Split(h.Approval.VoidReason, ",")...) + } + list = append(list, pendingOut{Run: h.Run, Project: h.Project, HeadSHA: h.Revision, ConfigDigest: h.ConfigDigest, HeadRepo: h.HeadRepo, Author: h.Author, Reasons: reasons}) + } + return list, nil +} diff --git a/free/ci/cmd/evidence.go b/free/ci/cmd/evidence.go index f9a66998c..fec3ea24d 100644 --- a/free/ci/cmd/evidence.go +++ b/free/ci/cmd/evidence.go @@ -169,9 +169,9 @@ func printEvidence(w io.Writer, run string, ev model.Evidence) { if ev.Freshness.Reason != nil { fresh += " (" + *ev.Freshness.Reason + ")" } - fmt.Fprintf(w, "run %s: %s, revision %.12s, %s, freshness %s\n", run, ev.Result, ev.Revision, ev.Binding.State, fresh) + _, _ = fmt.Fprintf(w, "run %s: %s, revision %.12s, %s, freshness %s\n", run, ev.Result, ev.Revision, ev.Binding.State, fresh) for _, c := range ev.Checks { - fmt.Fprintf(w, " %-6s %s\n", c.Result, c.ID) + _, _ = fmt.Fprintf(w, " %-6s %s\n", c.Result, c.ID) } } diff --git a/free/ci/cmd/explain.go b/free/ci/cmd/explain.go index 2b420cfe4..2454edcbd 100644 --- a/free/ci/cmd/explain.go +++ b/free/ci/cmd/explain.go @@ -106,7 +106,7 @@ func runExplain(args []string) int { } return 0 } - fmt.Fprintln(os.Stdout, strings.Join(planview.ExplainLines(doc, source), "\n")) + _, _ = fmt.Fprintln(os.Stdout, strings.Join(planview.ExplainLines(doc, source), "\n")) return 0 } diff --git a/free/ci/cmd/nodes_pair.go b/free/ci/cmd/nodes_pair.go index 743b86c1f..bf57b7ab9 100644 --- a/free/ci/cmd/nodes_pair.go +++ b/free/ci/cmd/nodes_pair.go @@ -84,7 +84,7 @@ func runNodesPair(args []string, stdin io.Reader, stdout, stderr io.Writer) int return 2 } if fs.NArg() != 0 { - fmt.Fprintln(stderr, "usage: nself ci nodes pair [--enroll-addr A] [--data-dir D] [--public-url U] [--label L] [--cert-ttl T]") + _, _ = fmt.Fprintln(stderr, "usage: nself ci nodes pair [--enroll-addr A] [--data-dir D] [--public-url U] [--label L] [--cert-ttl T]") return 2 } if f.dataDir != "" { @@ -94,20 +94,20 @@ func runNodesPair(args []string, stdin io.Reader, stdout, stderr io.Writer) int defer stop() d, closeFn, err := lifecycle.ProductionEnv(currentProject).Open(ctx, "") if err != nil { - fmt.Fprintf(stderr, "E607: the CI store cannot be opened: %v\n", err) + _, _ = fmt.Fprintf(stderr, "E607: the CI store cannot be opened: %v\n", err) return 2 } defer closeFn() ca, err := pki.New(ctx, pki.Options{Store: d.Store, DataDir: d.Home}) if err != nil { - fmt.Fprintf(stderr, "E660: the coordinator PKI cannot be opened: %v\n", err) + _, _ = fmt.Fprintf(stderr, "E660: the coordinator PKI cannot be opened: %v\n", err) return 2 } public := f.publicURL if public == "" { var uerr error if public, uerr = defaultPublicURL(f.addr, interfaceIPs()); uerr != nil { - fmt.Fprintf(stderr, "E660: %v\n", uerr) + _, _ = fmt.Fprintf(stderr, "E660: %v\n", uerr) return 2 } } @@ -122,27 +122,27 @@ func runNodesPair(args []string, stdin io.Reader, stdout, stderr io.Writer) int if a.Kind == "pairing.takeover_attempt" { what = "a code was used to claim a registered node's name; nothing was issued" } - fmt.Fprintf(stderr, "ALERT %s: offer %s node %s: %s\n", a.Kind, a.OfferID, a.NodeID, what) + _, _ = fmt.Fprintf(stderr, "ALERT %s: offer %s node %s: %s\n", a.Kind, a.OfferID, a.NodeID, what) }, }) pairing.Use(svc) w, err := svc.OpenWindow(ctx, f.addr, public, pairing.OfferOptions{Label: f.label, CertTTL: f.certTTL}) if err != nil { - fmt.Fprintf(stderr, "%v\n", err) + _, _ = fmt.Fprintf(stderr, "%v\n", err) return 2 } - fmt.Fprintf(stdout, "Pairing window open until %s.\n enroll URL: %s\n pairing code: %s\n root fingerprint: %s\nOn the node run: nself-ci-agent join %s\nThe node shows the root fingerprint; compare it with the one above before confirming.\n", + _, _ = fmt.Fprintf(stdout, "Pairing window open until %s.\n enroll URL: %s\n pairing code: %s\n root fingerprint: %s\nOn the node run: nself-ci-agent join %s\nThe node shows the root fingerprint; compare it with the one above before confirming.\n", w.Offer.ExpiresAt.Local().Format(time.TimeOnly), w.URL, w.Offer.Code, w.Offer.RootFingerprint, w.URL) redeemed, err := w.Wait(ctx, svc) switch { case err != nil: - fmt.Fprintf(stderr, "%v\n", err) + _, _ = fmt.Fprintf(stderr, "%v\n", err) return 2 case !redeemed: - fmt.Fprintln(stderr, "The pairing window closed without a node joining.") + _, _ = fmt.Fprintln(stderr, "The pairing window closed without a node joining.") return 1 } - fmt.Fprintln(stdout, "A node joined. The pairing window is closed.") + _, _ = fmt.Fprintln(stdout, "A node joined. The pairing window is closed.") return 0 } @@ -213,7 +213,7 @@ func rotateCertMode(ctx context.Context, d *lifecycle.Deps, r lifecycle.RotateRe if err != nil { return err } - fmt.Fprintf(d.Stdout, "%d certificate(s) of %s denied. The node must pair again: nself ci nodes pair, then nself-ci-agent join on the node.\n", n, r.NodeID) + _, _ = fmt.Fprintf(d.Stdout, "%d certificate(s) of %s denied. The node must pair again: nself ci nodes pair, then nself-ci-agent join on the node.\n", n, r.NodeID) return nil } @@ -232,6 +232,6 @@ func rotateNodeKeyMode(ctx context.Context, d *lifecycle.Deps, r lifecycle.Rotat if err = ca.RotateNodeKey(ctx, d.Actor, r.NodeID, id); err != nil { return err } - fmt.Fprintf(d.Stdout, "The next hello of %s may present node key %s only.\n", r.NodeID, id) + _, _ = fmt.Fprintf(d.Stdout, "The next hello of %s may present node key %s only.\n", r.NodeID, id) return nil } diff --git a/free/ci/cmd/nodes_secrets.go b/free/ci/cmd/nodes_secrets.go index 425fd5451..5af85f3c6 100644 --- a/free/ci/cmd/nodes_secrets.go +++ b/free/ci/cmd/nodes_secrets.go @@ -80,7 +80,7 @@ func dedicatedNode(c model.Capability) bool { } func runNodesSecrets(verb string, args []string, d nodesSecretsDeps) int { - fail := func(err error) int { fmt.Fprintf(d.Stderr, "Error: %v\n", err); return 1 } + fail := func(err error) int { _, _ = fmt.Fprintf(d.Stderr, "Error: %v\n", err); return 1 } var nodeID string if len(args) > 0 && args[0] != "" && args[0][0] != '-' { nodeID, args = args[0], args[1:] @@ -88,7 +88,7 @@ func runNodesSecrets(verb string, args []string, d nodesSecretsDeps) int { fs := nodesSecretsFlags() fs.SetOutput(io.Discard) if err := fs.Parse(args); err != nil || fs.NArg() != 0 || nodeID == "" { - fmt.Fprintf(d.Stderr, "usage: nself ci nodes secrets %s --class [/] [--project ]\n", verb) + _, _ = fmt.Fprintf(d.Stderr, "usage: nself ci nodes secrets %s --class [/] [--project ]\n", verb) return 1 } class, env, err := secrets.ParseClass(fs.Lookup("class").Value.String()) @@ -141,9 +141,9 @@ func runNodesSecrets(verb string, args []string, d nodesSecretsDeps) int { if err = st.AppendAudit(ctx, d.Actor, "node.secrets."+verb, nodeID, "class="+string(class)+";file="+file); err != nil { return fail(fmt.Errorf("the %s took effect on store file %s but its audit row could not be written: %w", verb, file, err)) } - fmt.Fprintf(d.Stdout, "%s: node %s, store file %s\n", verb, nodeID, file) + _, _ = fmt.Fprintf(d.Stdout, "%s: node %s, store file %s\n", verb, nodeID, file) if verb == "revoke" { - fmt.Fprintln(d.Stdout, "Removal does not revoke plaintext the node already read: rotate the values in that file.") + _, _ = fmt.Fprintln(d.Stdout, "Removal does not revoke plaintext the node already read: rotate the values in that file.") } return 0 } diff --git a/free/ci/cmd/policy_protect.go b/free/ci/cmd/policy_protect.go index ba98f8222..00f90308e 100644 --- a/free/ci/cmd/policy_protect.go +++ b/free/ci/cmd/policy_protect.go @@ -226,17 +226,17 @@ func (e protectEnv) print(asJSON bool, res protectResult, plan protect.Plan) err } switch { case res.AlreadySet: - fmt.Fprintf(e.out, "%s %s: nself-ci is already required; nothing to change\n", res.Repo, res.Branch) + _, _ = fmt.Fprintf(e.out, "%s %s: nself-ci is already required; nothing to change\n", res.Repo, res.Branch) case res.DryRun: - fmt.Fprintf(e.out, "%s %s: would add required check nself-ci (app %d)", res.Repo, res.Branch, res.Added[0].AppID) + _, _ = fmt.Fprintf(e.out, "%s %s: would add required check nself-ci (app %d)", res.Repo, res.Branch, res.Added[0].AppID) if res.Unprotected { - fmt.Fprint(e.out, "; the branch is not protected, so this creates protection holding only that check") + _, _ = fmt.Fprint(e.out, "; the branch is not protected, so this creates protection holding only that check") } - fmt.Fprintf(e.out, "\nPUT body:\n%s", plan.JSON()) + _, _ = fmt.Fprintf(e.out, "\nPUT body:\n%s", plan.JSON()) default: - fmt.Fprintf(e.out, "%s %s: nself-ci is now required (app %d); previous protection saved to %s\n", res.Repo, res.Branch, res.Added[0].AppID, res.Snapshot) + _, _ = fmt.Fprintf(e.out, "%s %s: nself-ci is now required (app %d); previous protection saved to %s\n", res.Repo, res.Branch, res.Added[0].AppID, res.Snapshot) if res.Unprotected { - fmt.Fprintln(e.out, "the branch had no protection; --restore cannot put that back (removing protection is a manual step)") + _, _ = fmt.Fprintln(e.out, "the branch had no protection; --restore cannot put that back (removing protection is a manual step)") } } return nil @@ -269,6 +269,6 @@ func (e protectEnv) restoreSnapshot(ctx context.Context, f protectFlags) error { } return e.noPlanHint(err) } - fmt.Fprintf(e.out, "%s %s: protection restored from %s; what it replaced is saved to %s\n", snap.Repo, snap.Branch, f.restore, before) + _, _ = fmt.Fprintf(e.out, "%s %s: protection restored from %s; what it replaced is saved to %s\n", snap.Repo, snap.Branch, f.restore, before) return nil } diff --git a/free/ci/cmd/policy_protect_ops.go b/free/ci/cmd/policy_protect_ops.go index 38b9d598d..dc2cd8608 100644 --- a/free/ci/cmd/policy_protect_ops.go +++ b/free/ci/cmd/policy_protect_ops.go @@ -42,12 +42,12 @@ func (e protectEnv) installHook(ctx context.Context, f protectFlags) error { return output.WriteData(e.out, "ci policy protect", hookResult{Path: path, Branch: branch, Installed: changed}, nil) } if changed { - fmt.Fprintf(e.out, "pre-push hook installed at %s: pushes to %s need current, bound, passing evidence (bypass once with `git push --no-verify`)\n", path, branch) + _, _ = fmt.Fprintf(e.out, "pre-push hook installed at %s: pushes to %s need current, bound, passing evidence (bypass once with `git push --no-verify`)\n", path, branch) if !strings.HasPrefix(path, e.root+string(filepath.Separator)) { - fmt.Fprintln(e.out, "note: this path is outside the repository (core.hooksPath); the hook applies to every repository that uses that directory") + _, _ = fmt.Fprintln(e.out, "note: this path is outside the repository (core.hooksPath); the hook applies to every repository that uses that directory") } } else { - fmt.Fprintf(e.out, "pre-push hook at %s is already installed\n", path) + _, _ = fmt.Fprintf(e.out, "pre-push hook at %s is already installed\n", path) } return nil } @@ -97,12 +97,12 @@ func (e protectEnv) audit(ctx context.Context, f protectFlags, repo string) int if f.json { _ = output.WriteData(e.out, "ci policy protect", rep, nil) } else { - fmt.Fprintf(e.out, "%s %s: %d commit(s) in the last %d day(s), %d without a green nself-ci\n", rep.Repo, rep.Branch, rep.Checked, rep.Days, len(rep.Missing)) + _, _ = fmt.Fprintf(e.out, "%s %s: %d commit(s) in the last %d day(s), %d without a green nself-ci\n", rep.Repo, rep.Branch, rep.Checked, rep.Days, len(rep.Missing)) for _, m := range rep.Missing { - fmt.Fprintf(e.out, " %.12s %s\n", m.SHA, m.Subject) + _, _ = fmt.Fprintf(e.out, " %.12s %s\n", m.SHA, m.Subject) } if rep.Truncated { - fmt.Fprintf(e.out, " (only the newest %d commits were looked at)\n", protect.MaxCommits) + _, _ = fmt.Fprintf(e.out, " (only the newest %d commits were looked at)\n", protect.MaxCommits) } } if len(rep.Missing) > 0 { diff --git a/free/ci/cmd/providers.go b/free/ci/cmd/providers.go index 0019de16c..dcb6383a2 100644 --- a/free/ci/cmd/providers.go +++ b/free/ci/cmd/providers.go @@ -91,7 +91,7 @@ func init() { registerV15("providers", subcommandEntry{V15Only: true, Flags: func() *flag.FlagSet { return flag.NewFlagSet("providers", flag.ContinueOnError) }, Handler: func(args []string) int { - fmt.Fprint(os.Stdout, providersHelp) + _, _ = fmt.Fprint(os.Stdout, providersHelp) if len(args) > 0 && args[0] != "--help" && args[0] != "-h" && args[0] != "help" { return 1 } diff --git a/free/ci/cmd/runs_reproduce.go b/free/ci/cmd/runs_reproduce.go index eb2e14589..7f00158db 100644 --- a/free/ci/cmd/runs_reproduce.go +++ b/free/ci/cmd/runs_reproduce.go @@ -139,19 +139,19 @@ func writeReproduceError(code string, err error, exit int) int { } func printReproduceReport(w io.Writer, r reproduce.Report) { - fmt.Fprintf(w, "reproduce %s at %.12s (%s, recorded on %s, now %s)\n", r.Job, r.Revision, r.Isolation, r.Platform, r.HostPlatform) + _, _ = fmt.Fprintf(w, "reproduce %s at %.12s (%s, recorded on %s, now %s)\n", r.Job, r.Revision, r.Isolation, r.Platform, r.HostPlatform) for _, d := range r.Drift { switch d.Kind { case reproduce.DriftToolGone: - fmt.Fprintf(w, " drift: %s is not available here (recorded %q)\n", d.Name, d.Recorded) + _, _ = fmt.Fprintf(w, " drift: %s is not available here (recorded %q)\n", d.Name, d.Recorded) case reproduce.DriftIsolation: - fmt.Fprintf(w, " drift: recorded isolation %s, running as %s\n", d.Recorded, d.Current) + _, _ = fmt.Fprintf(w, " drift: recorded isolation %s, running as %s\n", d.Recorded, d.Current) default: - fmt.Fprintf(w, " drift: %s %s: recorded %q, now %q\n", d.Kind, d.Name, d.Recorded, d.Current) + _, _ = fmt.Fprintf(w, " drift: %s %s: recorded %q, now %q\n", d.Kind, d.Name, d.Recorded, d.Current) } } for _, m := range r.Warnings { - fmt.Fprintln(w, " warning: "+m) + _, _ = fmt.Fprintln(w, " warning: "+m) } } diff --git a/free/ci/cmd/serve_admin.go b/free/ci/cmd/serve_admin.go index 015ee3cbc..fe63cb278 100644 --- a/free/ci/cmd/serve_admin.go +++ b/free/ci/cmd/serve_admin.go @@ -162,7 +162,7 @@ func init() { V15Only: true, Flags: func() *flag.FlagSet { return flag.NewFlagSet("serve admin", flag.ContinueOnError) }, Handler: func(args []string) int { - fmt.Fprint(os.Stdout, serveAdminHelp) + _, _ = fmt.Fprint(os.Stdout, serveAdminHelp) if len(args) > 0 && args[0] != "--help" && args[0] != "-h" && args[0] != "help" { return output.ExitUser } diff --git a/free/ci/cmd/v15_coordinator.go b/free/ci/cmd/v15_coordinator.go index 97be71841..f29c2e966 100644 --- a/free/ci/cmd/v15_coordinator.go +++ b/free/ci/cmd/v15_coordinator.go @@ -93,7 +93,7 @@ func coordinatorOptions(o *RunOptions) error { in.JSON, in.Out, in.Err = o.JSON, out, errw ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM, syscall.SIGHUP) defer stop() - fmt.Fprintf(errw, "nself ci: running on %s (%s)\n", firstOf(t.URL, t.Socket), t.Kind) + _, _ = fmt.Fprintf(errw, "nself ci: running on %s (%s)\n", firstOf(t.URL, t.Socket), t.Kind) res, err := cl.Run(ctx, in) if err != nil { coordExit(coordFail(out, errw, o.JSON, err)) @@ -154,7 +154,7 @@ func submitInput(o *RunOptions, errw io.Writer) (coordclient.RunInput, error) { return coordclient.RunInput{}, clientapi.Failf(clientapi.ReasonBadRequest, "remote origin of %s is not a github.com repository, so the coordinator cannot fetch it", o.Root) } if dirty, _ := coordGit(o.Root, "status", "--porcelain", "--untracked-files=no"); strings.TrimSpace(dirty) != "" { - fmt.Fprintln(errw, "warning: uncommitted changes are not part of the run; the coordinator runs the committed HEAD "+rev[:7]) + _, _ = fmt.Fprintln(errw, "warning: uncommitted changes are not part of the run; the coordinator runs the committed HEAD "+rev[:7]) } return coordclient.RunInput{Project: m[1] + "/" + m[2], Revision: rev, Pipeline: o.Pipeline}, nil } @@ -164,7 +164,7 @@ func submitInput(o *RunOptions, errw io.Writer) (coordclient.RunInput, error) { func coordFail(out, errw io.Writer, asJSON bool, err error) int { exit, class := output.ExitUser, "usage" if errors.Is(err, context.Canceled) { - fmt.Fprintln(errw, "interrupted; the run keeps going on the coordinator") + _, _ = fmt.Fprintln(errw, "interrupted; the run keeps going on the coordinator") return 130 } if r := clientapi.ReasonOf(err); r == clientapi.ReasonTransport || r == clientapi.ReasonServeDown || r == "" { @@ -176,6 +176,6 @@ func coordFail(out, errw io.Writer, asJSON bool, err error) int { return exit } } - fmt.Fprintln(errw, err) + _, _ = fmt.Fprintln(errw, err) return exit } diff --git a/free/ci/cmd/v15_detach.go b/free/ci/cmd/v15_detach.go index 6307f3e13..93cf285ba 100644 --- a/free/ci/cmd/v15_detach.go +++ b/free/ci/cmd/v15_detach.go @@ -95,7 +95,7 @@ func runDetach(o *RunOptions) int { } return 0 } - fmt.Fprintf(out, "run %s %s (pid %d)\n wait: %s\n follow: %s\n cancel: %s\n", h.RunID, h.State, h.PID, + _, _ = fmt.Fprintf(out, "run %s %s (pid %d)\n wait: %s\n follow: %s\n cancel: %s\n", h.RunID, h.State, h.PID, strings.Join(h.Commands.Wait, " "), strings.Join(h.Commands.Follow, " "), strings.Join(h.Commands.Cancel, " ")) return 0 } diff --git a/free/ci/internal/exec/detach.go b/free/ci/internal/exec/detach.go index b1c387708..c132e51e2 100644 --- a/free/ci/internal/exec/detach.go +++ b/free/ci/internal/exec/detach.go @@ -11,7 +11,6 @@ package exec // returns E648 reason spawn (Epic D35); no shell sits between parent and child. import ( - "errors" "fmt" "os" "os/exec" @@ -75,6 +74,3 @@ func StartDetached(argv, env []string, logPath string) (*Detached, error) { go func() { _ = cmd.Wait(); close(exited) }() return &Detached{PID: cmd.Process.Pid, Exited: exited}, nil } - -// errNoDetach is the cause the windows stub reports. -var errNoDetach = errors.New("detach is not supported on this platform") diff --git a/free/ci/internal/exec/detach_windows.go b/free/ci/internal/exec/detach_windows.go index 5e26eb7fa..9aa36406d 100644 --- a/free/ci/internal/exec/detach_windows.go +++ b/free/ci/internal/exec/detach_windows.go @@ -5,7 +5,10 @@ package exec // Purpose: windows stub of SpawnDetached (P7-CI-63, Epic D35): detach is not // supported, so it fails with E648 reason spawn and the package still builds. -import "os/exec" +import ( + "errors" + "os/exec" +) // applyDetach reports that detach is unsupported on windows. func applyDetach(*exec.Cmd) error { @@ -16,3 +19,6 @@ func applyDetach(*exec.Cmd) error { func TerminateDetached(int) error { return &Error{Code: detachCode, Message: "spawn: " + errNoDetach.Error(), Err: errNoDetach} } + +// errNoDetach is the cause the windows stub reports. +var errNoDetach = errors.New("detach is not supported on this platform") diff --git a/free/ci/internal/gates/truth/cli.go b/free/ci/internal/gates/truth/cli.go index f96dbedb1..4548e4de1 100644 --- a/free/ci/internal/gates/truth/cli.go +++ b/free/ci/internal/gates/truth/cli.go @@ -112,24 +112,24 @@ func cmdSweep(args []string, out, errw io.Writer) int { return 2 } if !revisionRe.MatchString(*revision) { - fmt.Fprintln(errw, "truth: --revision must be 40 lower-case hex digits") + _, _ = fmt.Fprintln(errw, "truth: --revision must be 40 lower-case hex digits") return 2 } files, repos := splitDash(fs.Args()) if len(files) == 0 || len(repos) == 0 { - fmt.Fprintln(errw, "usage: sweep --rules-root DIR --revision SHA RULEFILE... -- NAME=DIR|NAME=!reason...") + _, _ = fmt.Fprintln(errw, "usage: sweep --rules-root DIR --revision SHA RULEFILE... -- NAME=DIR|NAME=!reason...") return 2 } digest, err := digestFiles(*rulesRoot, files) if err != nil { - fmt.Fprintln(errw, failf(files[0], "", "%v", err)) + _, _ = fmt.Fprintln(errw, failf(files[0], "", "%v", err)) return 2 } var entries []SweepEntry for _, r := range repos { name, dir, ok := strings.Cut(r, "=") if !ok || name == "" { - fmt.Fprintf(errw, "truth: bad repository argument %q (want NAME=DIR or NAME=!reason)\n", r) + _, _ = fmt.Fprintf(errw, "truth: bad repository argument %q (want NAME=DIR or NAME=!reason)\n", r) return 2 } if reason, bad := strings.CutPrefix(dir, "!"); bad { @@ -140,17 +140,17 @@ func cmdSweep(args []string, out, errw io.Writer) int { } ev := SweepEvidence(*revision, digest, entries, time.Now()) for _, c := range ev.Checks { - fmt.Fprintf(errw, "%s %s (%d finding(s))\n", strings.ToUpper(string(c.Result)), c.ID, c.Findings) + _, _ = fmt.Fprintf(errw, "%s %s (%d finding(s))\n", strings.ToUpper(string(c.Result)), c.ID, c.Findings) if c.Result != "pass" && c.Excerpt != "" { - fmt.Fprintln(errw, indent(c.Excerpt)) + _, _ = fmt.Fprintln(errw, indent(c.Excerpt)) } } doc, err := json.Marshal(ev) if err != nil { - fmt.Fprintln(errw, "truth: cannot encode the evidence:", err) + _, _ = fmt.Fprintln(errw, "truth: cannot encode the evidence:", err) return 2 } - fmt.Fprintln(out, string(doc)) + _, _ = fmt.Fprintln(out, string(doc)) switch ev.Result { case "pass": return 0 @@ -187,12 +187,12 @@ func cmdHash(in io.Reader, out, errw io.Writer) int { n := 0 for sc.Scan() { if l := strings.TrimSpace(sc.Text()); l != "" { - fmt.Fprintln(out, HashLiteral(l)) + _, _ = fmt.Fprintln(out, HashLiteral(l)) n++ } } if sc.Err() != nil || n == 0 { - fmt.Fprintln(errw, "usage: hash < literals (one per line)") + _, _ = fmt.Fprintln(errw, "usage: hash < literals (one per line)") return 2 } return 0 diff --git a/free/ci/internal/gates/truth/report.go b/free/ci/internal/gates/truth/report.go index 272df6561..5d16d96d2 100644 --- a/free/ci/internal/gates/truth/report.go +++ b/free/ci/internal/gates/truth/report.go @@ -41,13 +41,13 @@ func matchText(f Finding) string { func report(checks []model.Check, found []Finding, out io.Writer) int { code := 0 for _, c := range checks { - fmt.Fprintf(out, "%s %s (%d finding(s))\n", strings.ToUpper(string(c.Result)), c.ID, c.Findings) + _, _ = fmt.Fprintf(out, "%s %s (%d finding(s))\n", strings.ToUpper(string(c.Result)), c.ID, c.Findings) if c.Result == "error" { - fmt.Fprintln(out, indent(c.Excerpt)) + _, _ = fmt.Fprintln(out, indent(c.Excerpt)) } for _, f := range found { if CheckIDOf(f.Kind) == c.ID { - fmt.Fprintf(out, "truth: %s %s:%d: %s\n fix: %s\n", f.RuleID, safe(f.Path), f.Line, matchText(f), f.Message) + _, _ = fmt.Fprintf(out, "truth: %s %s:%d: %s\n fix: %s\n", f.RuleID, safe(f.Path), f.Line, matchText(f), f.Message) } } switch { @@ -99,7 +99,7 @@ func reportJSON(checks []model.Check, found []Finding, out io.Writer) int { doc.Data.Findings = append(doc.Data.Findings, jsonFinding{Rule: f.RuleID, File: f.Path, Line: f.Line, Match: matchText(f), Message: f.Message}) } b, _ := json.Marshal(doc) - fmt.Fprintln(out, string(b)) + _, _ = fmt.Fprintln(out, string(b)) switch doc.Data.Result { case "error": return 2 diff --git a/free/ci/internal/nodes/lifecycle/separation.go b/free/ci/internal/nodes/lifecycle/separation.go index 07c6352ab..e5d5ea4cd 100644 --- a/free/ci/internal/nodes/lifecycle/separation.go +++ b/free/ci/internal/nodes/lifecycle/separation.go @@ -66,7 +66,7 @@ func keyOr(v, def string) string { return def } for _, c := range v { - if !(c >= 'a' && c <= 'z' || c >= '0' && c <= '9' || c == '.' || c == '_') { + if (c < 'a' || c > 'z') && (c < '0' || c > '9') && c != '.' && c != '_' { return def } } diff --git a/free/ci/internal/nodes/provision/cli.go b/free/ci/internal/nodes/provision/cli.go index 302c342bc..05644957a 100644 --- a/free/ci/internal/nodes/provision/cli.go +++ b/free/ci/internal/nodes/provision/cli.go @@ -79,7 +79,7 @@ func finish(d Deps, err error) int { if err == nil { return 0 } - fmt.Fprintf(d.Err, "Error: %v\n", err) + _, _ = fmt.Fprintf(d.Err, "Error: %v\n", err) var coder interface{ ExitCode() int } if errors.As(err, &coder) { return coder.ExitCode() @@ -155,12 +155,12 @@ func RunProvision(args []string, d Deps) int { } } if !jsonOut { - fmt.Fprintf(d.Out, "Provisioning %s (%d instance(s))...\n", ex.Label(), opts.Instances) + _, _ = fmt.Fprintf(d.Out, "Provisioning %s (%d instance(s))...\n", ex.Label(), opts.Instances) } result, err := Provision(d.Ctx, ex, m, opts) if !jsonOut { for _, step := range result.Steps { - fmt.Fprintf(d.Out, " [%s] %s\n", step.Name, step.Output) + _, _ = fmt.Fprintf(d.Out, " [%s] %s\n", step.Name, step.Output) } } if err != nil { @@ -183,7 +183,7 @@ func RunProvision(args []string, d Deps) int { } return 0 } - fmt.Fprintln(d.Out, "Provision complete.") + _, _ = fmt.Fprintln(d.Out, "Provision complete.") return 0 } diff --git a/free/ci/internal/run/hooks.go b/free/ci/internal/run/hooks.go index 95259405b..3ee65f8b2 100644 --- a/free/ci/internal/run/hooks.go +++ b/free/ci/internal/run/hooks.go @@ -11,7 +11,6 @@ package run import ( "context" - "sort" ciexec "github.com/nself-org/plugins/free/ci/internal/exec" "github.com/nself-org/plugins/free/ci/internal/model" @@ -225,78 +224,3 @@ type Hooks struct { AttemptFinalizes []AttemptFinalizeHook Auto AutoHooks // PublishGate, PublishDetail, Offload, Race, Notify (hooks_auto.go) } - -// Registered is the registry other packages write from init(). -var Registered Hooks - -// Register helpers mirror the Hooks fields; a second registration of a -// single-slot hook replaces the first. -func RegisterRemote(r RemoteResolver) { Registered.Remote = append(Registered.Remote, r) } -func RegisterTrust(c TrustClassifier) { Registered.Trust = c } -func RegisterPolicy(p PolicyResolver) { Registered.Policy = p } -func RegisterFinalizer(f Finalizer) { Registered.Finalizers = append(Registered.Finalizers, f) } -func RegisterCache(c CacheHook) { Registered.Cache = c } -func RegisterServices(s ServicesHook) { Registered.Services = s } -func RegisterAdhoc(a AdhocHandler) { Registered.Adhoc = a } -func RegisterImage(i ContainerImageHook) { Registered.Image = i } -func RegisterPublisher(p Publisher) { Registered.Publisher = p } -func RegisterIsolation(i ciexec.Isolator) { Registered.Isolation = i } -func RegisterEvidence(b EvidenceBuilder) { Registered.Evidence = b } -func RegisterLogs(h LogHook) { Registered.Logs = h } -func RegisterAttemptStart(h AttemptStartHook) { - Registered.AttemptStarts = append(Registered.AttemptStarts, h) -} -func RegisterAfterRun(h AfterRunHook) { Registered.AfterRuns = append(Registered.AfterRuns, h) } - -// RegisterRedactor sets the per-attempt Redactor factory (single slot). -func RegisterRedactor(f func() ciexec.Redactor) { Registered.NewRedactor = f } - -// Resolved fills every nil single-slot field from Registered and appends the -// registered lists, so a test can add a fake on top of the registry. -func (h Hooks) Resolved() Hooks { - r := Registered - out := h - if out.Trust == nil { - out.Trust = r.Trust - } - if out.Policy == nil { - out.Policy = r.Policy - } - if out.Cache == nil { - out.Cache = r.Cache - } - if out.Services == nil { - out.Services = r.Services - } - if out.Adhoc == nil { - out.Adhoc = r.Adhoc - } - if out.Image == nil { - out.Image = r.Image - } - if out.Publisher == nil { - out.Publisher = r.Publisher - } - if out.Isolation == nil { - out.Isolation = r.Isolation - } - if out.NewRedactor == nil { - out.NewRedactor = r.NewRedactor - } - if out.Evidence == nil { - out.Evidence = r.Evidence - } - if out.Logs == nil { - out.Logs = r.Logs - } - out.Remote = append(append([]RemoteResolver(nil), h.Remote...), r.Remote...) - out.Finalizers = append(append([]Finalizer(nil), h.Finalizers...), r.Finalizers...) - out.AttemptStarts = append(append([]AttemptStartHook(nil), h.AttemptStarts...), r.AttemptStarts...) - out.AfterRuns = append(append([]AfterRunHook(nil), h.AfterRuns...), r.AfterRuns...) - out.Outcomes = append(append([]OutcomeHook(nil), h.Outcomes...), r.Outcomes...) - out.Workspaces = append(append([]WorkspaceHook(nil), h.Workspaces...), r.Workspaces...) - out.AttemptFinalizes = append(append([]AttemptFinalizeHook(nil), h.AttemptFinalizes...), r.AttemptFinalizes...) - out.Auto = h.Auto.resolved(r.Auto) - sort.SliceStable(out.Finalizers, func(i, j int) bool { return out.Finalizers[i].Name() < out.Finalizers[j].Name() }) - return out -} diff --git a/free/ci/internal/run/hooks_registry.go b/free/ci/internal/run/hooks_registry.go new file mode 100644 index 000000000..6be39e528 --- /dev/null +++ b/free/ci/internal/run/hooks_registry.go @@ -0,0 +1,82 @@ +package run + +import ( + "sort" + + ciexec "github.com/nself-org/plugins/free/ci/internal/exec" +) + +// Registered is the registry other packages write from init(). +var Registered Hooks + +// Register helpers mirror the Hooks fields; a second registration of a +// single-slot hook replaces the first. +func RegisterRemote(r RemoteResolver) { Registered.Remote = append(Registered.Remote, r) } +func RegisterTrust(c TrustClassifier) { Registered.Trust = c } +func RegisterPolicy(p PolicyResolver) { Registered.Policy = p } +func RegisterFinalizer(f Finalizer) { Registered.Finalizers = append(Registered.Finalizers, f) } +func RegisterCache(c CacheHook) { Registered.Cache = c } +func RegisterServices(s ServicesHook) { Registered.Services = s } +func RegisterAdhoc(a AdhocHandler) { Registered.Adhoc = a } +func RegisterImage(i ContainerImageHook) { Registered.Image = i } +func RegisterPublisher(p Publisher) { Registered.Publisher = p } +func RegisterIsolation(i ciexec.Isolator) { Registered.Isolation = i } +func RegisterEvidence(b EvidenceBuilder) { Registered.Evidence = b } +func RegisterLogs(h LogHook) { Registered.Logs = h } +func RegisterAttemptStart(h AttemptStartHook) { + Registered.AttemptStarts = append(Registered.AttemptStarts, h) +} +func RegisterAfterRun(h AfterRunHook) { Registered.AfterRuns = append(Registered.AfterRuns, h) } + +// RegisterRedactor sets the per-attempt Redactor factory (single slot). +func RegisterRedactor(f func() ciexec.Redactor) { Registered.NewRedactor = f } + +// Resolved fills every nil single-slot field from Registered and appends the +// registered lists, so a test can add a fake on top of the registry. +func (h Hooks) Resolved() Hooks { + r := Registered + out := h + if out.Trust == nil { + out.Trust = r.Trust + } + if out.Policy == nil { + out.Policy = r.Policy + } + if out.Cache == nil { + out.Cache = r.Cache + } + if out.Services == nil { + out.Services = r.Services + } + if out.Adhoc == nil { + out.Adhoc = r.Adhoc + } + if out.Image == nil { + out.Image = r.Image + } + if out.Publisher == nil { + out.Publisher = r.Publisher + } + if out.Isolation == nil { + out.Isolation = r.Isolation + } + if out.NewRedactor == nil { + out.NewRedactor = r.NewRedactor + } + if out.Evidence == nil { + out.Evidence = r.Evidence + } + if out.Logs == nil { + out.Logs = r.Logs + } + out.Remote = append(append([]RemoteResolver(nil), h.Remote...), r.Remote...) + out.Finalizers = append(append([]Finalizer(nil), h.Finalizers...), r.Finalizers...) + out.AttemptStarts = append(append([]AttemptStartHook(nil), h.AttemptStarts...), r.AttemptStarts...) + out.AfterRuns = append(append([]AfterRunHook(nil), h.AfterRuns...), r.AfterRuns...) + out.Outcomes = append(append([]OutcomeHook(nil), h.Outcomes...), r.Outcomes...) + out.Workspaces = append(append([]WorkspaceHook(nil), h.Workspaces...), r.Workspaces...) + out.AttemptFinalizes = append(append([]AttemptFinalizeHook(nil), h.AttemptFinalizes...), r.AttemptFinalizes...) + out.Auto = h.Auto.resolved(r.Auto) + sort.SliceStable(out.Finalizers, func(i, j int) bool { return out.Finalizers[i].Name() < out.Finalizers[j].Name() }) + return out +} diff --git a/free/ci/internal/run/render.go b/free/ci/internal/run/render.go index 82e11cad5..0246d3e46 100644 --- a/free/ci/internal/run/render.go +++ b/free/ci/internal/run/render.go @@ -151,22 +151,22 @@ func (h *human) Event(e Event) { switch e.Type { case "log": if h.stream { - fmt.Fprintf(h.w, "[%s] %s\n", e.Job, strings.TrimRight(e.Text, "\r\n")) + _, _ = fmt.Fprintf(h.w, "[%s] %s\n", e.Job, strings.TrimRight(e.Text, "\r\n")) } case "finished": o := e.Out switch o.State { case StatePassed: - fmt.Fprintf(h.w, " pass %s (%s)\n", o.ID, o.Duration.Round(time.Millisecond)) + _, _ = fmt.Fprintf(h.w, " pass %s (%s)\n", o.ID, o.Duration.Round(time.Millisecond)) case StateFailed: - fmt.Fprintf(h.w, " FAIL %s (%s)\n", o.ID, failText(*o)) + _, _ = fmt.Fprintf(h.w, " FAIL %s (%s)\n", o.ID, failText(*o)) if o.Tail != "" && !h.stream { for _, l := range strings.Split(strings.TrimRight(o.Tail, "\n"), "\n") { - fmt.Fprintf(h.w, " | %s\n", l) + _, _ = fmt.Fprintf(h.w, " | %s\n", l) } } default: - fmt.Fprintf(h.w, " %-8s %s: %s\n", o.State, o.ID, reasonText(*o)) + _, _ = fmt.Fprintf(h.w, " %-8s %s: %s\n", o.State, o.ID, reasonText(*o)) } } } @@ -201,7 +201,7 @@ func (h *human) Done(s Summary) { skipped := append([]plan.Skipped(nil), s.Skipped...) sort.Slice(skipped, func(i, j int) bool { return skipped[i].Job < skipped[j].Job }) for _, k := range skipped { - fmt.Fprintf(h.w, " skipped %s: %s\n", k.Job, k.Reason) + _, _ = fmt.Fprintf(h.w, " skipped %s: %s\n", k.Job, k.Reason) } pass, total := 0, 0 for _, j := range s.Jobs { @@ -210,11 +210,11 @@ func (h *human) Done(s Summary) { pass++ } } - fmt.Fprintf(h.w, "nself-ci: %s (%d/%d jobs passed) run %s\n", s.Result, pass, total, s.RunID) + _, _ = fmt.Fprintf(h.w, "nself-ci: %s (%d/%d jobs passed) run %s\n", s.Result, pass, total, s.RunID) for _, n := range s.Notes { - fmt.Fprintln(h.w, "nself-ci: note: "+n) + _, _ = fmt.Fprintln(h.w, "nself-ci: note: "+n) } if s.Status != "" { - fmt.Fprintln(h.w, s.Status) + _, _ = fmt.Fprintln(h.w, s.Status) } } From 685c6eb40164624496a6075e94201c36fd37c6d8 Mon Sep 17 00:00:00 2001 From: admin Date: Tue, 20 Oct 2026 08:27:43 -0400 Subject: [PATCH 19/19] fix(ci): bound agent termination below the transport cancellation deadline --- free/ci/cmd/agent/exec.go | 4 +- free/ci/cmd/agent/exec_cancel_test.go | 57 +++++++++++++++++++++++++++ free/ci/cmd/agent/pull_e2e_test.go | 6 ++- 3 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 free/ci/cmd/agent/exec_cancel_test.go diff --git a/free/ci/cmd/agent/exec.go b/free/ci/cmd/agent/exec.go index e78f9bca7..802cd8fe3 100644 --- a/free/ci/cmd/agent/exec.go +++ b/free/ci/cmd/agent/exec.go @@ -211,7 +211,9 @@ func execute(ctx context.Context, spec execSpec, in jobInputs) (ciexec.AttemptRe if iso == nil { return ciexec.AttemptResult{}, "", errors.New("no isolator is registered; the agent will not run a job without one") } - ex := &ciexec.Executor{Env: ciexec.AllowlistEnv{}, NewRedactor: ciexec.NewOutputRedactor, Stale: agentStale{}, Isolation: iso} + // The transport expects a cancelled result within five seconds. Leave time + // for evidence and cleanup after escalating SIGTERM to SIGKILL. + ex := &ciexec.Executor{Env: ciexec.AllowlistEnv{}, NewRedactor: ciexec.NewOutputRedactor, Stale: agentStale{}, Isolation: iso, Grace: time.Second} if len(in.Secrets) > 0 { ex.Secrets = mapSecrets(in.Secrets) } diff --git a/free/ci/cmd/agent/exec_cancel_test.go b/free/ci/cmd/agent/exec_cancel_test.go new file mode 100644 index 000000000..d06af4099 --- /dev/null +++ b/free/ci/cmd/agent/exec_cancel_test.go @@ -0,0 +1,57 @@ +package main + +import ( + "context" + "strings" + "testing" + "time" + + ciexec "github.com/nself-org/plugins/free/ci/internal/exec" + "github.com/nself-org/plugins/free/ci/internal/model" +) + +func TestAgentCancellationFitsTransportResultDeadline(t *testing.T) { + skipUnixOnly(t) + for _, tc := range []struct{ name, script string }{ + {"cooperative", "echo cancel-ready; sleep 60 & wait"}, + {"ignores TERM", "trap '' TERM; echo cancel-ready; sleep 60 & wait"}, + } { + t.Run(tc.name, func(t *testing.T) { + useState(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + started := make(chan struct{}, 1) + done := make(chan struct{}) + var res ciexec.AttemptResult + var err error + go func() { + defer close(done) + res, _, err = execute(ctx, execSpec{AttemptID: "cancel-bound", CoordinatorID: "cancel-coord", Job: model.Job{Kind: "static", Isolation: "process", Run: []string{"sh", "-c", tc.script}}}, jobInputs{Output: func(b []byte) { + if strings.Contains(string(b), "cancel-ready") { + select { + case started <- struct{}{}: + default: + } + } + }}) + }() + defer func() { cancel(); <-done }() + select { + case <-started: + case <-done: + t.Fatalf("job exited before cancellation: %+v %v", res, err) + case <-time.After(15 * time.Second): + t.Fatal("job did not start") + } + cancel() + select { + case <-done: + if err != nil || res.Attempt.State != "cancelled" { + t.Fatalf("cancel result: %+v %v", res, err) + } + case <-time.After(5 * time.Second): + t.Fatal("cancelled result missed the transport deadline") + } + }) + } +} diff --git a/free/ci/cmd/agent/pull_e2e_test.go b/free/ci/cmd/agent/pull_e2e_test.go index 2a257e48f..5b899fdf2 100644 --- a/free/ci/cmd/agent/pull_e2e_test.go +++ b/free/ci/cmd/agent/pull_e2e_test.go @@ -14,7 +14,9 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/json" + "errors" "net" + "net/http" "os" "path/filepath" "strings" @@ -133,7 +135,9 @@ func newPullRigWith(t *testing.T, o rigOpts) *pullRig { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() _ = srv.Shutdown(ctx) - _ = <-done + if err := <-done; err != nil && !errors.Is(err, http.ErrServerClosed) { + t.Errorf("agent server shutdown: %v", err) + } }) return r }