From cd431188ed9b288f4dc101860d66152819a29e93 Mon Sep 17 00:00:00 2001 From: admin Date: Sun, 25 Oct 2026 11:02:28 -0400 Subject: [PATCH 1/3] fix(ci): skip the automatic prune when retention.json is unusable (D-1189) --- free/ci/internal/history/prune.go | 9 ++++++++- free/ci/internal/history/prune_test.go | 27 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/free/ci/internal/history/prune.go b/free/ci/internal/history/prune.go index 2ed2277e9..58e048924 100644 --- a/free/ci/internal/history/prune.go +++ b/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}) } } diff --git a/free/ci/internal/history/prune_test.go b/free/ci/internal/history/prune_test.go index 6a7698b93..bc96bfb3a 100644 --- a/free/ci/internal/history/prune_test.go +++ b/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) +} From 0396386fbad365998aad56ca1dce2fca1efbae11 Mon Sep 17 00:00:00 2001 From: admin Date: Sun, 25 Oct 2026 11:02:29 -0400 Subject: [PATCH 2/3] fix(ci): only shell-prefixed 'not found' output counts as a missing tool (D-1192) --- free/ci/internal/diagnose/classify_test.go | 2 ++ free/ci/internal/diagnose/signatures.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/free/ci/internal/diagnose/classify_test.go b/free/ci/internal/diagnose/classify_test.go index 2a3637c4a..3e879d5ba 100644 --- a/free/ci/internal/diagnose/classify_test.go +++ b/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}, diff --git a/free/ci/internal/diagnose/signatures.go b/free/ci/internal/diagnose/signatures.go index 382de72c6..37187f9f5 100644 --- a/free/ci/internal/diagnose/signatures.go +++ b/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.", }, } From f35e1738911ab2c9eaaa5f7ca53650aea61511e0 Mon Sep 17 00:00:00 2001 From: admin Date: Sun, 25 Oct 2026 11:02:29 -0400 Subject: [PATCH 3/3] fix(ci): key flaky annotations by job and check (D-1193) --- free/ci/internal/flaky/annotate.go | 12 +++++++++--- free/ci/internal/flaky/flaky_test.go | 21 +++++++++++++++++++++ free/ci/internal/model/evidence.go | 4 ++++ free/ci/internal/run/flaky_hook.go | 4 ++-- free/ci/internal/run/flaky_hook_test.go | 23 +++++++++++++++++++++++ 5 files changed, 59 insertions(+), 5 deletions(-) create mode 100644 free/ci/internal/run/flaky_hook_test.go diff --git a/free/ci/internal/flaky/annotate.go b/free/ci/internal/flaky/annotate.go index 45e3e8e17..e68f33b8e 100644 --- a/free/ci/internal/flaky/annotate.go +++ b/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,6 +29,12 @@ 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 { @@ -36,7 +42,7 @@ func Annotate(ctx context.Context, st *store.Store, ev model.Evidence) map[strin } 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} } } } diff --git a/free/ci/internal/flaky/flaky_test.go b/free/ci/internal/flaky/flaky_test.go index a8ce2521d..319f40358 100644 --- a/free/ci/internal/flaky/flaky_test.go +++ b/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) + } +} diff --git a/free/ci/internal/model/evidence.go b/free/ci/internal/model/evidence.go index 021858f6c..3685c1dfa 100644 --- a/free/ci/internal/model/evidence.go +++ b/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"` diff --git a/free/ci/internal/run/flaky_hook.go b/free/ci/internal/run/flaky_hook.go index e2731d196..b6888055d 100644 --- a/free/ci/internal/run/flaky_hook.go +++ b/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 } } diff --git a/free/ci/internal/run/flaky_hook_test.go b/free/ci/internal/run/flaky_hook_test.go new file mode 100644 index 000000000..1e84e5ab6 --- /dev/null +++ b/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) + } +}