fix(ci): prune, tool-signature and flaky-annotation fixes (D-1189, D-1192, D-1193)

free/ci/internal/diagnose/classify_test.go

@@ -43,6 +43,8 @@ func TestClassifyClasses(t *testing.T) {
 		{"dns", Input{ExecClass: "code", ExitCode: 1, Tail: "curl: (6) Could not resolve host: proxy.golang.org\n"}, "infra", SigDNS, true},
 		{"docker down", Input{ExecClass: "infra", ExitCode: -1, Err: "Cannot connect to the Docker daemon at unix:///var/run/docker.sock"}, "infra", SigDocker, true},
 		{"missing tool by output", Input{ExecClass: "code", ExitCode: 127, Tail: "sh: 1: golangci-lint: not found\n"}, "infra", SigTool, true},
+		{"missing tool by bash prefix", Input{ExecClass: "code", ExitCode: 127, Tail: "/bin/sh: line 3: golangci-lint: not found\n"}, "infra", SigTool, true},
+		{"an application message ending in not found is code", Input{ExecClass: "code", ExitCode: 1, Tail: "customer: not found\n"}, "code", SigCode, false},
 		{"missing tool by exec error", Input{ExecClass: "infra", Err: `exec: "pnpm": executable file not found in $PATH`}, "infra", SigTool, true},
 		{"a program choosing exit 137 is code", Input{ExecClass: "code", ExitCode: 137, Tail: "assertion failed\n"}, "code", SigCode, false},
 		{"a program choosing exit 127 is code", Input{ExecClass: "code", ExitCode: 127, Tail: "assertion failed\n"}, "code", SigCode, false},

free/ci/internal/diagnose/signatures.go

@@ -81,7 +81,7 @@ var signatures = []signature{
 	{
 		id:          SigTool,
 		errText:     re(`(?i)executable file not found|fork/exec .*: no such file or directory|command not found`),
-		output:      re(`(?i)command not found|executable file not found|: not found$`),
+		output:      re(`(?im)command not found|executable file not found|^\S*sh: (line \d+: |\d+: )?\S+: not found$`),
 		remediation: "A tool the job needs is not installed on the runner. Install it, or declare it under `requires.tools` so the job only lands where it exists.",
 	},
 }

free/ci/internal/flaky/annotate.go

@@ -4,7 +4,7 @@ package flaky
 // (P7-CI-40): the same check passed in one run and failed in another on
 // identical inputs, counting this run too.
 // Inputs:  the store and the finished run's evidence (input digests set).
-// Outputs: a Flaky note per check id; the caller attaches it to the check.
+// Outputs: a Flaky note per check (model.FlakyKey: job and check id); the caller attaches it to the check.
 // Constraints: read-only; a marker never changes a result; at most
 // AnnotateRuns recent runs are read; a store error yields no marker (the
 // listing in `ci runs --flaky` is the source of truth, evidence only echoes it).
@@ -19,7 +19,7 @@ import (
 // AnnotateRuns is how many recent runs a finishing run compares itself with.
 const AnnotateRuns = 50
 
-// Annotate returns, by check id, the history flake notes for ev.
+// Annotate returns, by model.FlakyKey (job and check id), the history flake notes for ev.
 func Annotate(ctx context.Context, st *store.Store, ev model.Evidence) map[string]*model.Flaky {
 	out := map[string]*model.Flaky{}
 	if st == nil || ev.Pipeline.Kind == "adhoc" {
@@ -29,14 +29,20 @@ func Annotate(ctx context.Context, st *store.Store, ev model.Evidence) map[strin
 	if err != nil {
 		return out
 	}
+	return annotateDocs(ev, past)
+}
+
+// annotateDocs is Annotate over already-read history (newest first).
+func annotateDocs(ev model.Evidence, past []Doc) map[string]*model.Flaky {
+	out := map[string]*model.Flaky{}
 	docs := append([]Doc{{RunID: ev.RunID, Evidence: ev}}, past...)
 	for _, f := range Detect(docs) {
 		if f.Source != SourceHistory || f.Revision != ev.Revision {
 			continue
 		}
 		for _, r := range f.Runs {
 			if r.Run == ev.RunID {
-				out[f.Check] = &model.Flaky{Source: SourceHistory, Passes: f.Passes, Fails: f.Fails}
+				out[model.FlakyKey(f.Job, f.Check)] = &model.Flaky{Source: SourceHistory, Passes: f.Passes, Fails: f.Fails}
 			}
 		}
 	}

free/ci/internal/flaky/flaky_test.go

@@ -205,3 +205,24 @@ func TestDirtyRunKeepsItsConfirmationEntry(t *testing.T) {
 		t.Fatalf("got %+v", got)
 	}
 }
+
+// Two jobs may share a check id; a flake of one job must not mark the other.
+func TestAnnotateKeysByJobAndCheck(t *testing.T) {
+	doc := func(runID string, a, b model.CheckResult) Doc {
+		d := evidenceOf("r1", "", runID, a, b)
+		d.Evidence.RunID = runID
+		d.Evidence.Checks[0].ID, d.Evidence.Checks[1].ID = "unit", "unit" // same check id under jobs "unit" and "lint"
+		return d
+	}
+	cur := doc("cur", "pass", "pass")
+	got := annotateDocs(cur.Evidence, []Doc{doc("old", "fail", "pass")})
+	if n := got[model.FlakyKey("unit", "unit")]; n == nil || n.Passes != 1 || n.Fails != 1 {
+		t.Errorf("the flaky job is not marked: %+v", got)
+	}
+	if n := got[model.FlakyKey("lint", "unit")]; n != nil {
+		t.Errorf("the stable job with the same check id was marked flaky: %+v", n)
+	}
+	if len(got) != 1 {
+		t.Errorf("notes %+v, want only the flaky job's", got)
+	}
+}

free/ci/internal/history/prune.go

@@ -15,6 +15,8 @@ package history
 
 import (
 	"context"
+	"fmt"
+	"os"
 	"time"
 
 	"github.com/nself-org/plugins/free/ci/internal/logs"
@@ -159,7 +161,12 @@ func init() {
 			return
 		}
 		home := s.StateDir()
-		pol, _ := LoadPolicy(home)
+		pol, err := LoadPolicy(home)
+		if err != nil {
+			// an unusable retention file must not turn into default limits that delete more than the user allowed
+			fmt.Fprintf(os.Stderr, "note: %s is unusable (%v); skipping the automatic prune\n", ConfigPath(home), err)
+			return
+		}
 		_, _ = Prune(ctx, s.Store, PruneParams{Home: home, Policy: pol, Keep: []string{s.RunID}, MaxRuns: runStartPruneMax})
 	}
 }

free/ci/internal/history/prune_test.go

@@ -306,3 +306,30 @@ func TestPruneResultListsAreNeverNullAndRunStartPruneIsBounded(t *testing.T) {
 		t.Fatalf("rows left: %v", f.ids())
 	}
 }
+
+// A retention.json that cannot be parsed must not make the automatic prune run
+// on the default limits: the old runs stay until the file is fixed.
+func TestRunStartSkipsPruneWhenRetentionFileIsBad(t *testing.T) {
+	f := newFixture(t)
+	f.add("p1", "passed", 90*day, 20) // far beyond the default 30-day age
+	f.add("p2", "passed", 80*day, 20)
+	if err := os.WriteFile(ConfigPath(f.home), []byte(`{nope`), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	_ = f.st.Close()
+	res := runOnce(t, f.home, "echo hi")
+	st, err := store.Open(filepath.Join(f.home, "state.db"), store.Options{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() { _ = st.Close() }()
+	ages, _ := st.PipelineAges(context.Background())
+	var ids []string
+	for _, a := range ages {
+		ids = append(ids, a.ID)
+	}
+	sort.Strings(ids)
+	want := []string{"p1", "p2", res.Summary.RunID}
+	sort.Strings(want)
+	same(t, "rows after the run", ids, want)
+}

free/ci/internal/model/evidence.go

@@ -141,6 +141,10 @@ type Flaky struct {
 	Fails  int    `json:"fails"`
 }
 
+// FlakyKey names a check inside one run for the flaky-by-history notes: the
+// same check id can appear under several jobs, so the job is part of the key.
+func FlakyKey(jobID, checkID string) string { return jobID + "\x00" + checkID }
+
 // Artifact is a committed output digest.
 type Artifact struct {
 	Path      string `json:"path"`

free/ci/internal/run/flaky_hook.go

@@ -3,7 +3,7 @@ package run
 // Purpose: the seam that lets a finished run's evidence carry flaky-by-history
 // debt (P7-CI-40) without run importing the history reader.
 // Inputs:  the store and the finished run's evidence.
-// Outputs: a Flaky note per check id, attached to the check.
+// Outputs: a Flaky note per check (model.FlakyKey), attached to the check.
 // Constraints: registered at init (cmd/runs_flaky.go); a nil hook, a panic or
 // an empty answer leaves the evidence as it was; a marker never changes a
 // result, a verdict or an exit code.
@@ -31,7 +31,7 @@ func markHistoryFlaky(ctx context.Context, st *store.Store, ev *model.Evidence)
 		return
 	}
 	for i := range ev.Checks {
-		if n := notes[ev.Checks[i].ID]; n != nil && ev.Checks[i].Flaky == nil {
+		if n := notes[model.FlakyKey(ev.Checks[i].JobID, ev.Checks[i].ID)]; n != nil && ev.Checks[i].Flaky == nil {
 			ev.Checks[i].Flaky = n
 		}
 	}

free/ci/internal/run/flaky_hook_test.go

@@ -0,0 +1,23 @@
+package run
+
+import (
+	"context"
+	"testing"
+
+	"github.com/nself-org/plugins/free/ci/internal/model"
+	"github.com/nself-org/plugins/free/ci/internal/store"
+)
+
+// A note for one job's check must not land on a same-named check of another job.
+func TestMarkHistoryFlakyMatchesJobAndCheck(t *testing.T) {
+	defer func(h FlakyHistoryHook) { FlakyHistory = h }(FlakyHistory)
+	note := &model.Flaky{Source: "history", Passes: 1, Fails: 1}
+	FlakyHistory = func(context.Context, *store.Store, model.Evidence) map[string]*model.Flaky {
+		return map[string]*model.Flaky{model.FlakyKey("a", "unit"): note}
+	}
+	ev := model.Evidence{Checks: []model.Check{{ID: "unit", JobID: "a"}, {ID: "unit", JobID: "b"}}}
+	markHistoryFlaky(context.Background(), nil, &ev)
+	if ev.Checks[0].Flaky != note || ev.Checks[1].Flaky != nil {
+		t.Fatalf("flaky marks: a=%+v b=%+v", ev.Checks[0].Flaky, ev.Checks[1].Flaky)
+	}
+}