From 1c30b10b4dd9db424124a52d73b0bd5e69ee5b83 Mon Sep 17 00:00:00 2001 From: admin Date: Tue, 20 Oct 2026 08:01:00 -0400 Subject: [PATCH 01/23] test(ci): make production-marker fixture independent of host Docker --- free/ci/cmd/agent/prodguard_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/free/ci/cmd/agent/prodguard_test.go b/free/ci/cmd/agent/prodguard_test.go index d0ae18cde..4017d2fa0 100644 --- a/free/ci/cmd/agent/prodguard_test.go +++ b/free/ci/cmd/agent/prodguard_test.go @@ -69,6 +69,12 @@ func TestServeStdioExitsNonZeroWithE663OnAProdStackMarker(t *testing.T) { } t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) 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) } From d53dacb094297d654a9d72b73f4e3d423f2347b0 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Mon, 19 Oct 2026 18:20:35 -0400 Subject: [PATCH 02/23] fix(plugins): nself-ci signer cli children get NSELF_V15=1 (D-1236) The exec funnel allowlists the child environment, so the signer's config secrets resolve and set calls ran without NSELF_V15 and failed on cli 1.4.x with E717. ExecRunner gains a base Env; the signer's resolver and key store pass NSELF_V15=1. --- free/ci/cmd/artifacts_signer.go | 8 ++- free/ci/cmd/artifacts_signer_env_test.go | 71 ++++++++++++++++++++++++ free/ci/internal/secrets/runner_exec.go | 4 +- 3 files changed, 80 insertions(+), 3 deletions(-) create mode 100644 free/ci/cmd/artifacts_signer_env_test.go diff --git a/free/ci/cmd/artifacts_signer.go b/free/ci/cmd/artifacts_signer.go index 2ecec911b..2ecd5bf24 100644 --- a/free/ci/cmd/artifacts_signer.go +++ b/free/ci/cmd/artifacts_signer.go @@ -106,9 +106,13 @@ func countTrue(b ...bool) (n int) { return n } +// signerCLIEnv is added to every cli child of the signer: the funnel allowlists the +// environment, and the released cli (1.4.x) has `config secrets resolve` only with NSELF_V15=1 (D-1236). +var signerCLIEnv = []string{"NSELF_V15=1"} + // signerResolver resolves release secrets through the exec funnel and the cli. func signerResolver(root string) secrets.ValueResolver { - return &secrets.Resolver{Runner: &secrets.ExecRunner{Home: os.Getenv("NSELF_CI_HOME"), CoordinatorID: "signer", Attempt: "key"}, Project: root} + return &secrets.Resolver{Runner: &secrets.ExecRunner{Home: os.Getenv("NSELF_CI_HOME"), CoordinatorID: "signer", Attempt: "key", Env: signerCLIEnv}, Project: root} } // signerStore writes the key with `nself config secrets set` over stdin. @@ -120,7 +124,7 @@ func signerStore(root string) (signing.SecretStore, error) { if err != nil { return nil, err } - return &signing.CLIStore{Runner: &signing.FunnelRunner{Home: os.Getenv("NSELF_CI_HOME")}, Project: root, LockDir: filepath.Join(home, ".nself", "locks")}, nil + return &signing.CLIStore{Runner: &signing.FunnelRunner{Home: os.Getenv("NSELF_CI_HOME")}, Project: root, Env: signerCLIEnv, LockDir: filepath.Join(home, ".nself", "locks")}, nil } func signerRequest(ctx context.Context, root string) int { diff --git a/free/ci/cmd/artifacts_signer_env_test.go b/free/ci/cmd/artifacts_signer_env_test.go new file mode 100644 index 000000000..8341b2642 --- /dev/null +++ b/free/ci/cmd/artifacts_signer_env_test.go @@ -0,0 +1,71 @@ +package main + +// Purpose: the signer's cli children get NSELF_V15=1 (D-1236). The released cli 1.4.x has +// `config secrets resolve` only with it, and the exec funnel strips the caller's environment. +// Inputs: the fake cli of artifacts_key_test.go behind a wrapper that gates resolve like cli 1.4. +// Outputs: pass or fail. +// Constraints: unix only (shell wrapper). + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/nself-org/plugins/free/ci/internal/secrets" +) + +// gateResolveOnV15 puts a wrapper in front of the fake cli: `config secrets resolve` answers exit 2 +// (unknown command) unless the child has NSELF_V15=1, as a released cli 1.4.x does. +func gateResolveOnV15(t *testing.T) { + t.Helper() + real, err := exec.LookPath("nself") + if err != nil { + t.Fatal(err) + } + if err = os.Rename(real, real+".real"); err != nil { + t.Fatal(err) + } + script := "#!/bin/sh\nif [ \"$1 $2 $3\" = \"config secrets resolve\" ] && [ \"$NSELF_V15\" != 1 ]; then exit 2; fi\nexec '" + real + ".real' \"$@\"\n" + if err = os.WriteFile(real, []byte(script), 0o755); err != nil { + t.Fatal(err) + } +} + +func TestSignerResolverChildSeesV15(t *testing.T) { + for _, tc := range []struct { + name string + gated bool + env string // the caller's own NSELF_V15 (the funnel must not depend on it) + }{ + {"cli 1.4 gates resolve, caller env stripped", true, ""}, + {"cli 1.4 gates resolve, caller env set", true, "1"}, + {"cli without the gate", false, ""}, + } { + t.Run(tc.name, func(t *testing.T) { + dir := fakeNself(t) + if tc.gated { + gateResolveOnV15(t) + } + t.Setenv("NSELF_V15", tc.env) + t.Setenv("NSELF_CI_HOME", t.TempDir()) + if err := os.WriteFile(filepath.Join(dir, "RELEASE_KEY"), []byte("k"), 0o600); err != nil { + t.Fatal(err) + } + res := signerResolver(t.TempDir()) + ctx := context.Background() + if err := res.Probe(ctx); err != nil { + t.Fatalf("probe: %v", err) + } + ref, err := secrets.Parse("release/RELEASE_KEY") + if err != nil { + t.Fatal(err) + } + vals, err := res.Resolve(ctx, "key", []secrets.Ref{ref}) + if err != nil || len(vals) != 1 || string(vals[0].Bytes) != "k" { + t.Fatalf("resolve: %v %v", vals, err) + } + }) + } +} diff --git a/free/ci/internal/secrets/runner_exec.go b/free/ci/internal/secrets/runner_exec.go index 7d21e6f45..e34dbefe8 100644 --- a/free/ci/internal/secrets/runner_exec.go +++ b/free/ci/internal/secrets/runner_exec.go @@ -26,6 +26,8 @@ type ExecRunner struct { CoordinatorID string // Attempt is the attempt the calls serve; it prefixes the funnel's own call ids. Attempt string + // Env is added to the environment of every call (a call's own Env follows it). + Env []string seq atomic.Uint64 } @@ -83,7 +85,7 @@ func (x *ExecRunner) Run(ctx context.Context, c Call) (Output, error) { if coord == "" { coord = "secrets" } - ex := &ciexec.Executor{Isolation: ciexec.BaselineIsolator{}, Env: callEnv(c.Env), NewRedactor: func() ciexec.Redactor { return passRedactor{} }} + ex := &ciexec.Executor{Isolation: ciexec.BaselineIsolator{}, Env: callEnv(append(append([]string(nil), x.Env...), c.Env...)), NewRedactor: func() ciexec.Redactor { return passRedactor{} }} var cap capture res := ex.Run(ctx, ciexec.JobSpec{AttemptID: x.id(), CoordinatorID: coord, Command: c.Argv, Home: x.Home, Timeout: c.Timeout, Output: cap.write}) out := Output{Bytes: cap.buf, ExitCode: res.ExitCode} From 29d70a102af85b0e1df0c79c54a1741a795f6c5e Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Mon, 19 Oct 2026 18:33:51 -0400 Subject: [PATCH 03/23] test(plugins): the signer key store passes NSELF_V15=1 too (D-1236) --- free/ci/cmd/artifacts_signer_env_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/free/ci/cmd/artifacts_signer_env_test.go b/free/ci/cmd/artifacts_signer_env_test.go index 8341b2642..b3e19c98a 100644 --- a/free/ci/cmd/artifacts_signer_env_test.go +++ b/free/ci/cmd/artifacts_signer_env_test.go @@ -11,9 +11,11 @@ import ( "os" "os/exec" "path/filepath" + "slices" "testing" "github.com/nself-org/plugins/free/ci/internal/secrets" + "github.com/nself-org/plugins/free/ci/internal/signing" ) // gateResolveOnV15 puts a wrapper in front of the fake cli: `config secrets resolve` answers exit 2 @@ -69,3 +71,16 @@ func TestSignerResolverChildSeesV15(t *testing.T) { }) } } + +// The key store the signer writes with passes the same environment to its cli children. +func TestSignerStoreChildSeesV15(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + st, err := signerStore(t.TempDir()) + if err != nil { + t.Fatal(err) + } + cs, ok := st.(*signing.CLIStore) + if !ok || !slices.Contains(cs.Env, "NSELF_V15=1") { + t.Fatalf("store %T env %v", st, cs) + } +} From a511458f475162b38b615a59127eb4e1650d8fbb Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Mon, 19 Oct 2026 18:24:37 -0400 Subject: [PATCH 04/23] test(plugins): admission refuses a revoked node from the real grant path (D-1086) AdmitPlacement re-reads the node row inside the admission transaction (state and authorization), so the grant path already refuses a revoked node. The recheck table lacked the revoked case; add it. --- free/ci/internal/dispatch/recheck_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/free/ci/internal/dispatch/recheck_test.go b/free/ci/internal/dispatch/recheck_test.go index a29e8eab0..9da0a9de1 100644 --- a/free/ci/internal/dispatch/recheck_test.go +++ b/free/ci/internal/dispatch/recheck_test.go @@ -15,8 +15,8 @@ import ( "github.com/nself-org/plugins/free/ci/internal/store" ) -// TestDispatchNodeRecheck: a node drained, or deauthorized for the project, -// after the world was built gets no lease: admission re-reads the node row. +// TestDispatchNodeRecheck: a node drained, revoked or deauthorized for the project +// after the world was built gets no lease: admission re-reads the node row (D-1086). func TestDispatchNodeRecheck(t *testing.T) { for _, c := range []struct { name string @@ -27,6 +27,10 @@ func TestDispatchNodeRecheck(t *testing.T) { n.State = "draining" return st.NodeSetState(ctx, n, "test", "drain") }, model.NodeDraining}, + {"revoked", func(ctx context.Context, st *store.Store, n store.NodeRecord) error { + _, err := st.NodeRevoke(ctx, n.ID, "cli:test", nil) + return err + }, model.NodeRevoked}, {"deauthorized", func(ctx context.Context, st *store.Store, n store.NodeRecord) error { var cap model.Capability if err := json.Unmarshal(n.Capability, &cap); err != nil { From 3a07388b8a1935c7fe79414f184c3b1a303c288f Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Mon, 19 Oct 2026 18:29:37 -0400 Subject: [PATCH 05/23] fix(plugins): nodes add checks for a deploy host before it contacts the host (D-1219) Add a pre-add hook point that runs on the destination alone, after ParseHostSpec and before the host-key scan. The deploy-host check registers there (identity from ssh -G and DNS only): a match without --allow-deploy-host is refused before any scan, probe or install, and an override is confirmed and audited once, there. Matches that need the host key or machine id are still caught at the add point. --- free/ci/cmd/nodes_override.go | 21 ++++++++- free/ci/cmd/nodes_override_test.go | 39 ++++++++++++++++ .../ci/internal/nodes/deployguard/override.go | 23 +++++++++- .../nodes/deployguard/override_test.go | 43 ++++++++++++++++++ free/ci/internal/nodes/lifecycle/add.go | 4 ++ free/ci/internal/nodes/lifecycle/hooks.go | 12 ++--- .../nodes/lifecycle/hooks_deployhost_test.go | 45 +++++++++++++++++++ 7 files changed, 178 insertions(+), 9 deletions(-) diff --git a/free/ci/cmd/nodes_override.go b/free/ci/cmd/nodes_override.go index de42cbddb..81f91fdb7 100644 --- a/free/ci/cmd/nodes_override.go +++ b/free/ci/cmd/nodes_override.go @@ -17,6 +17,7 @@ import ( "os" "os/user" "path/filepath" + "sync" "github.com/nself-org/plugins/free/ci/internal/nodes/deployguard" "github.com/nself-org/plugins/free/ci/internal/nodes/lifecycle" @@ -24,6 +25,10 @@ import ( const allowDeployHostFlag = "allow-deploy-host" +// preConfirmed holds the node ids whose override the pre-contact check confirmed and audited, so the add +// check does not ask or write a second audit row. +var preConfirmed sync.Map + // newGuard builds the coordinator-side guard; tests replace it with stub inventory and DNS seams. var newGuard = deployguard.New @@ -31,7 +36,7 @@ func init() { lifecycle.RegisterFlags("add", func(fs *flag.FlagSet) { fs.Bool(allowDeployHostFlag, false, "allow a node whose host is a deploy host (confirmed on a terminal, audited)") }) - for _, p := range []lifecycle.Point{lifecycle.PointAdd, lifecycle.PointPair, lifecycle.PointTest} { + for _, p := range []lifecycle.Point{lifecycle.PointPreAdd, lifecycle.PointAdd, lifecycle.PointPair, lifecycle.PointTest} { point := p lifecycle.RegisterAddHook(point, func(ctx context.Context, e lifecycle.HookEvent) error { return deployHostHook(ctx, point, e) }) } @@ -101,13 +106,25 @@ func deployHostHook(ctx context.Context, point lifecycle.Point, e lifecycle.Hook } q := deployguard.AddRequest{NodeID: e.NodeID, Dest: e.Dest, Capability: e.Capability, Actor: adminActor(), Allow: flagBool(e.Flags, allowDeployHostFlag), Confirm: deployguard.TerminalConfirm(os.Stdin, os.Stderr, isTerminal(os.Stdin))} - r, warns, err := g.CheckAdd(ctx, q) + check := g.CheckAdd + if point == lifecycle.PointPreAdd { + check = g.CheckPreAdd // before the host is contacted: a match is refused, or confirmed once with the flag + } else { + _, q.Confirmed = preConfirmed.LoadAndDelete(e.NodeID) + } + r, warns, err := check(ctx, q) for _, w := range warns { fmt.Fprintln(os.Stderr, "warning:", w) } if err != nil { return err } + if point == lifecycle.PointPreAdd { + if r.Matched { + preConfirmed.Store(e.NodeID, true) // CheckPreAdd refuses a match without the flag + } + return nil + } if r.Matched && e.Capability == nil { // No draft to mark: the stored node must carry the match, or the pairing is refused. if again, eerr := g.EvaluateNode(ctx, e.NodeID); eerr != nil || !again.Matched { diff --git a/free/ci/cmd/nodes_override_test.go b/free/ci/cmd/nodes_override_test.go index 45659e829..6b7a13d28 100644 --- a/free/ci/cmd/nodes_override_test.go +++ b/free/ci/cmd/nodes_override_test.go @@ -204,3 +204,42 @@ func TestProjectRootAndHelpers(t *testing.T) { t.Fatal(adminActor()) } } + +// D-1219: the pre-contact hook refuses a deploy host before `nodes add` scans or installs anything, and +// an override confirmed there is not asked for again at the add point. +func TestPreAddHookRefusesADeployHostBeforeContact(t *testing.T) { + stubNself(t) + ctx := context.Background() + for _, tc := range []struct { + name string + dest string + allow bool + reason string + }{ + {"deploy host", "ci@" + deployguard.FixtureHostV4, false, deployguard.ReasonMatched}, + {"deploy host, flag, no terminal", deployguard.FixtureHostV4, true, deployguard.ReasonNoTTY}, + {"unlisted host", deployguard.FixtureOther, false, ""}, + {"unlisted host, flag", deployguard.FixtureOther, true, ""}, + } { + t.Run(tc.name, func(t *testing.T) { + e := lifecycle.HookEvent{Point: lifecycle.PointPreAdd, NodeID: "pre-box", Dest: tc.dest, Flags: addFlagSet(tc.allow)} + err := deployHostHook(ctx, lifecycle.PointPreAdd, e) + if got := deployguard.ReasonOf(err); got != tc.reason || (tc.reason == "") != (err == nil) { + t.Fatalf("err %v reason %q, want %q", err, got, tc.reason) + } + if _, kept := preConfirmed.LoadAndDelete("pre-box"); kept { + t.Fatal("a refused or unmatched pre-add recorded a confirmation") + } + }) + } + // A confirmation recorded by the pre-contact check is consumed by the add check: no second prompt. + preConfirmed.Store("pre-box", true) + c := draft(t, "pre-box", deployguard.FixtureHostV4) + e := lifecycle.HookEvent{Point: lifecycle.PointAdd, NodeID: "pre-box", Dest: deployguard.FixtureHostV4, Capability: c, Flags: addFlagSet(true)} + if err := deployHostHook(ctx, lifecycle.PointAdd, e); err != nil || !c.DeployHost.Matched || !c.DeployHost.Override { + t.Fatalf("add after a pre-add confirmation: %v %+v", err, c.DeployHost) + } + if _, kept := preConfirmed.Load("pre-box"); kept { + t.Fatal("the confirmation was not consumed") + } +} diff --git a/free/ci/internal/nodes/deployguard/override.go b/free/ci/internal/nodes/deployguard/override.go index c7473d8ab..eca4e6108 100644 --- a/free/ci/internal/nodes/deployguard/override.go +++ b/free/ci/internal/nodes/deployguard/override.go @@ -6,7 +6,8 @@ package deployguard // Inputs: AddRequest (node id, destination, the draft capability, the flag, a confirmer). // Outputs: the Result; the draft's DeployHost set (matched, source, override); one audit row for a // confirmed override. -// Constraints: a match without the flag registers the node ineligible (deploy_host.matched true); the flag +// Constraints: a match on the destination alone is refused before the host is contacted (CheckPreAdd); a match +// found only after contact (fingerprint, machine id) registers the node ineligible (deploy_host.matched true); the flag // without a match changes nothing; the flag with a match and no confirmation (no terminal, or // anything but the node id typed back) fails with E663 and registers nothing; the audit row is // written before the node exists and names the source and kind, never a key. @@ -44,6 +45,22 @@ type AddRequest struct { Allow bool Confirm Confirmer Actor string + // Confirmed says an earlier CheckPreAdd for this node already confirmed and audited the override: CheckAdd + // then neither asks nor writes a second audit row. + Confirmed bool +} + +// CheckPreAdd is the check before the host is contacted (`nodes add`): the destination and node id alone, with +// identity from ssh -G and DNS. A match without Allow is refused here, so nothing is scanned, probed or installed +// on a deploy host; with Allow the override is confirmed and audited now and the caller passes Confirmed to the +// later CheckAdd. An unmatched destination is left to CheckAdd, which also sees the host key and machine id. +func (g *Guard) CheckPreAdd(ctx context.Context, q AddRequest) (Result, []string, error) { + q.Capability, q.Confirmed = nil, false + r, warn, err := g.CheckAdd(ctx, q) + if err == nil && r.Matched && !q.Allow { + err = newErr(ReasonMatched, "%s is a deploy host (%s); it was not contacted and not added (--allow-deploy-host adds it after a confirmation)", q.NodeID, r.Source) + } + return r, warn, err } // CheckAdd refreshes the set, matches the node and applies the override rules. The returned warnings are @@ -80,7 +97,9 @@ func (g *Guard) CheckAdd(ctx context.Context, q AddRequest) (Result, []string, e return r, warn, g.unverified(ctx) } dh := DeployHostFor(r, model.CapabilityDeployHost{}) - if q.Allow { + if q.Allow && q.Confirmed { + dh.Override = true + } else if q.Allow { if q.Confirm == nil { return r, warn, newErr(ReasonNoTTY, "--allow-deploy-host needs an interactive terminal to confirm") } diff --git a/free/ci/internal/nodes/deployguard/override_test.go b/free/ci/internal/nodes/deployguard/override_test.go index 8b2d2844d..52a00b9cd 100644 --- a/free/ci/internal/nodes/deployguard/override_test.go +++ b/free/ci/internal/nodes/deployguard/override_test.go @@ -148,3 +148,46 @@ func TestCheckAddTakesTheDestinationFromTheDraftWhenNoneIsGiven(t *testing.T) { t.Fatalf("%+v %v %+v", res, err, c.DeployHost) } } + +// D-1219: the check before contact refuses a match unless the override is confirmed; one confirmation and one +// audit row cover the pre-contact check and the later CheckAdd of the same node. +func TestCheckPreAddRefusesAMatchBeforeContact(t *testing.T) { + for _, tc := range []struct { + name string + dest string + allow bool + confirm Confirmer + reason string + audits int + }{ + {"match without the flag", FixtureHostV4, false, yes, ReasonMatched, 0}, + {"match with the flag, confirmed", FixtureHostV4, true, yes, "", 1}, + {"match with the flag, not confirmed", FixtureHostV4, true, func(string, string) (bool, error) { return false, nil }, ReasonNotConfirmed, 0}, + {"match with the flag, no terminal", FixtureHostV4, true, nil, ReasonNoTTY, 0}, + {"unlisted host", FixtureOther, false, yes, "", 0}, + } { + t.Run(tc.name, func(t *testing.T) { + r := newRig(t) + ctx := context.Background() + _ = r.g.Seen(ctx, r.rootA) + q := AddRequest{NodeID: "box", Dest: "ci@" + tc.dest, Allow: tc.allow, Confirm: tc.confirm} + res, _, err := r.g.CheckPreAdd(ctx, q) + if got := ReasonOf(err); got != tc.reason || (tc.reason == "") != (err == nil) { + t.Fatalf("err %v reason %q, want %q", err, got, tc.reason) + } + if n := r.auditCount("node.deploy_host.override"); n != tc.audits { + t.Fatalf("%d audit rows, want %d", n, tc.audits) + } + if err != nil || !res.Matched { + return + } + c := draftCapability(t, r, tc.dest) + q.Capability, q.Confirmed = c, true + asked := false + q.Confirm = func(string, string) (bool, error) { asked = true; return true, nil } + if _, _, err = r.g.CheckAdd(ctx, q); err != nil || asked || !c.DeployHost.Matched || !c.DeployHost.Override || r.auditCount("node.deploy_host.override") != 1 { + t.Fatalf("confirmed CheckAdd: err %v asked %v draft %+v audits %d", err, asked, c.DeployHost, r.auditCount("node.deploy_host.override")) + } + }) + } +} diff --git a/free/ci/internal/nodes/lifecycle/add.go b/free/ci/internal/nodes/lifecycle/add.go index e8fbd4d17..d5005bf91 100644 --- a/free/ci/internal/nodes/lifecycle/add.go +++ b/free/ci/internal/nodes/lifecycle/add.go @@ -87,6 +87,10 @@ func (d *Deps) Add(ctx context.Context, o AddOptions) (res AddResult, err error) } else if CodeOf(gerr) != CodeNotFound { return res, gerr } + // Before any contact: a hook that vets the destination alone (the deploy-host check, D-1219) runs here. + if err = RunHooks(ctx, PointPreAdd, HookEvent{NodeID: id, Project: d.Project, Flags: o.Flags, Dest: o.Dest}); err != nil { + return res, err + } installer, ok := ext.installer(orDefault(o.Installer, DefaultInstaller)) if !ok { return res, usage("unknown agent installer %q", o.Installer) diff --git a/free/ci/internal/nodes/lifecycle/hooks.go b/free/ci/internal/nodes/lifecycle/hooks.go index 090192afe..1f8bf1dec 100644 --- a/free/ci/internal/nodes/lifecycle/hooks.go +++ b/free/ci/internal/nodes/lifecycle/hooks.go @@ -21,11 +21,13 @@ import ( // Point is where an AddHook runs. type Point string -// Hook points: add (after the probe, before the node is registered), pair (P7-NODE-07's pairing), test. +// Hook points: pre-add (`nodes add`, after the destination is parsed and before the host is contacted: a Dest-only +// event, no Capability), add (after the probe, before the node is registered), pair (P7-NODE-07's pairing), test. const ( - PointAdd Point = "add" - PointPair Point = "pair" - PointTest Point = "test" + PointPreAdd Point = "pre-add" + PointAdd Point = "add" + PointPair Point = "pair" + PointTest Point = "test" ) // HookEvent is what a hook sees. Flags is the parsed flag set of the command, including the flags @@ -103,7 +105,7 @@ func newExtensions() *extensions { // RegisterAddHook runs fn at point. func RegisterAddHook(point Point, fn AddHook) { - if fn == nil || (point != PointAdd && point != PointPair && point != PointTest) { + if fn == nil || (point != PointPreAdd && point != PointAdd && point != PointPair && point != PointTest) { panic("nself-ci: invalid lifecycle hook " + string(point)) } ext.mu.Lock() diff --git a/free/ci/internal/nodes/lifecycle/hooks_deployhost_test.go b/free/ci/internal/nodes/lifecycle/hooks_deployhost_test.go index 39f856c82..71cb10a87 100644 --- a/free/ci/internal/nodes/lifecycle/hooks_deployhost_test.go +++ b/free/ci/internal/nodes/lifecycle/hooks_deployhost_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/nself-org/cli/sdk/go/v2/remote" "github.com/nself-org/plugins/free/ci/internal/model" ) @@ -55,3 +56,47 @@ func TestTestReportsTheNodeAsAHookLeftIt(t *testing.T) { t.Fatal("test hook error swallowed") } } + +// D-1219: the pre-add hook runs on the destination alone and a refusal there happens before any contact: +// no host-key scan, no ssh command, no copy, no pin, no node. +func TestPreAddHookRunsBeforeAnyContact(t *testing.T) { + for _, tc := range []struct { + name string + hook error + }{{"refused", errBoom}, {"passes", nil}} { + t.Run(tc.name, func(t *testing.T) { + t.Cleanup(resetExtensions) + k := newKit(t) + scans := 0 + scan := k.d.Scan + k.d.Scan = func(ctx context.Context, tg remote.Target, id string) ([]remote.HostKey, error) { + scans++ + return scan(ctx, tg, id) + } + var ev HookEvent + RegisterAddHook(PointPreAdd, func(_ context.Context, e HookEvent) error { + ev = e + if scans != 0 || len(k.runs) != 0 { + t.Errorf("pre-add ran after contact: %d scans, %d runs", scans, len(k.runs)) + } + return tc.hook + }) + _, err := k.d.Add(context.Background(), AddOptions{Dest: "ci@pre1", HostKey: fpA, Source: LocalFile{Path: elfFixture(t, elf.EM_X86_64)}, Trust: DefaultTrust()}) + if ev.Point != PointPreAdd || ev.Dest != "ci@pre1" || ev.NodeID == "" || ev.Capability != nil { + t.Fatalf("event %+v", ev) + } + if tc.hook == nil { + if err != nil || scans == 0 { + t.Fatalf("err %v scans %d", err, scans) + } + return + } + if err == nil || scans != 0 || len(k.runs) != 0 || len(k.copies) != 0 { + t.Fatalf("a refused pre-add still made contact: err %v scans %d runs %v copies %v", err, scans, k.runs, k.copies) + } + if p, _ := k.d.pins().Lookup(pinAlias("pre1")); len(p) != 0 { + t.Fatal("a refused pre-add pinned a key") + } + }) + } +} From 3fcf9a27531f8700a7269d71e5693dfb7f639f02 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Mon, 19 Oct 2026 18:33:53 -0400 Subject: [PATCH 06/23] fix(plugins): pre-add override confirmation is bound to the matched source (D-1219) The confirmation taken before contact covers only the match it was given for; a different match found after contact is confirmed again. The installer name is validated before the pre-add hook, so no override is confirmed for an add that cannot proceed. --- free/ci/cmd/nodes_override.go | 10 ++++++---- free/ci/cmd/nodes_override_test.go | 9 ++++++++- free/ci/internal/nodes/deployguard/override.go | 14 +++++++++----- .../ci/internal/nodes/deployguard/override_test.go | 2 +- free/ci/internal/nodes/lifecycle/add.go | 8 ++++---- .../nodes/lifecycle/hooks_deployhost_test.go | 13 +++++++++++++ 6 files changed, 41 insertions(+), 15 deletions(-) diff --git a/free/ci/cmd/nodes_override.go b/free/ci/cmd/nodes_override.go index 81f91fdb7..88fc28b57 100644 --- a/free/ci/cmd/nodes_override.go +++ b/free/ci/cmd/nodes_override.go @@ -25,8 +25,8 @@ import ( const allowDeployHostFlag = "allow-deploy-host" -// preConfirmed holds the node ids whose override the pre-contact check confirmed and audited, so the add -// check does not ask or write a second audit row. +// preConfirmed maps a node id to the ConfirmKey of the override the pre-contact check confirmed and audited, so +// the add check does not ask or write a second audit row for the same match. var preConfirmed sync.Map // newGuard builds the coordinator-side guard; tests replace it with stub inventory and DNS seams. @@ -110,7 +110,9 @@ func deployHostHook(ctx context.Context, point lifecycle.Point, e lifecycle.Hook if point == lifecycle.PointPreAdd { check = g.CheckPreAdd // before the host is contacted: a match is refused, or confirmed once with the flag } else { - _, q.Confirmed = preConfirmed.LoadAndDelete(e.NodeID) + if v, ok := preConfirmed.LoadAndDelete(e.NodeID); ok { + q.Confirmed, _ = v.(string) + } } r, warns, err := check(ctx, q) for _, w := range warns { @@ -121,7 +123,7 @@ func deployHostHook(ctx context.Context, point lifecycle.Point, e lifecycle.Hook } if point == lifecycle.PointPreAdd { if r.Matched { - preConfirmed.Store(e.NodeID, true) // CheckPreAdd refuses a match without the flag + preConfirmed.Store(e.NodeID, r.ConfirmKey()) // CheckPreAdd refuses a match without the flag } return nil } diff --git a/free/ci/cmd/nodes_override_test.go b/free/ci/cmd/nodes_override_test.go index 6b7a13d28..3ee7726c9 100644 --- a/free/ci/cmd/nodes_override_test.go +++ b/free/ci/cmd/nodes_override_test.go @@ -233,7 +233,14 @@ func TestPreAddHookRefusesADeployHostBeforeContact(t *testing.T) { }) } // A confirmation recorded by the pre-contact check is consumed by the add check: no second prompt. - preConfirmed.Store("pre-box", true) + // A different match found after contact (here: another kind) is not covered by that confirmation. + preConfirmed.Store("pre-box", deployguard.Result{Source: deployguard.SourceInventory, Kind: deployguard.KindFingerprint}.ConfirmKey()) + cx := draft(t, "pre-box", deployguard.FixtureHostV4) + ex := lifecycle.HookEvent{Point: lifecycle.PointAdd, NodeID: "pre-box", Dest: deployguard.FixtureHostV4, Capability: cx, Flags: addFlagSet(true)} + if err := deployHostHook(ctx, lifecycle.PointAdd, ex); deployguard.ReasonOf(err) != deployguard.ReasonNoTTY || cx.DeployHost.Override { + t.Fatalf("a confirmation of another match was reused: %v %+v", err, cx.DeployHost) + } + preConfirmed.Store("pre-box", deployguard.Result{Source: deployguard.SourceInventory, Kind: deployguard.KindAddress}.ConfirmKey()) c := draft(t, "pre-box", deployguard.FixtureHostV4) e := lifecycle.HookEvent{Point: lifecycle.PointAdd, NodeID: "pre-box", Dest: deployguard.FixtureHostV4, Capability: c, Flags: addFlagSet(true)} if err := deployHostHook(ctx, lifecycle.PointAdd, e); err != nil || !c.DeployHost.Matched || !c.DeployHost.Override { diff --git a/free/ci/internal/nodes/deployguard/override.go b/free/ci/internal/nodes/deployguard/override.go index eca4e6108..d0e27b5cc 100644 --- a/free/ci/internal/nodes/deployguard/override.go +++ b/free/ci/internal/nodes/deployguard/override.go @@ -45,17 +45,21 @@ type AddRequest struct { Allow bool Confirm Confirmer Actor string - // Confirmed says an earlier CheckPreAdd for this node already confirmed and audited the override: CheckAdd - // then neither asks nor writes a second audit row. - Confirmed bool + // Confirmed is the ConfirmKey of the match an earlier CheckPreAdd confirmed and audited for this node: when + // CheckAdd finds the same match it neither asks nor writes a second audit row; a different match (found + // once the host key is known) is confirmed afresh. + Confirmed string } +// ConfirmKey names a match for Confirmed. +func (r Result) ConfirmKey() string { return r.Source + "/" + r.Kind } + // CheckPreAdd is the check before the host is contacted (`nodes add`): the destination and node id alone, with // identity from ssh -G and DNS. A match without Allow is refused here, so nothing is scanned, probed or installed // on a deploy host; with Allow the override is confirmed and audited now and the caller passes Confirmed to the // later CheckAdd. An unmatched destination is left to CheckAdd, which also sees the host key and machine id. func (g *Guard) CheckPreAdd(ctx context.Context, q AddRequest) (Result, []string, error) { - q.Capability, q.Confirmed = nil, false + q.Capability, q.Confirmed = nil, "" r, warn, err := g.CheckAdd(ctx, q) if err == nil && r.Matched && !q.Allow { err = newErr(ReasonMatched, "%s is a deploy host (%s); it was not contacted and not added (--allow-deploy-host adds it after a confirmation)", q.NodeID, r.Source) @@ -97,7 +101,7 @@ func (g *Guard) CheckAdd(ctx context.Context, q AddRequest) (Result, []string, e return r, warn, g.unverified(ctx) } dh := DeployHostFor(r, model.CapabilityDeployHost{}) - if q.Allow && q.Confirmed { + if q.Allow && q.Confirmed == r.ConfirmKey() { dh.Override = true } else if q.Allow { if q.Confirm == nil { diff --git a/free/ci/internal/nodes/deployguard/override_test.go b/free/ci/internal/nodes/deployguard/override_test.go index 52a00b9cd..b3cdc17bc 100644 --- a/free/ci/internal/nodes/deployguard/override_test.go +++ b/free/ci/internal/nodes/deployguard/override_test.go @@ -182,7 +182,7 @@ func TestCheckPreAddRefusesAMatchBeforeContact(t *testing.T) { return } c := draftCapability(t, r, tc.dest) - q.Capability, q.Confirmed = c, true + q.Capability, q.Confirmed = c, res.ConfirmKey() asked := false q.Confirm = func(string, string) (bool, error) { asked = true; return true, nil } if _, _, err = r.g.CheckAdd(ctx, q); err != nil || asked || !c.DeployHost.Matched || !c.DeployHost.Override || r.auditCount("node.deploy_host.override") != 1 { diff --git a/free/ci/internal/nodes/lifecycle/add.go b/free/ci/internal/nodes/lifecycle/add.go index d5005bf91..c86ef8c77 100644 --- a/free/ci/internal/nodes/lifecycle/add.go +++ b/free/ci/internal/nodes/lifecycle/add.go @@ -87,14 +87,14 @@ func (d *Deps) Add(ctx context.Context, o AddOptions) (res AddResult, err error) } else if CodeOf(gerr) != CodeNotFound { return res, gerr } - // Before any contact: a hook that vets the destination alone (the deploy-host check, D-1219) runs here. - if err = RunHooks(ctx, PointPreAdd, HookEvent{NodeID: id, Project: d.Project, Flags: o.Flags, Dest: o.Dest}); err != nil { - return res, err - } installer, ok := ext.installer(orDefault(o.Installer, DefaultInstaller)) if !ok { return res, usage("unknown agent installer %q", o.Installer) } + // Before any contact: a hook that vets the destination alone (the deploy-host check, D-1219) runs here. + if err = RunHooks(ctx, PointPreAdd, HookEvent{NodeID: id, Project: d.Project, Flags: o.Flags, Dest: o.Dest}); err != nil { + return res, err + } keys, err := d.Scan(ctx, remote.Target{Dest: spec.Dest()}, id) if err != nil { return res, transportErr("scan", "cannot capture the host key of "+spec.Dest(), err) diff --git a/free/ci/internal/nodes/lifecycle/hooks_deployhost_test.go b/free/ci/internal/nodes/lifecycle/hooks_deployhost_test.go index 71cb10a87..45903cd3a 100644 --- a/free/ci/internal/nodes/lifecycle/hooks_deployhost_test.go +++ b/free/ci/internal/nodes/lifecycle/hooks_deployhost_test.go @@ -100,3 +100,16 @@ func TestPreAddHookRunsBeforeAnyContact(t *testing.T) { }) } } + +// An unknown installer fails before the pre-add hook: the operator is not asked to confirm an override for an +// add that cannot proceed. +func TestPreAddHookDoesNotRunForAnUnknownInstaller(t *testing.T) { + t.Cleanup(resetExtensions) + k := newKit(t) + ran := false + RegisterAddHook(PointPreAdd, func(context.Context, HookEvent) error { ran = true; return nil }) + _, err := k.d.Add(context.Background(), AddOptions{Dest: "ci@pre2", HostKey: fpA, Installer: "typo", Trust: DefaultTrust()}) + if err == nil || ran { + t.Fatalf("err %v, pre-add ran %v", err, ran) + } +} From 2dd6df568b37ed8f18dce851e3e17a92b7e5bc59 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 21:07:00 -0400 Subject: [PATCH 07/23] 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 ac9437b21..b2c879fbe 100644 --- a/free/ci/architecture/layers.yaml +++ b/free/ci/architecture/layers.yaml @@ -173,6 +173,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 c5d46fa5a..5409cf145 100644 --- a/free/ci/internal/nodes/provision/single_exec_test.go +++ b/free/ci/internal/nodes/provision/single_exec_test.go @@ -63,6 +63,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 d8a24dcbf8880249fc00b74053700ebcbd1554f6 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 21:21:39 -0400 Subject: [PATCH 08/23] 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 ffd122ed7d827f2ab8f8c0bd729eecc29001b7eb Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 21:28:20 -0400 Subject: [PATCH 09/23] 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 5d01143881033df85491be68f4062675dcb73b26 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 21:41:12 -0400 Subject: [PATCH 10/23] 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 c9288061d3ca49d0bf5beadd09b2181bd2605e16 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 21:50:52 -0400 Subject: [PATCH 11/23] 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 b157ba8198945eb9008ec0cb82b6fc9dc591c0ef Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 22:05:03 -0400 Subject: [PATCH 12/23] 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 2386b7346d260f4c2893ad3412cbac7799dd6976 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 22:18:43 -0400 Subject: [PATCH 13/23] 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 82b612c35c862a012a05323e72affeff584e336b Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 22:40:12 -0400 Subject: [PATCH 14/23] 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 00e5721517424e18f8eba3ccc8abb5ab6ae1eac7 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 22:57:26 -0400 Subject: [PATCH 15/23] 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 ca7a55524e8009fb6d6003df925de0e65ad8d5cb Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Sun, 18 Oct 2026 23:39:55 -0400 Subject: [PATCH 16/23] 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 bf524bf3035fa5f3a4c7b7dbdbb3afce93f44c36 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Mon, 19 Oct 2026 00:06:12 -0400 Subject: [PATCH 17/23] 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 3a6916a873bf08c84579c7b14275f7069db065fc Mon Sep 17 00:00:00 2001 From: admin Date: Tue, 20 Oct 2026 08:11:34 -0400 Subject: [PATCH 18/23] 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 +++++++++++++++++++ 4 files changed, 46 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 9cf81bdff..755a85b37 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 83d17f934..c4205c4da 100644 --- a/free/ci/internal/exec/sweep.go +++ b/free/ci/internal/exec/sweep.go @@ -157,6 +157,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) + } + }) + } +} From df7a8e1fbd3ab64349060aa464baa1ff22f04b3b Mon Sep 17 00:00:00 2001 From: admin Date: Tue, 20 Oct 2026 08:15:33 -0400 Subject: [PATCH 19/23] 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/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 ++ 31 files changed, 76 insertions(+), 70 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 39e660acd..28246002e 100644 --- a/free/ci/cmd/agent/join.go +++ b/free/ci/cmd/agent/join.go @@ -86,7 +86,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 12a8e2e2b..d681c5453 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 48136aaa9..2a708c08d 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 30f4afceb..39df60a12 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) @@ -554,7 +554,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() @@ -562,7 +562,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")) } @@ -578,7 +578,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 @@ -590,7 +590,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 0a18f8cf1..2a19a3365 100644 --- a/free/ci/cmd/nodes_remote.go +++ b/free/ci/cmd/nodes_remote.go @@ -112,7 +112,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 a4300cece..773794394 100644 --- a/free/ci/cmd/runs_reproduce.go +++ b/free/ci/cmd/runs_reproduce.go @@ -140,10 +140,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/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 3e7e2dd08..fabb09be4 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 0cd83a8c9..d4635815e 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 8092e8a7a..94d5cb045 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 a0bb08f5e..cb0286c1a 100644 --- a/free/ci/internal/run/hygiene_hook.go +++ b/free/ci/internal/run/hygiene_hook.go @@ -145,7 +145,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 e703cb996..a83d2d318 100644 --- a/free/ci/internal/run/jobrun.go +++ b/free/ci/internal/run/jobrun.go @@ -201,13 +201,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 b20e49d4c..52e493325 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 eed4182db..daf4dc0b3 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 d2dd8a628eeb45a6753462b3dd69fb8952327727 Mon Sep 17 00:00:00 2001 From: admin Date: Tue, 20 Oct 2026 08:19:19 -0400 Subject: [PATCH 20/23] fix(ci): audit operator trust inside disposable hosted workflow VMs --- .../ci/internal/dispatch/backpressure_test.go | 2 +- free/ci/internal/protect/template_test.go | 39 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) 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 2a9b7206a603f2066853ffd5798095fd624045b1 Mon Sep 17 00:00:00 2001 From: admin Date: Tue, 20 Oct 2026 08:23:12 -0400 Subject: [PATCH 21/23] 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 | 4 +-- free/ci/cmd/approve_pending.go | 4 +-- 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/render.go | 18 ++++++------ 34 files changed, 171 insertions(+), 167 deletions(-) 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 e8d4e00d5..3fa60255f 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 28246002e..94f5cb3e8 100644 --- a/free/ci/cmd/agent/join.go +++ b/free/ci/cmd/agent/join.go @@ -54,11 +54,11 @@ 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] [--advertise] ") + _, _ = fmt.Fprintln(sio.Stderr, "usage: nself-ci-agent join [--expect-fingerprint SHA256:] [--name N] [--advertise] ") 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 } if err := RunPostStart(ctx, "join"); err != nil { @@ -67,12 +67,12 @@ func runJoin(ctx context.Context, args []string, sio IO) int { } 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() @@ -100,23 +100,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 3991271db..d9a25aea8 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 2a708c08d..6cd882296 100644 --- a/free/ci/cmd/agent/uninstall.go +++ b/free/ci/cmd/agent/uninstall.go @@ -92,17 +92,17 @@ 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 } fmt.Fprintf(sio.Stderr, "uninstall layout bundled with nself-ci-agent %s\n", Version) 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 { @@ -110,13 +110,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 80b364d68..f0cb78279 100644 --- a/free/ci/cmd/agent/upgrade.go +++ b/free/ci/cmd/agent/upgrade.go @@ -48,7 +48,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) @@ -56,7 +56,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) @@ -64,7 +64,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() @@ -72,12 +72,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{ @@ -87,13 +87,13 @@ func runUpgrade(ctx context.Context, args []string, sio IO) int { RunVersion: upgradeRunVersion, }) 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 1f2aed596..ba69d0f52 100644 --- a/free/ci/cmd/approve.go +++ b/free/ci/cmd/approve.go @@ -121,10 +121,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 } diff --git a/free/ci/cmd/approve_pending.go b/free/ci/cmd/approve_pending.go index 4418142ee..aff24f345 100644 --- a/free/ci/cmd/approve_pending.go +++ b/free/ci/cmd/approve_pending.go @@ -51,10 +51,10 @@ func runPending(args []string, env approveEnv) int { 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)) + _, _ = 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") + _, _ = fmt.Fprintln(env.Out, "no run waits for approval") } return 0 } diff --git a/free/ci/cmd/evidence.go b/free/ci/cmd/evidence.go index 8ee7e2394..c9ae697ae 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 41e522c99..9b27fd647 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 1614c737f..f349039a9 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 } } @@ -117,26 +117,26 @@ func runNodesPair(args []string, stdin io.Reader, stdout, stderr io.Writer) int AfterIssue: func(ctx context.Context, nodeID string) error { return lifecycle.RunHooks(ctx, lifecycle.PointPair, lifecycle.HookEvent{Point: lifecycle.PointPair, NodeID: nodeID, Project: d.Project, Flags: fs}) }, - Alert: func(a pairing.Alert) { fmt.Fprintln(stderr, pairingAlertLine(a)) }, + Alert: func(a pairing.Alert) { _, _ = fmt.Fprintln(stderr, pairingAlertLine(a)) }, }) 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 } @@ -250,7 +250,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 } @@ -269,6 +269,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 9f5a34710..e97efb7ec 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 773794394..f8b4815f9 100644 --- a/free/ci/cmd/runs_reproduce.go +++ b/free/ci/cmd/runs_reproduce.go @@ -138,19 +138,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 dee6e9d66..83add007b 100644 --- a/free/ci/cmd/serve_admin.go +++ b/free/ci/cmd/serve_admin.go @@ -163,7 +163,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/render.go b/free/ci/internal/run/render.go index b718b9825..062383899 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)) } } } @@ -205,7 +205,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 { @@ -214,11 +214,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 fd579896713f61ab85ec1c47e2f3d63f2c3434f4 Mon Sep 17 00:00:00 2001 From: admin Date: Tue, 20 Oct 2026 08:27:43 -0400 Subject: [PATCH 22/23] 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 3fa60255f..ab9ec6321 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 } From efd0c1ee36347d9baa2f1fd06bdbb09decc14982 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Thu, 29 Oct 2026 12:09:58 -0400 Subject: [PATCH 23/23] fix(ci): align rebased bench executor and hosted trust test with main's isolation and same-repo trust --- free/ci/bench/measure.go | 2 +- free/ci/cmd/agent/prodguard_test.go | 6 ------ free/ci/internal/protect/template_test.go | 12 ++++++++---- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/free/ci/bench/measure.go b/free/ci/bench/measure.go index 3b0090dc5..de9ccc701 100644 --- a/free/ci/bench/measure.go +++ b/free/ci/bench/measure.go @@ -89,7 +89,7 @@ func (p *Inproc) StoreOpen() error { func (p *Inproc) ExecOverhead(ctx context.Context) (Timing, error) { argv := []string{"true"} n := p.seq.Add(1) - ex := &ciexec.Executor{Env: ciexec.AllowlistEnv{}} + ex := &ciexec.Executor{Isolation: ciexec.BaselineIsolator{}, 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 { diff --git a/free/ci/cmd/agent/prodguard_test.go b/free/ci/cmd/agent/prodguard_test.go index 4017d2fa0..d0ae18cde 100644 --- a/free/ci/cmd/agent/prodguard_test.go +++ b/free/ci/cmd/agent/prodguard_test.go @@ -69,12 +69,6 @@ func TestServeStdioExitsNonZeroWithE663OnAProdStackMarker(t *testing.T) { } t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) 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/protect/template_test.go b/free/ci/internal/protect/template_test.go index fbda575aa..7005b3b99 100644 --- a/free/ci/internal/protect/template_test.go +++ b/free/ci/internal/protect/template_test.go @@ -120,7 +120,8 @@ 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. +// Its override is scoped to disposable hosted runners, without secrets or writes, +// and only a head in this very repository gets it: a fork head stays untrusted. func TestHostedSourceWorkflowTrust(t *testing.T) { for _, tc := range []struct { file, job, command, runner string @@ -138,6 +139,9 @@ func TestHostedSourceWorkflowTrust(t *testing.T) { 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") } + if !strings.Contains(raw, "NSELF_CI_TRUST: ${{ github.event.pull_request.head.repo.full_name == github.repository && 'owner' || '' }}") { + t.Fatal("the audited override must be set only for a head in this repository") + } calls := 0 for _, step := range job.Steps { for _, line := range strings.Split(step.Run, "\n") { @@ -145,9 +149,9 @@ func TestHostedSourceWorkflowTrust(t *testing.T) { continue } calls++ - if !strings.Contains(line, " --trust owner ") { - t.Fatalf("missing audited operator override: %s", line) - } + } + if strings.Contains(step.Run, tc.command+" ") && !strings.Contains(step.Run, `--trust "$NSELF_CI_TRUST"`) { + t.Fatalf("missing audited operator override: %s", step.Run) } } if calls != tc.calls {