diff --git a/.agents/pipelines/impl-issue.yaml b/.agents/pipelines/impl-issue.yaml index 398d717e7..6e81e84ab 100644 --- a/.agents/pipelines/impl-issue.yaml +++ b/.agents/pipelines/impl-issue.yaml @@ -233,6 +233,11 @@ steps: command: "{{ project.contract_test_command }}" must_pass: true on_failure: fail + - type: test_diff + on_failure: rework + - type: test_count_baseline + base_ref: HEAD~1 + on_failure: rework - type: agent_review persona: navigator criteria_path: .agents/contracts/impl-review-criteria.md diff --git a/internal/contract/contract.go b/internal/contract/contract.go index 7dc862f96..253f5c05f 100644 --- a/internal/contract/contract.go +++ b/internal/contract/contract.go @@ -63,6 +63,9 @@ type ContractConfig struct { TestFilePattern []string `json:"test_file_pattern,omitempty" yaml:"test_file_pattern,omitempty"` // Pathspecs (e.g. ["*_test.go"], ["**/test_*.py"], ["**/*.test.ts"]) TestFuncPattern string `json:"test_func_pattern,omitempty" yaml:"test_func_pattern,omitempty"` // Regex matching one test declaration per line + // test_count_baseline contract fields — post-commit defense-in-depth alongside test_diff. + BaseRef string `json:"base_ref,omitempty" yaml:"base_ref,omitempty"` // Git ref to compare HEAD against (default HEAD~1) + // event_contains contract fields — validated by executor (needs event store access) Events []EventPattern `json:"events,omitempty" yaml:"events,omitempty"` // Expected event patterns to match against the step's event log @@ -130,6 +133,8 @@ func NewValidator(cfg ContractConfig) ContractValidator { return &sourceDiffValidator{} case "test_diff": return &testDiffValidator{} + case "test_count_baseline": + return &testCountBaselineValidator{} case "agent_review": // agent_review requires an adapter runner — NewValidator returns nil. // The executor uses ValidateWithRunner() instead for this type. diff --git a/internal/contract/test_count_baseline.go b/internal/contract/test_count_baseline.go new file mode 100644 index 000000000..ed1789e3d --- /dev/null +++ b/internal/contract/test_count_baseline.go @@ -0,0 +1,109 @@ +package contract + +import ( + "bytes" + "fmt" + "os/exec" + "path" + "regexp" + "strings" +) + +// testCountBaselineValidator is the post-commit "last line of defense" +// against test deletions. Where test_diff inspects the working-tree +// diff, this one compares COMMITTED tree counts: HEAD vs BaseRef +// (default HEAD~1). Catches deletions that slipped past diff inspection +// (file moves, force-pushes within session, multi-commit sequences). +// +// Language-agnostic: shares TestFilePattern + TestFuncPattern with +// test_diff so a project configures patterns once. +// +// Operation: +// 1. `git ls-tree -r --name-only ` → filter by TestFilePattern globs +// 2. `git show :` per file → count regex matches +// 3. Fail if (base - head) > MaxTestDeletions +type testCountBaselineValidator struct{} + +func (v *testCountBaselineValidator) Validate(cfg ContractConfig, workspacePath string) error { + baseRef := cfg.BaseRef + if baseRef == "" { + baseRef = "HEAD~1" + } + max := cfg.MaxTestDeletions + + globs := cfg.TestFilePattern + if len(globs) == 0 { + globs = []string{defaultTestFilePathspec} + } + patternStr := cfg.TestFuncPattern + if patternStr == "" { + patternStr = defaultTestFuncPattern + } + re, err := regexp.Compile(patternStr) + if err != nil { + return fmt.Errorf("test_count_baseline: invalid TestFuncPattern %q: %w", patternStr, err) + } + + headCount, err := countTestFuncsAtRef(workspacePath, "HEAD", globs, re) + if err != nil { + return nil + } + baseCount, err := countTestFuncsAtRef(workspacePath, baseRef, globs, re) + if err != nil { + return nil + } + + net := baseCount - headCount + if net > max { + return fmt.Errorf("test_count_baseline: HEAD has %d test declarations vs %s=%d (net deletion %d, max allowed %d); persona must replace removed tests, not net-delete them across commits", + headCount, baseRef, baseCount, net, max) + } + return nil +} + +func countTestFuncsAtRef(dir, ref string, globs []string, re *regexp.Regexp) (int, error) { + out, err := runGitCmd(dir, "ls-tree", "-r", "--name-only", ref) + if err != nil { + return 0, err + } + total := 0 + for _, p := range strings.Split(strings.TrimSpace(out), "\n") { + if p == "" || !matchesAnyGlob(p, globs) { + continue + } + blob, err := runGitCmd(dir, "show", ref+":"+p) + if err != nil { + continue + } + total += len(re.FindAllString(blob, -1)) + } + return total, nil +} + +// matchesAnyGlob checks both the full path and the basename against each +// pattern. path.Match doesn't grok `**`, so basename match is a +// pragmatic fallback covering `*_test.go`, `test_*.py`, `*.test.ts`, etc. +func matchesAnyGlob(p string, globs []string) bool { + base := path.Base(p) + for _, g := range globs { + if ok, _ := path.Match(g, p); ok { + return true + } + if ok, _ := path.Match(g, base); ok { + return true + } + } + return false +} + +func runGitCmd(dir string, args ...string) (string, error) { + cmd := exec.Command("git", args...) + cmd.Dir = dir + var out bytes.Buffer + cmd.Stdout = &out + cmd.Stderr = &bytes.Buffer{} + if err := cmd.Run(); err != nil { + return "", err + } + return out.String(), nil +} diff --git a/internal/contract/test_count_baseline_test.go b/internal/contract/test_count_baseline_test.go new file mode 100644 index 000000000..b31ea5800 --- /dev/null +++ b/internal/contract/test_count_baseline_test.go @@ -0,0 +1,145 @@ +package contract + +import ( + "testing" +) + +// initRepoTwoCommits seeds a repo with one *_test.go (2 tests), commits, +// then optionally mutates and recommits. Returns the dir. +func initRepoTwoCommits(t *testing.T, mutate func(dir string)) string { + t.Helper() + dir := t.TempDir() + runGit(t, dir, "init", "-q") + writeFile(t, dir, "x_test.go", `package x + +import "testing" + +func TestAlpha(t *testing.T) { _ = t } +func TestBeta(t *testing.T) { _ = t } +`) + runGit(t, dir, "add", "x_test.go") + runGit(t, dir, "commit", "-q", "-m", "base") + if mutate != nil { + mutate(dir) + runGit(t, dir, "add", "-A") + runGit(t, dir, "commit", "-q", "-m", "head") + } + return dir +} + +func TestTestCountBaseline_NoChange_Passes(t *testing.T) { + dir := initRepoTwoCommits(t, func(d string) { + writeFile(t, d, "y.go", `package x +`) + }) + v := &testCountBaselineValidator{} + if err := v.Validate(ContractConfig{Type: "test_count_baseline"}, dir); err != nil { + t.Fatalf("expected pass, got: %v", err) + } +} + +func TestTestCountBaseline_Addition_Passes(t *testing.T) { + dir := initRepoTwoCommits(t, func(d string) { + writeFile(t, d, "x_test.go", `package x + +import "testing" + +func TestAlpha(t *testing.T) { _ = t } +func TestBeta(t *testing.T) { _ = t } +func TestGamma(t *testing.T) { _ = t } +`) + }) + v := &testCountBaselineValidator{} + if err := v.Validate(ContractConfig{Type: "test_count_baseline"}, dir); err != nil { + t.Fatalf("expected pass on addition, got: %v", err) + } +} + +func TestTestCountBaseline_Deletion_Fails(t *testing.T) { + dir := initRepoTwoCommits(t, func(d string) { + writeFile(t, d, "x_test.go", `package x + +import "testing" + +func TestAlpha(t *testing.T) { _ = t } +`) + }) + v := &testCountBaselineValidator{} + if err := v.Validate(ContractConfig{Type: "test_count_baseline"}, dir); err == nil { + t.Fatal("expected error on deletion, got nil") + } +} + +func TestTestCountBaseline_FileMove_NetsZero(t *testing.T) { + dir := initRepoTwoCommits(t, func(d string) { + // Delete x_test.go, recreate same tests under different filename. + runGit(t, d, "rm", "-q", "x_test.go") + writeFile(t, d, "renamed_test.go", `package x + +import "testing" + +func TestAlpha(t *testing.T) { _ = t } +func TestBeta(t *testing.T) { _ = t } +`) + }) + v := &testCountBaselineValidator{} + if err := v.Validate(ContractConfig{Type: "test_count_baseline"}, dir); err != nil { + t.Fatalf("expected pass on file move, got: %v", err) + } +} + +func TestTestCountBaseline_HigherTolerance_Passes(t *testing.T) { + dir := initRepoTwoCommits(t, func(d string) { + writeFile(t, d, "x_test.go", `package x +`) + }) + v := &testCountBaselineValidator{} + cfg := ContractConfig{Type: "test_count_baseline", MaxTestDeletions: 2} + if err := v.Validate(cfg, dir); err != nil { + t.Fatalf("expected pass with tolerance=2, got: %v", err) + } +} + +func TestTestCountBaseline_NoBaseRef_PassesSilently(t *testing.T) { + // Single-commit repo — HEAD~1 doesn't resolve. + dir := initRepoTwoCommits(t, nil) + v := &testCountBaselineValidator{} + if err := v.Validate(ContractConfig{Type: "test_count_baseline"}, dir); err != nil { + t.Fatalf("expected silent pass without base ref, got: %v", err) + } +} + +func TestTestCountBaseline_PythonConfig_DetectsDeletion(t *testing.T) { + dir := t.TempDir() + runGit(t, dir, "init", "-q") + writeFile(t, dir, "test_things.py", `def test_alpha(): + pass + +def test_beta(): + pass +`) + runGit(t, dir, "add", "-A") + runGit(t, dir, "commit", "-q", "-m", "base") + writeFile(t, dir, "test_things.py", `def test_alpha(): + pass +`) + runGit(t, dir, "add", "-A") + runGit(t, dir, "commit", "-q", "-m", "head") + v := &testCountBaselineValidator{} + cfg := ContractConfig{ + Type: "test_count_baseline", + TestFilePattern: []string{"test_*.py", "*_test.py"}, + TestFuncPattern: `(?m)^[ \t]*def[ \t]+test_\w+`, + } + if err := v.Validate(cfg, dir); err == nil { + t.Fatal("expected error for python deletion, got nil") + } +} + +func TestTestCountBaseline_NoGit_PassesSilently(t *testing.T) { + dir := t.TempDir() + v := &testCountBaselineValidator{} + if err := v.Validate(ContractConfig{Type: "test_count_baseline"}, dir); err != nil { + t.Fatalf("expected silent pass without git, got: %v", err) + } +} diff --git a/internal/contract/test_diff.go b/internal/contract/test_diff.go index fecde4db5..cc144b075 100644 --- a/internal/contract/test_diff.go +++ b/internal/contract/test_diff.go @@ -26,7 +26,7 @@ type testDiffValidator struct{} const ( defaultTestFilePathspec = "*_test.go" - defaultTestFuncPattern = `^[ \t]*func[ \t]+(Test|Example|Benchmark|Fuzz)[A-Za-z0-9_]*\b` + defaultTestFuncPattern = `(?m)^[ \t]*func[ \t]+(Test|Example|Benchmark|Fuzz)[A-Za-z0-9_]*\b` ) func (v *testDiffValidator) Validate(cfg ContractConfig, workspacePath string) error { diff --git a/internal/defaults/embedfs/pipelines/impl-issue.yaml b/internal/defaults/embedfs/pipelines/impl-issue.yaml index 64a415d73..43fdeba1e 100644 --- a/internal/defaults/embedfs/pipelines/impl-issue.yaml +++ b/internal/defaults/embedfs/pipelines/impl-issue.yaml @@ -233,6 +233,11 @@ steps: command: "{{ project.contract_test_command }}" must_pass: true on_failure: fail + - type: test_diff + on_failure: rework + - type: test_count_baseline + base_ref: HEAD~1 + on_failure: rework - type: agent_review persona: navigator criteria_path: .agents/contracts/impl-review-criteria.md diff --git a/internal/manifest/types.go b/internal/manifest/types.go index 80c1cc819..2523831d7 100644 --- a/internal/manifest/types.go +++ b/internal/manifest/types.go @@ -26,6 +26,8 @@ type Project struct { BuildCommand string `yaml:"build_command,omitempty"` FormatCommand string `yaml:"format_command,omitempty"` SourceGlob string `yaml:"source_glob,omitempty"` + TestFilePattern []string `yaml:"test_file_pattern,omitempty"` // test_diff / test_count_baseline pathspecs (#1583, #1584) + TestFuncPattern string `yaml:"test_func_pattern,omitempty"` // test_diff / test_count_baseline regex (#1583, #1584) Skill string `yaml:"skill,omitempty"` Services map[string]ServiceConfig `yaml:"services,omitempty"` } diff --git a/wave.yaml b/wave.yaml index 74e725d62..9e3a6a250 100644 --- a/wave.yaml +++ b/wave.yaml @@ -14,6 +14,12 @@ project: lint_command: "go vet ./..." build_command: "go build ./..." source_glob: "*.go" + # test_diff / test_count_baseline contracts (#1583, #1584): how to + # spot test declarations in this codebase. Defaults already match + # Go, but make it explicit so non-Go projects know the knobs. + test_file_pattern: + - "*_test.go" + test_func_pattern: '(?m)^[ \t]*func[ \t]+(Test|Example|Benchmark|Fuzz)[A-Za-z0-9_]*\b' adapters: claude: binary: claude