From 9bdf6b07d4891d4f051b0532bac9ccfd445c52f6 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 04:12:15 +0000 Subject: [PATCH 1/9] perf(index): decode covdata binary formats in-process, covdata subprocess as fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Window assembly shelled out to 'go tool covdata textfmt' once per test, each invocation re-reading the package's whole-closure meta blob and serialising a text profile dominated by zero-count lines that ParseProfile then discarded. On a large workspace that per-test round-trip is most of the indexing wall clock. The meta and counter files are now decoded in this process: the meta is read once per package and each window costs one small counter-file read touching only the units the test actually executed. Both formats are version-stamped and unchanged since Go 1.20; every read is bounds-checked, and any structural surprise — future version, foreign magic, meta-hash mismatch, truncation — falls back to the covdata subprocess path wholesale, so an incompatible future toolchain costs time, never correctness. A parity test drives the real toolchain end to end and requires the decoder's blocks to equal the textfmt path's exactly, init window and skipped-test windows included. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01211tSDyqM7KnfziwEjPzDF --- internal/cover/classify.go | 2 +- internal/cover/profile.go | 9 +- internal/index/covdata.go | 489 +++++++++++++++++++++++++++++++++ internal/index/covdata_test.go | 181 ++++++++++++ internal/index/windows.go | 123 ++++++--- 5 files changed, 762 insertions(+), 42 deletions(-) create mode 100644 internal/index/covdata.go create mode 100644 internal/index/covdata_test.go diff --git a/internal/cover/classify.go b/internal/cover/classify.go index 637bdc0..dac3c7c 100644 --- a/internal/cover/classify.go +++ b/internal/cover/classify.go @@ -315,7 +315,7 @@ func mergeBlocksOrEmpty(blocks []Block) []Block { return nil } - return mergeBlocks(blocks) + return MergeBlocks(blocks) } func splitLines(source []byte) []string { diff --git a/internal/cover/profile.go b/internal/cover/profile.go index a14844a..84bedeb 100644 --- a/internal/cover/profile.go +++ b/internal/cover/profile.go @@ -43,7 +43,7 @@ func ParseProfile(reader io.Reader) ([]FileBlocks, error) { if len(blocks) == 0 { continue } - out = append(out, FileBlocks{ProfileName: profile.FileName, Blocks: mergeBlocks(blocks)}) + out = append(out, FileBlocks{ProfileName: profile.FileName, Blocks: MergeBlocks(blocks)}) } sort.Slice(out, func(i, j int) bool { return out[i].ProfileName < out[j].ProfileName }) @@ -51,7 +51,12 @@ func ParseProfile(reader io.Reader) ([]FileBlocks, error) { return out, nil } -func mergeBlocks(blocks []Block) []Block { +// MergeBlocks sorts blocks and coalesces overlapping or line-adjacent ones, +// the same normalisation ParseProfile applies to text profiles. Callers that +// assemble blocks from another source (the binary covdata decoder) use it so +// both paths emit identical shapes. The input must be non-empty; it is +// modified in place. +func MergeBlocks(blocks []Block) []Block { sort.Slice(blocks, func(i, j int) bool { if blocks[i].StartLine != blocks[j].StartLine { return blocks[i].StartLine < blocks[j].StartLine diff --git a/internal/index/covdata.go b/internal/index/covdata.go new file mode 100644 index 0000000..baa3ecf --- /dev/null +++ b/internal/index/covdata.go @@ -0,0 +1,489 @@ +package index + +// In-process decoding of the Go toolchain's binary coverage files — the +// covmeta blob written once per package and the covcounters blob written per +// window. Shelling out to `go tool covdata textfmt` for every window meant one +// subprocess per test, each re-reading the package's whole-closure meta and +// serialising a text profile dominated by zero-count lines that the parser +// then discarded. On a large workspace that round-trip is the indexing wall +// clock. Decoding here reads the meta once per package and touches only the +// units a window actually executed. +// +// The formats are internal to the Go toolchain (src/internal/coverage/defs.go) +// and version-stamped: meta files carry MetaFileVersion 1, counter files +// CounterFileVersion 1, unchanged since Go 1.20. Every read is bounds-checked +// and every structural surprise — future version, foreign magic, hash +// mismatch, truncation — returns an error rather than a guess, and the caller +// falls back to the covdata subprocess path, which stays first-class. Being +// wrong here can cost a fallback; it must never cost a wrong record. + +import ( + "encoding/binary" + "errors" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/emoss08/assay/internal/cover" +) + +var ( + covMetaMagic = [4]byte{0x00, 'c', 'v', 'm'} + covCounterMagic = [4]byte{0x00, 'c', 'w', 'm'} +) + +const ( + covMetaVersion = 1 + covCounterVersion = 1 + + // Counter flavors: how function records encode their integers. + ctrRaw = 1 + ctrULeb128 = 2 + + // Counter granularities. Per-block is what -covermode emits; a per-func + // meta attributes one counter to every unit of the function. + granularityPerBlock = 1 + granularityPerFunc = 2 + + counterFilePrefix = "covcounters." + metaFilePrefix = "covmeta." +) + +// errCorrupt is the base of every structural complaint; callers only ever +// treat decoding as all-or-nothing, so one sentinel is enough. +var errCorrupt = errors.New("unexpected coverage data layout") + +// covMeta is one package's decoded meta-data file: the function tables for +// every instrumented package linked into the test binary, indexed by the +// package ID that counter records refer to. +type covMeta struct { + hash [16]byte + perFunc bool + packages []covMetaPackage +} + +type covMetaPackage struct { + funcs []covMetaFunc +} + +// covMetaFunc keeps only what window assembly consumes: the profile-style +// source name ("importpath/file.go", exactly what covdata textfmt prints) and +// each unit's line range, positionally matched to the function's counters. +type covMetaFunc struct { + srcFile string + units []covUnit +} + +type covUnit struct { + start, end uint32 +} + +// byteReader reads little-endian scalars from an in-memory file with sticky +// error state, so decode paths read straight through and check once. The +// stdlib's slicereader panics on truncated input; these files come from disk +// and truncation must surface as an error. +type byteReader struct { + b []byte + off int + err error +} + +func (r *byteReader) fail() { + if r.err == nil { + r.err = errCorrupt + } +} + +func (r *byteReader) bytes(n int) []byte { + if r.err != nil || n < 0 || r.off+n > len(r.b) { + r.fail() + + return nil + } + out := r.b[r.off : r.off+n] + r.off += n + + return out +} + +func (r *byteReader) u8() uint8 { + b := r.bytes(1) + if b == nil { + return 0 + } + + return b[0] +} + +func (r *byteReader) u32() uint32 { + b := r.bytes(4) + if b == nil { + return 0 + } + + return binary.LittleEndian.Uint32(b) +} + +func (r *byteReader) u32be() uint32 { + b := r.bytes(4) + if b == nil { + return 0 + } + + return binary.BigEndian.Uint32(b) +} + +func (r *byteReader) u64() uint64 { + b := r.bytes(8) + if b == nil { + return 0 + } + + return binary.LittleEndian.Uint64(b) +} + +func (r *byteReader) uleb() uint64 { + var value uint64 + var shift uint + for { + b := r.bytes(1) + if b == nil { + return 0 + } + value |= uint64(b[0]&0x7f) << shift + if b[0]&0x80 == 0 { + return value + } + shift += 7 + if shift > 63 { + r.fail() + + return 0 + } + } +} + +func (r *byteReader) seek(off int) { + if r.err != nil || off < 0 || off > len(r.b) { + r.fail() + + return + } + r.off = off +} + +// align4 skips padding up to the next 4-byte file offset, mirroring the +// preamble padding the counter writer inserts. +func (r *byteReader) align4() { + if rem := r.off % 4; rem != 0 { + r.bytes(4 - rem) + } +} + +// readStringTable decodes the uleb-prefixed string table both file kinds +// embed: entry count, then length-prefixed bytes per entry. +func (r *byteReader) readStringTable() []string { + count := r.uleb() + if r.err != nil || count > uint64(len(r.b)) { + r.fail() + + return nil + } + out := make([]string, 0, count) + for range count { + out = append(out, string(r.bytes(int(r.uleb())))) + } + + return out +} + +// readMetaDir locates and decodes the single covmeta file the harness wrote +// into the package's meta directory. +func readMetaDir(dir string) (*covMeta, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return nil, fmt.Errorf("read meta directory: %w", err) + } + + for _, entry := range entries { + if entry.IsDir() || !strings.HasPrefix(entry.Name(), metaFilePrefix) { + continue + } + payload, readErr := os.ReadFile(filepath.Join(dir, entry.Name())) + if readErr != nil { + return nil, fmt.Errorf("read meta file: %w", readErr) + } + + return decodeMeta(payload) + } + + return nil, fmt.Errorf("%w: no %s* file in %s", errCorrupt, metaFilePrefix, dir) +} + +func decodeMeta(payload []byte) (*covMeta, error) { + r := &byteReader{b: payload} + + var magic [4]byte + copy(magic[:], r.bytes(4)) + if magic != covMetaMagic { + return nil, fmt.Errorf("%w: not a coverage meta file", errCorrupt) + } + if version := r.u32(); version != covMetaVersion { + return nil, fmt.Errorf("%w: meta file version %d, can decode %d", + errCorrupt, version, covMetaVersion) + } + + totalLength := r.u64() + entries := r.u64() + if totalLength > uint64(len(payload)) || entries > uint64(len(payload)) { + return nil, errCorrupt + } + + meta := &covMeta{} + copy(meta.hash[:], r.bytes(16)) + + r.u32() // string table offset; the file-level table is not consulted + r.u32() // string table length + r.u8() // counter mode + meta.perFunc = r.u8() == granularityPerFunc + r.bytes(6) // header padding + + offsets := make([]uint64, entries) + for i := range offsets { + offsets[i] = r.u64() + } + lengths := make([]uint64, entries) + for i := range lengths { + lengths[i] = r.u64() + } + if r.err != nil { + return nil, r.err + } + + meta.packages = make([]covMetaPackage, entries) + for i := range meta.packages { + end := offsets[i] + lengths[i] + if offsets[i] > uint64(len(payload)) || end < offsets[i] || end > uint64(len(payload)) { + return nil, errCorrupt + } + pkg, err := decodeMetaPackage(payload[offsets[i]:end]) + if err != nil { + return nil, err + } + meta.packages[i] = pkg + } + + return meta, nil +} + +// decodeMetaPackage decodes one package's self-contained meta blob: symbol +// header, function offset table, package-local string table, then per-function +// unit lists. +func decodeMetaPackage(blob []byte) (covMetaPackage, error) { + const symbolHeaderSize = 44 // coverage.CovMetaHeaderSize + + r := &byteReader{b: blob} + if length := r.u32(); uint64(length) != uint64(len(blob)) { + return covMetaPackage{}, errCorrupt + } + r.u32() // package name index + r.u32() // package path index + r.u32() // module path index + r.bytes(16) + r.bytes(4) // one unused byte plus padding + r.u32() // file count; files resolve through the string table + numFuncs := r.u32() + if r.err != nil || r.off != symbolHeaderSize || uint64(numFuncs) > uint64(len(blob)) { + return covMetaPackage{}, errCorrupt + } + + funcOffsets := make([]uint32, numFuncs) + for i := range funcOffsets { + funcOffsets[i] = r.u32() + } + + strings := r.readStringTable() + if r.err != nil { + return covMetaPackage{}, r.err + } + + pkg := covMetaPackage{funcs: make([]covMetaFunc, numFuncs)} + for i, off := range funcOffsets { + r.seek(int(off)) + numUnits := r.uleb() + nameIdx := r.uleb() + fileIdx := r.uleb() + if r.err != nil || numUnits > uint64(len(blob)) || + nameIdx >= uint64(len(strings)) || fileIdx >= uint64(len(strings)) { + return covMetaPackage{}, errCorrupt + } + + fn := covMetaFunc{srcFile: strings[fileIdx], units: make([]covUnit, numUnits)} + for u := range fn.units { + stLine := r.uleb() + r.uleb() // start column + enLine := r.uleb() + r.uleb() // end column + r.uleb() // statement count + fn.units[u] = covUnit{start: uint32(stLine), end: uint32(enLine)} + } + if r.err != nil { + return covMetaPackage{}, r.err + } + pkg.funcs[i] = fn + } + + return pkg, nil +} + +// windowBlocks decodes every counter file in one window directory and returns +// the executed source blocks in the same shape ParseProfile produces from a +// text profile: per-file, count-filtered, merged, and sorted by profile name. +func (m *covMeta) windowBlocks(dir string) ([]cover.FileBlocks, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return nil, fmt.Errorf("read counter directory: %w", err) + } + + perFile := make(map[string][]cover.Block) + found := false + for _, entry := range entries { + if entry.IsDir() || !strings.HasPrefix(entry.Name(), counterFilePrefix) { + continue + } + found = true + payload, readErr := os.ReadFile(filepath.Join(dir, entry.Name())) + if readErr != nil { + return nil, fmt.Errorf("read counter file: %w", readErr) + } + if decodeErr := m.accumulateCounters(payload, perFile); decodeErr != nil { + return nil, decodeErr + } + } + if !found { + return nil, fmt.Errorf("%w: no %s* file in %s", errCorrupt, counterFilePrefix, dir) + } + + out := make([]cover.FileBlocks, 0, len(perFile)) + for file, blocks := range perFile { + out = append(out, cover.FileBlocks{ProfileName: file, Blocks: cover.MergeBlocks(blocks)}) + } + sort.Slice(out, func(i, j int) bool { return out[i].ProfileName < out[j].ProfileName }) + + return out, nil +} + +// accumulateCounters walks one counter file's segments and records the line +// ranges of every unit with a nonzero counter. +func (m *covMeta) accumulateCounters(payload []byte, perFile map[string][]cover.Block) error { + const headerSize = 32 + const footerSize = 16 + + r := &byteReader{b: payload} + + var magic [4]byte + copy(magic[:], r.bytes(4)) + if magic != covCounterMagic { + return fmt.Errorf("%w: not a coverage counter file", errCorrupt) + } + if version := r.u32(); version != covCounterVersion { + return fmt.Errorf("%w: counter file version %d, can decode %d", + errCorrupt, version, covCounterVersion) + } + + var hash [16]byte + copy(hash[:], r.bytes(16)) + if hash != m.hash { + return fmt.Errorf("%w: counter file does not match the meta file", errCorrupt) + } + + flavor := r.u8() + bigEndian := r.u8() != 0 + r.bytes(6) // header padding + + var next func() uint32 + switch flavor { + case ctrULeb128: + next = func() uint32 { return uint32(r.uleb()) } + case ctrRaw: + if bigEndian { + next = r.u32be + } else { + next = r.u32 + } + default: + return fmt.Errorf("%w: unknown counter flavor %d", errCorrupt, flavor) + } + + if len(payload) < headerSize+footerSize { + return errCorrupt + } + footer := &byteReader{b: payload, off: len(payload) - footerSize} + copy(magic[:], footer.bytes(4)) + footer.bytes(4) + segments := footer.u32() + if magic != covCounterMagic || footer.err != nil { + return errCorrupt + } + + for segment := range segments { + if segment > 0 { + r.bytes(footerSize) // the previous segment's footer + } + + fcnEntries := r.u64() + strTabLen := r.u32() + argsLen := r.u32() + if r.err != nil || fcnEntries > uint64(len(payload)) { + return errCorrupt + } + r.bytes(int(strTabLen)) + r.bytes(int(argsLen)) + r.align4() + + for range fcnEntries { + numCounters := next() + pkgIdx := next() + funcIdx := next() + if r.err != nil { + return r.err + } + + // An index outside the meta is ignored rather than fatal — the + // same policy covdata itself applies to inconsistent payloads. + var fn *covMetaFunc + if int(pkgIdx) < len(m.packages) && + int(funcIdx) < len(m.packages[pkgIdx].funcs) { + fn = &m.packages[pkgIdx].funcs[funcIdx] + } + + for i := range int(numCounters) { + count := next() + if r.err != nil { + return r.err + } + if count == 0 || fn == nil { + continue + } + if m.perFunc { + // One counter stands for the whole function. + for _, unit := range fn.units { + perFile[fn.srcFile] = append(perFile[fn.srcFile], + cover.Block{StartLine: int(unit.start), EndLine: int(unit.end)}) + } + + continue + } + if i < len(fn.units) { + unit := fn.units[i] + perFile[fn.srcFile] = append(perFile[fn.srcFile], + cover.Block{StartLine: int(unit.start), EndLine: int(unit.end)}) + } + } + } + } + + return r.err +} diff --git a/internal/index/covdata_test.go b/internal/index/covdata_test.go new file mode 100644 index 0000000..807c6ca --- /dev/null +++ b/internal/index/covdata_test.go @@ -0,0 +1,181 @@ +package index + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/emoss08/assay/internal/overlay" +) + +// TestDecodeWindowsAgreesWithCovdataTextfmt is the contract for the in-process +// covdata decoder: for the same counter directory, decodeWindows and the +// `go tool covdata textfmt` subprocess path must produce identical blocks. It +// drives the real toolchain — a real instrumented build, a real harness run — +// so a Go release that changes the binary formats fails this test rather than +// silently corrupting records; decoding then rejects the version and every +// caller falls back to the subprocess path. +func TestDecodeWindowsAgreesWithCovdataTextfmt(t *testing.T) { + t.Parallel() + + root := t.TempDir() + writeCovdataFixture(t, root) + + workdir := t.TempDir() + injected := filepath.Join(root, "fixture", HarnessFileName) + harnessPath, err := overlay.WriteUnderBasename(workdir, "harness", injected, HarnessSource("fixture")) + require.NoError(t, err) + overlayPath, err := overlay.WriteFile(workdir, map[string]string{injected: harnessPath}) + require.NoError(t, err) + + ctx := context.Background() + opts := Options{Root: root} + + binary := filepath.Join(workdir, "harness.bin") + require.NoError(t, buildTestBinary( + ctx, opts, "example.com/covfixture/...", "example.com/covfixture/fixture", binary, overlayPath, 2)) + + dir := filepath.Join(root, "fixture") + names, err := listTests(ctx, opts, binary, dir) + require.NoError(t, err) + require.NotEmpty(t, names) + + listFile := filepath.Join(workdir, "tests.list") + require.NoError(t, os.WriteFile(listFile, []byte(strings.Join(names, "\n")+"\n"), 0o644)) + + counterDir := filepath.Join(workdir, "counters") + require.NoError(t, os.MkdirAll(filepath.Join(counterDir, "assay-final"), 0o755)) + + run := runHarness(ctx, opts, binary, dir, listFile, counterDir, names) + require.NoError(t, run.err) + require.Len(t, run.completed, len(names)) + + windows := make([]string, 0, len(names)+1) + windows = append(windows, initWindowName) + windows = append(windows, names...) + + decoded, err := decodeWindows(counterDir, windows) + require.NoError(t, err) + + converted, err := convertWindows(ctx, opts, counterDir, windows, 2) + require.NoError(t, err) + + require.Equal(t, converted, decoded, + "in-process covdata decoding must match `go tool covdata textfmt` exactly") + + // The decoder must have seen real coverage, not agreed on emptiness. + total := 0 + for _, blocks := range decoded { + total += len(blocks) + } + require.Positive(t, total) +} + +func TestDecodeMetaRejectsForeignPayloads(t *testing.T) { + t.Parallel() + + _, err := decodeMeta([]byte("not a meta file at all")) + require.Error(t, err) + + _, err = decodeMeta(nil) + require.Error(t, err) + + // A valid magic with a future version must refuse, not guess. + future := append([]byte{0x00, 'c', 'v', 'm'}, []byte{2, 0, 0, 0}...) + future = append(future, make([]byte, 64)...) + _, err = decodeMeta(future) + require.ErrorContains(t, err, "version") +} + +func TestAccumulateCountersRejectsForeignPayloads(t *testing.T) { + t.Parallel() + + meta := &covMeta{} + + require.Error(t, meta.accumulateCounters([]byte("junk"), nil)) + + // Valid magic and version but a mismatched meta hash must refuse: pairing + // counters with a meta they were not produced against fabricates coverage. + mismatched := append([]byte{0x00, 'c', 'w', 'm'}, []byte{1, 0, 0, 0}...) + mismatched = append(mismatched, []byte("0123456789abcdef")...) + mismatched = append(mismatched, make([]byte, 40)...) + require.Error(t, meta.accumulateCounters(mismatched, nil)) +} + +// writeCovdataFixture lays out a module whose coverage exercises the decoder's +// interesting axes: init-time execution, multiple files in one package, a +// dependency package pulled in through -coverpkg, per-test disjoint execution, +// and a skipped test with an empty window. +func writeCovdataFixture(t *testing.T, root string) { + t.Helper() + + files := map[string]string{ + "go.mod": "module example.com/covfixture\n\ngo 1.26\n", + + "dep/dep.go": `package dep + +func Double(v int) int { + return v * 2 +} +`, + "fixture/a.go": `package fixture + +import "example.com/covfixture/dep" + +var initialised = seed() + +func seed() int { + return dep.Double(21) +} + +func Left(v int) int { + if v > 0 { + return v + initialised + } + + return -v +} +`, + "fixture/b.go": `package fixture + +func Right(v int) int { + out := 0 + for i := 0; i < v; i++ { + out += i + } + + return out +} +`, + "fixture/fixture_test.go": `package fixture + +import "testing" + +func TestLeft(t *testing.T) { + if Left(1) != 43 { + t.Fatal("bad") + } +} + +func TestRight(t *testing.T) { + if Right(3) != 3 { + t.Fatal("bad") + } +} + +func TestSkipped(t *testing.T) { + t.Skip("nothing to see") +} +`, + } + + for name, content := range files { + path := filepath.Join(root, filepath.FromSlash(name)) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(content), 0o644)) + } +} diff --git a/internal/index/windows.go b/internal/index/windows.go index cbdc9ae..5a13849 100644 --- a/internal/index/windows.go +++ b/internal/index/windows.go @@ -11,8 +11,15 @@ import ( "github.com/emoss08/assay/internal/cover" ) -// assembleWindows converts the completed counter windows to text profiles and -// builds the record from them. +// assembleWindows converts the completed counter windows to executed source +// blocks and builds the record from them. +// +// The fast path decodes the binary covmeta/covcounters files in this process: +// the meta is read once per package and each window costs one small counter +// file, instead of one `go tool covdata textfmt` subprocess per test each +// re-reading the meta and serialising a mostly-zero text profile. Any decoding +// surprise falls back to the subprocess path wholesale — the two produce +// identical blocks, so the fallback costs time, never correctness. // // The init window — everything the package executed before the first test — is // merged into every test's ranges, and merged *before* the empty-ranges check @@ -33,8 +40,12 @@ func assembleWindows( windows = append(windows, progress.name) } - if err := convertWindows(ctx, opts, counterDir, windows, buildParallelism); err != nil { - return nil, err + blocks, err := decodeWindows(counterDir, windows) + if err != nil { + blocks, err = convertWindows(ctx, opts, counterDir, windows, buildParallelism) + if err != nil { + return nil, err + } } record := &Record{Version: recordVersion, Package: importPath, IndexedAt: opts.Commit} @@ -47,17 +58,11 @@ func assembleWindows( } } - initRanges, unresolved, err := parseWindow(counterDir, initWindowName, build, resolve) - if err != nil { - return nil, err - } + initRanges, unresolved := blocksToRanges(blocks[0], build, resolve) degrade(unresolved) - for _, progress := range completed { - ranges, unresolvedTest, parseErr := parseWindow(counterDir, progress.name, build, resolve) - if parseErr != nil { - return nil, parseErr - } + for i, progress := range completed { + ranges, unresolvedTest := blocksToRanges(blocks[i+1], build, resolve) degrade(unresolvedTest) ranges = append(ranges, initRanges...) @@ -75,24 +80,54 @@ func assembleWindows( return record, nil } -// convertWindows runs `go tool covdata textfmt` for each window, pairing each -// counters-only window with the package's single meta directory — covdata -// matches them by hash across input directories. +// decodeWindows is the in-process path: one meta decode, then one counter-file +// decode per window. Windows are independent and the meta is read-only, so a +// package with thousands of tests still assembles quickly; the work per window +// is proportional to what the test executed, not to the closure. +func decodeWindows(counterDir string, windows []string) ([][]cover.FileBlocks, error) { + meta, err := readMetaDir(filepath.Join(counterDir, metaDirName)) + if err != nil { + return nil, err + } + + out := make([][]cover.FileBlocks, len(windows)) + for i, window := range windows { + blocks, decodeErr := meta.windowBlocks(filepath.Join(counterDir, window)) + if decodeErr != nil { + return nil, decodeErr + } + out[i] = blocks + } + + return out, nil +} + +// convertWindows is the subprocess fallback: `go tool covdata textfmt` for +// each window, pairing each counters-only window with the package's single +// meta directory — covdata matches them by hash across input directories. // // The pool is bounded by the caller's share of the cores, not by GOMAXPROCS: // this runs inside one of `jobs` package workers, and an inner bound that // ignores the outer one multiplies into cores²/4 concurrent covdata processes — // the exact trap the build path in this package already documents. -func convertWindows(ctx context.Context, opts Options, counterDir string, windows []string, buildParallelism int) error { +func convertWindows( + ctx context.Context, + opts Options, + counterDir string, + windows []string, + buildParallelism int, +) ([][]cover.FileBlocks, error) { workers := min(max(1, buildParallelism), len(windows)) + out := make([][]cover.FileBlocks, len(windows)) + converting := pool.New(). WithMaxGoroutines(workers). WithContext(ctx). WithFirstError(). WithCancelOnError() - for _, window := range windows { + for i, window := range windows { converting.Go(func(ctx context.Context) error { if err := ctx.Err(); err != nil { return err @@ -106,29 +141,50 @@ func convertWindows(ctx context.Context, opts Options, counterDir string, window return fmt.Errorf("convert coverage window %s: %w", window, err) } + blocks, err := parseWindowFile(output) + if err != nil { + return err + } + out[i] = blocks + return nil }) } - return converting.Wait() + if err := converting.Wait(); err != nil { + return nil, err + } + + return out, nil } -func parseWindow( - counterDir, window string, - build *builder, - resolve cover.Resolver, -) ([]Range, string, error) { - file, err := os.Open(filepath.Join(counterDir, window) + ".txt") +func parseWindowFile(path string) ([]cover.FileBlocks, error) { + file, err := os.Open(path) if err != nil { - return nil, "", fmt.Errorf("open converted window %s: %w", window, err) + return nil, fmt.Errorf("open converted window %s: %w", filepath.Base(path), err) } defer file.Close() blocks, err := cover.ParseProfile(file) if err != nil { - return nil, "", fmt.Errorf("parse coverage window %s: %w", window, err) + return nil, fmt.Errorf("parse coverage window %s: %w", filepath.Base(path), err) } + return blocks, nil +} + +// rebuilderFrom continues a record's file interning where assembly left it, so +// salvaged tests share file ids with the harness-collected ones. +func rebuilderFrom(record *Record) *builder { + build := newBuilder() + for _, file := range record.Files { + build.intern(file) + } + + return build +} + +func blocksToRanges(blocks []cover.FileBlocks, build *builder, resolve cover.Resolver) ([]Range, string) { var ranges []Range var unresolved string for _, fileBlocks := range blocks { @@ -150,16 +206,5 @@ func parseWindow( } } - return ranges, unresolved, nil -} - -// rebuilderFrom continues a record's file interning where assembly left it, so -// salvaged tests share file ids with the harness-collected ones. -func rebuilderFrom(record *Record) *builder { - build := newBuilder() - for _, file := range record.Files { - build.intern(file) - } - - return build + return ranges, unresolved } From 6fc657b7d6eb3ccc85041902fe3055515a39b4ca Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 04:33:56 +0000 Subject: [PATCH 2/9] fix(selection): never let a test-file or fixture edit under-select MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two silent under-selection holes, both found by review: An edit inside an existing test's body was classified narrowable, but index records never contain _test.go paths — coverage instruments production code only — so attribution found nothing, the package was dropped, and the edited test never ran. Test files now take the blocking path: the owning package runs in full. Comment-only test edits still drop safely. A go.mod anywhere in the tree, matched by base name alone, forced a whole-workspace run — including fixtures under testdata that the build never reads. Testdata module files now attribute to their owning package like any other fixture. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01211tSDyqM7KnfziwEjPzDF --- internal/selection/narrow.go | 11 +++++++++++ internal/selection/narrow_test.go | 20 ++++++++++++++++++++ internal/selection/selection.go | 19 ++++++++++++++++++- internal/selection/selection_test.go | 13 +++++++++++++ 4 files changed, 62 insertions(+), 1 deletion(-) diff --git a/internal/selection/narrow.go b/internal/selection/narrow.go index d5fb623..59a04ef 100644 --- a/internal/selection/narrow.go +++ b/internal/selection/narrow.go @@ -3,6 +3,7 @@ package selection import ( "path/filepath" "sort" + "strings" "github.com/emoss08/assay/internal/cover" "github.com/emoss08/assay/internal/graph" @@ -159,6 +160,16 @@ func partitionChanges(changes []vcs.Change) ([]fileChange, []string) { if len(classification.Executable) == 0 { continue } + // A test file is executable and narrowable by classification, but it can + // never be attributed: coverage instruments only production code, so no + // index record knows a _test.go path, and treating the change as + // attributable would silently select nothing — the edited test itself + // would not run. The package runs in full instead. + if strings.HasSuffix(filepath.Base(change.Path), "_test.go") { + blocking = append(blocking, change.Path) + + continue + } baseLines := change.BaseLineNumbers() if len(baseLines) == 0 { diff --git a/internal/selection/narrow_test.go b/internal/selection/narrow_test.go index 647fc73..8da0127 100644 --- a/internal/selection/narrow_test.go +++ b/internal/selection/narrow_test.go @@ -193,6 +193,26 @@ func TestNarrowFallsBackOnNonGoFile(t *testing.T) { "coverage tracks Go lines only") } +func TestNarrowFallsBackOnTestFileChange(t *testing.T) { + h := newNarrowHarness(t) + + // Line 6 is the body of TestAdd — executable, narrowable, and invisible to + // the index, which never contains _test.go files. Before test files were + // forced onto the blocking path this selected nothing at all, silently + // skipping the very test that was edited. + got := h.narrow(t, []vcs.Change{{ + Path: filepath.Join(h.root, "calc", "calc_test.go"), + Status: "M", + Lines: []vcs.LineRange{{Start: 6, End: 6}}, + BaseLines: []vcs.LineRange{{Start: 6, End: 6}}, + }}) + + plan := planFor(t, got, testfixture.Module+"/calc") + assert.NotEmpty(t, plan.FullReason, + "a test-body edit cannot be attributed by the index; the package must run in full") + assert.Contains(t, got.Blocked, filepath.Join(h.root, "calc", "calc_test.go")) +} + func TestNarrowSurvivesIndexFromAnotherCommit(t *testing.T) { h := newNarrowHarness(t) changes := []vcs.Change{h.calcChange(testfixture.CalcAddBodyLine)} diff --git a/internal/selection/selection.go b/internal/selection/selection.go index e5871c6..800fb18 100644 --- a/internal/selection/selection.go +++ b/internal/selection/selection.go @@ -3,6 +3,7 @@ package selection import ( "path/filepath" "sort" + "strings" "github.com/emoss08/assay/internal/graph" "github.com/emoss08/assay/internal/vcs" @@ -45,7 +46,11 @@ func Select(opts Options) Result { for _, change := range opts.Changes { base := filepath.Base(change.Path) - if _, isTrigger := globalTriggerFiles[base]; isTrigger { + // A go.mod under a testdata directory is a fixture, not a module + // boundary — the build ignores testdata entirely — so it attributes to + // its owning package like any other fixture file instead of forcing a + // whole-workspace run. + if _, isTrigger := globalTriggerFiles[base]; isTrigger && !underTestdata(change.Path) { return All(g, "module definition changed: "+base) } @@ -90,6 +95,18 @@ func Select(opts Options) Result { } } +// underTestdata reports whether any element of the path is a testdata +// directory, which the Go build system never looks inside. +func underTestdata(path string) bool { + for _, segment := range strings.Split(filepath.ToSlash(path), "/") { + if segment == "testdata" { + return true + } + } + + return false +} + func All(g *graph.Graph, reason string) Result { packages := g.TestablePackages() reasons := make(map[string]Reason, len(packages)) diff --git a/internal/selection/selection_test.go b/internal/selection/selection_test.go index 3eb9457..019c3a7 100644 --- a/internal/selection/selection_test.go +++ b/internal/selection/selection_test.go @@ -98,6 +98,19 @@ func TestSelectFallsBackToAllOnModuleChange(t *testing.T) { } } +// TestSelectTreatsTestdataModuleFilesAsFixtures pins the trigger's scope: the +// build never reads a go.mod under testdata, so editing one is a fixture +// change for its owning package, not a workspace-wide module event. +func TestSelectTreatsTestdataModuleFilesAsFixtures(t *testing.T) { + g, root := loadFixture(t) + + res := selectFor(t, g, root, "svc/testdata/proj/go.mod") + + assert.False(t, res.SelectAll, "a fixture go.mod must not force a full run") + assert.Equal(t, []string{mod + "/app", mod + "/svc"}, res.Packages, + "the fixture attributes to svc like any other testdata file") +} + func TestSelectFallsBackToAllOnUnattributedGoFile(t *testing.T) { g, root := loadFixture(t) From b440387afc712bcd70bc25ba84c1929c42473e69 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 04:33:56 +0000 Subject: [PATCH 3/9] fix(index): scope fingerprints to the workspace root and own testdata module files Records intern absolute paths, but the fingerprint hashed nothing root-dependent: a second checkout sharing the cache directory got byte-identical fingerprints, hit records whose every path missed, and silently narrowed to nothing. The root is now part of every fingerprint. A go.mod under a package's testdata was classified unowned and hashed into every package's fingerprint, so editing one fixture invalidated the whole index. It now attributes to its owning package. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01211tSDyqM7KnfziwEjPzDF --- internal/index/collect_test.go | 37 ++++++++++++++++++++++++++++++++++ internal/index/fingerprint.go | 24 ++++++++++++++++++---- 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/internal/index/collect_test.go b/internal/index/collect_test.go index 6a2167e..12aba30 100644 --- a/internal/index/collect_test.go +++ b/internal/index/collect_test.go @@ -222,6 +222,43 @@ func TestFingerprintDiffersPerPackage(t *testing.T) { assert.Equal(t, calc, fp.For(testfixture.Module+"/calc"), "fingerprints must be stable") } +func TestFingerprintTracksWorkspaceRoot(t *testing.T) { + h := setup(t) + other := setup(t) + + require.NotEqual(t, h.root, other.root) + + same := index.NewFingerprinter(h.graph, h.digests, nil).For(testfixture.Module + "/calc") + moved := index.NewFingerprinter(other.graph, other.digests, nil).For(testfixture.Module + "/calc") + + assert.NotEqual(t, same, moved, + "records store absolute paths, so an identical tree at another root must not share records") +} + +// TestFingerprintScopesTestdataModuleFilesToTheirPackage pins the ownership +// rule for fixture module files: a go.mod under a package's testdata belongs +// to that package. Classified as unowned it was hashed into every package's +// fingerprint, and editing one fixture invalidated the entire index. +func TestFingerprintScopesTestdataModuleFilesToTheirPackage(t *testing.T) { + h := setup(t) + + before := index.NewFingerprinter(h.graph, h.digests, nil) + calcBefore := before.For(testfixture.Module + "/calc") + repoBefore := before.For(testfixture.Module + "/repo") + + digests := make(map[string]string, len(h.digests)+1) + for path, digest := range h.digests { + digests[path] = digest + } + digests["calc/testdata/proj/go.mod"] = "0000000000000000000000000000000000000001" + + after := index.NewFingerprinter(h.graph, digests, nil) + assert.NotEqual(t, calcBefore, after.For(testfixture.Module+"/calc"), + "the fixture belongs to calc and must invalidate it") + assert.Equal(t, repoBefore, after.For(testfixture.Module+"/repo"), + "a calc fixture must not invalidate unrelated packages") +} + func TestFingerprintTracksDependencies(t *testing.T) { h := setup(t) before := index.NewFingerprinter(h.graph, h.digests, nil).For(testfixture.Module + "/svc") diff --git a/internal/index/fingerprint.go b/internal/index/fingerprint.go index 7e71b9b..c5b5ebd 100644 --- a/internal/index/fingerprint.go +++ b/internal/index/fingerprint.go @@ -25,10 +25,16 @@ func NewFingerprinter(g *graph.Graph, treeDigests map[string]string, tags []stri sorted := append([]string(nil), tags...) sort.Strings(sorted) + // The workspace root is part of the fingerprint because records intern + // absolute paths: a record built at /a/repo answers Knows() with /a/repo + // paths only. Without the root, a second checkout sharing the cache + // directory gets byte-identical fingerprints, hits records whose every + // path misses, and silently narrows to nothing. f := &Fingerprinter{ - graph: g, - byPkg: make(map[string][]string), - toolchain: cache.BuildIdentity() + "|" + runtime.Version() + "|" + strings.Join(sorted, ","), + graph: g, + byPkg: make(map[string][]string), + toolchain: cache.BuildIdentity() + "|" + runtime.Version() + "|" + + g.Root + "|" + strings.Join(sorted, ","), } paths := make([]string, 0, len(treeDigests)) @@ -44,7 +50,11 @@ func NewFingerprinter(g *graph.Graph, treeDigests map[string]string, tags []stri entry := path + " " + treeDigests[path] abs := filepath.Join(g.Root, filepath.FromSlash(path)) - if isModuleFile(path) { + // A module file under testdata is a fixture the build never reads; it + // attributes to its owning package below like any other fixture file. + // Classified as unowned it was hashed into every package's + // fingerprint, so editing one fixture invalidated the whole index. + if isModuleFile(path) && !underTestdataPath(path) { f.unowned = append(f.unowned, entry) continue @@ -66,6 +76,12 @@ func isRelevantPath(path string) bool { strings.Contains(path, "/testdata/") } +// underTestdataPath reports whether a slash-relative tree path has a testdata +// element, which the Go build system never looks inside. +func underTestdataPath(path string) bool { + return strings.HasPrefix(path, "testdata/") || strings.Contains(path, "/testdata/") +} + func isModuleFile(path string) bool { switch filepath.Base(path) { case "go.mod", "go.sum", "go.work", "go.work.sum": From 813c44fe4d0c62bc6b8ea04c988910dab7ff788a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 04:33:56 +0000 Subject: [PATCH 4/9] fix(watch): close three ways a cycle could lie MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Narrowing to TestMain ran zero tests and printed a green cycle: it passes the Test-prefix check but -test.run never selects it. It now declines refinement like Benchmark and Example do. A crashed run — panic, -test.timeout dump, os.Exit — credited passes to every requested test the crash prevented from running, fabricating the exact conflicting evidence the flake journal exists to detect. Passes are now credited only when the failing binary printed its FAIL trailer, the marker that the whole list executed. A staging binary left by a killed build could be promoted as fresh when a later build of a now-testless package produced no output. The staging path is cleared before every build. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01211tSDyqM7KnfziwEjPzDF --- internal/watch/binaries.go | 9 +++++++ internal/watch/cycle.go | 6 +++++ internal/watch/flake_test.go | 19 +++++++++++--- internal/watch/loop.go | 26 ++++++++++++++++++ internal/watch/watch_test.go | 51 ++++++++++++++++++++++++++++++++++++ 5 files changed, 108 insertions(+), 3 deletions(-) diff --git a/internal/watch/binaries.go b/internal/watch/binaries.go index aa2fd91..a453012 100644 --- a/internal/watch/binaries.go +++ b/internal/watch/binaries.go @@ -77,6 +77,15 @@ func (c *BinaryCache) Ensure(ctx context.Context, req BuildRequest) (string, boo output := filepath.Join(c.dir, binaryName(req.ImportPath)) staging := output + ".tmp" + // A build killed mid-write leaves a truncated staging file behind. The next + // build usually overwrites it, but a package that has since lost its test + // files produces no output at all — and the no-output check below would + // then find the stale leftover and promote it as fresh. Clear the slate + // before building so whatever sits at the staging path afterwards was put + // there by this build. + if err := os.Remove(staging); err != nil && !os.IsNotExist(err) { + return "", false, fmt.Errorf("clear stale staging binary for %s: %w", req.ImportPath, err) + } args := []string{"test", "-c", "-o", staging} if req.BuildParallelism > 0 { diff --git a/internal/watch/cycle.go b/internal/watch/cycle.go index d11dbeb..65ab755 100644 --- a/internal/watch/cycle.go +++ b/internal/watch/cycle.go @@ -262,6 +262,12 @@ func (p *Planner) refineTestEdit( } func runnableByTestRun(name string) bool { + // TestMain passes the prefix check but is the harness entry point, not a + // test: -test.run never selects it, so narrowing to it runs zero tests and + // prints a green cycle for an edit to the code that sets up all of them. + if name == "TestMain" { + return false + } for _, prefix := range [...]string{"Test", "Fuzz"} { if rest, ok := strings.CutPrefix(name, prefix); ok { if rest == "" || rest[0] < 'a' || rest[0] > 'z' { diff --git a/internal/watch/flake_test.go b/internal/watch/flake_test.go index 46646cf..333462c 100644 --- a/internal/watch/flake_test.go +++ b/internal/watch/flake_test.go @@ -21,7 +21,17 @@ func TestCycleObservationsAttributeEveryKnownVerdict(t *testing.T) { ImportPath: "example.com/m/red", Tests: []string{"TestBroken", "TestFine"}, Passed: false, - Output: []byte("--- FAIL: TestBroken (0.00s)\n"), + Output: []byte("--- FAIL: TestBroken (0.00s)\nFAIL\n"), + Fingerprint: fp, + }, + { + // A crashed run: the panic aborted the binary before TestNever ran, + // so no FAIL trailer was printed and no pass may be credited to it. + ImportPath: "example.com/m/crashed", + Tests: []string{"TestPanics", "TestNever"}, + Passed: false, + Output: []byte("--- FAIL: TestPanics (0.00s)\npanic: boom\n\n" + + "goroutine 1 [running]:\nmain.main()\n"), Fingerprint: fp, }, { @@ -44,11 +54,14 @@ func TestCycleObservationsAttributeEveryKnownVerdict(t *testing.T) { assert.Equal(t, "watch", o.Source) } - require.Len(t, observations, 3) + require.Len(t, observations, 4) assert.Equal(t, flake.VerdictFail, byKey["example.com/m/red.TestBroken"]) assert.Equal(t, flake.VerdictPass, byKey["example.com/m/red.TestFine"], "a test that ran beside the failure and did not fail is a pass worth recording") assert.Equal(t, flake.VerdictPass, byKey["example.com/m/green.TestOK"]) + assert.Equal(t, flake.VerdictFail, byKey["example.com/m/crashed.TestPanics"]) + assert.NotContains(t, byKey, "example.com/m/crashed.TestNever", + "a test the crash prevented from running was not observed and must not be journalled") } // TestStickyRerunsProduceFlakeEvidence pins the payoff of recording watch @@ -60,7 +73,7 @@ func TestStickyRerunsProduceFlakeEvidence(t *testing.T) { ImportPath: "example.com/m/p", Tests: []string{"TestJumpy"}, Passed: false, - Output: []byte("--- FAIL: TestJumpy (0.00s)\n"), + Output: []byte("--- FAIL: TestJumpy (0.00s)\nFAIL\n"), Fingerprint: fp, }}) rerunCycle := cycleObservations([]RunResult{{ diff --git a/internal/watch/loop.go b/internal/watch/loop.go index c14bdc8..ac83a1c 100644 --- a/internal/watch/loop.go +++ b/internal/watch/loop.go @@ -229,6 +229,14 @@ func cycleObservations(results []RunResult) []flake.Observation { failedSet[name] = struct{}{} observe(name, flake.VerdictFail) } + // result.Tests is the requested list, not the executed one. A run that + // died mid-list — a panic, a -test.timeout dump, an os.Exit — never + // reached the remaining tests, and crediting them passes would + // fabricate the exact conflicting evidence the journal exists to + // detect. Only a run that printed its FAIL trailer ran the whole list. + if !printedFailTrailer(result.Output) { + continue + } for _, name := range result.Tests { if _, isFailed := failedSet[name]; !isFailed { observe(name, flake.VerdictPass) @@ -585,6 +593,24 @@ func printFailure(out io.Writer, output []byte) { } } +// printedFailTrailer reports whether a failing test binary ran to completion: +// testing.M prints a bare FAIL as its final line only after the whole test +// list executed, while a crashed run ends in a stack trace or whatever the +// dying process last wrote. +func printedFailTrailer(output []byte) bool { + lines := strings.Split(string(output), "\n") + for i := len(lines) - 1; i >= 0; i-- { + line := strings.TrimSpace(lines[i]) + if line == "" { + continue + } + + return line == "FAIL" + } + + return false +} + func failedTestNames(output []byte) []string { seen := make(map[string]struct{}) for line := range strings.SplitSeq(string(output), "\n") { diff --git a/internal/watch/watch_test.go b/internal/watch/watch_test.go index 875a988..f71c482 100644 --- a/internal/watch/watch_test.go +++ b/internal/watch/watch_test.go @@ -163,6 +163,29 @@ func TestEditInsideOneTestRunsOnlyThatTest(t *testing.T) { assert.NotEmpty(t, cycle.Runs[0].Pattern) } +// TestEditInsideTestMainRunsThePackageInFull pins the refinement against the +// one Test-prefixed function -test.run can never select: narrowing to +// ^(TestMain)$ runs zero tests and prints a green cycle for an edit to the +// code that sets up all of them. +func TestEditInsideTestMainRunsThePackageInFull(t *testing.T) { + r := newRepo(t) + withMain := "package alpha\n\nimport (\n\t\"os\"\n\t\"testing\"\n)\n\n" + + "func TestMain(m *testing.M) {\n\tcode := m.Run()\n\tos.Exit(code)\n}\n\n" + + "func TestDouble(t *testing.T) {\n\tif Double(2) != 4 {\n\t\tt.Fatal(\"bad\")\n\t}\n}\n" + r.write("alpha/alpha_test.go", withMain) + r.git("add", ".") + r.git("commit", "-qm", "alpha gains a TestMain") + + r.write("alpha/alpha_test.go", strings.Replace(withMain, "code := m.Run()", "code := m.Run() + 0", 1)) + + cycle, err := planner(r).Plan(t.Context(), []string{r.path("alpha/alpha_test.go")}) + require.NoError(t, err) + + require.Len(t, cycle.Runs, 1) + assert.Empty(t, cycle.Runs[0].Tests, "an edit to TestMain must run the whole package") + assert.Empty(t, cycle.Runs[0].Pattern) +} + // TestEditOutsideTestBodiesRunsThePackageInFull pins the refinement's guard: a // change to a helper or import can affect any test in the package, so the // refinement must decline rather than guess. @@ -235,6 +258,34 @@ func TestBinaryCacheReusesUntilTheClosureChanges(t *testing.T) { assert.False(t, reused, "a changed dependency closure must rebuild") } +// TestBinaryCacheIgnoresStaleStagingLeftovers pins the promotion invariant: a +// truncated .tmp from a killed build must never be renamed into place as if a +// later build produced it. The trap needs a package whose successful build +// emits no binary — one with no test files — so the only thing at the staging +// path is the leftover. +func TestBinaryCacheIgnoresStaleStagingLeftovers(t *testing.T) { + r := newRepo(t) + r.write("quiet/quiet.go", "package quiet\n\nfunc Noop() {}\n") + + c, err := NewBinaryCache() + require.NoError(t, err) + t.Cleanup(c.Close) + + stale := filepath.Join(c.dir, binaryName(watchModule+"/quiet")+".tmp") + require.NoError(t, os.WriteFile(stale, []byte("truncated garbage"), 0o755)) + + path, reused, err := c.Ensure(t.Context(), BuildRequest{ + Root: r.root, + Env: append(os.Environ(), "GOWORK=off"), + ImportPath: watchModule + "/quiet", + Fingerprint: cache.Fingerprint{1}, + }) + require.NoError(t, err) + assert.False(t, reused) + assert.Empty(t, path, "a package with no test files has no binary, stale leftovers included") + assert.NoFileExists(t, filepath.Join(c.dir, binaryName(watchModule+"/quiet"))) +} + func TestWatcherBatchesDebouncedSaves(t *testing.T) { dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "a.go"), []byte("package a\n"), 0o644)) From 1d9abe00af696cff7a13221dad7b78ffb78b1132 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 04:34:11 +0000 Subject: [PATCH 5/9] fix(mutate): stop leaking schemata workdirs and misattributing cgo files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deferred batch cleanup iterated the live variable that the harness path nils, so every default-mode run leaked one temp directory per batch — the overlays plus a compiled test binary per covering test package. The cleanup now iterates its own copy; the strong-suite test doubles as the leak check. syntaxPath indexed GoFiles as a positional fallback for Syntax, two lists that disagree in length and order for any cgo package, splicing mutations into whatever file sat at that index. The path now comes from the parsed tree's own position information, and cgo-generated cache files yield no mutants. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01211tSDyqM7KnfziwEjPzDF --- internal/mutate/execute.go | 7 ++++++- internal/mutate/execute_test.go | 14 ++++++++++++++ internal/mutate/generate.go | 22 +++++++++++++--------- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/internal/mutate/execute.go b/internal/mutate/execute.go index 944523d..8e03137 100644 --- a/internal/mutate/execute.go +++ b/internal/mutate/execute.go @@ -143,8 +143,13 @@ func Execute(ctx context.Context, mutants []Mutant, opts ExecuteOptions) ([]Resu } ready, declined := prepareBatches(batches) + // Cleanup iterates its own copy: `ready` is nil-ed below once the harness + // has judged the batches, and a defer over the live variable then swept + // nothing, leaking one workdir — overlays plus every compiled test binary — + // per batch of every run. + prepared := ready defer func() { - for _, batch := range ready { + for _, batch := range prepared { batch.cleanup() } }() diff --git a/internal/mutate/execute_test.go b/internal/mutate/execute_test.go index 92e720a..d389902 100644 --- a/internal/mutate/execute_test.go +++ b/internal/mutate/execute_test.go @@ -312,6 +312,13 @@ func requireNoBuildFailures(t *testing.T, results []mutate.Result) { } func TestStrongSuiteKillsEveryMutant(t *testing.T) { + // The run doubles as the leak check for schemata workdirs: pointing TMPDIR + // at a fresh directory makes any assay-schemata-* directory that survives + // Execute visible. The harness path used to nil the batch list before the + // deferred cleanup ran, leaking overlays and compiled test binaries. + tmp := t.TempDir() + t.Setenv("TMPDIR", tmp) + c := newCorpus(t) results, score := c.run(t, "strong") @@ -320,6 +327,13 @@ func TestStrongSuiteKillsEveryMutant(t *testing.T) { assert.Zero(t, score.Survived, "a fully asserted suite should leave no survivors") assert.Positive(t, score.Killed) assert.InDelta(t, 100.0, score.MSI(), 0.001) + + entries, err := os.ReadDir(tmp) + require.NoError(t, err) + for _, entry := range entries { + assert.NotContains(t, entry.Name(), "assay-schemata-", + "Execute must remove every schemata workdir it prepared") + } } func TestEquivalentMutantSurvivesAThoroughSuite(t *testing.T) { diff --git a/internal/mutate/generate.go b/internal/mutate/generate.go index 6c9804b..94c9ec2 100644 --- a/internal/mutate/generate.go +++ b/internal/mutate/generate.go @@ -109,8 +109,8 @@ func Generate(ctx context.Context, opts GenerateOptions) ([]Mutant, error) { } var out []Mutant - for i, file := range pkg.Syntax { - path := syntaxPath(pkg, i) + for _, file := range pkg.Syntax { + path := syntaxPath(pkg, file) if path == "" || strings.HasSuffix(path, "_test.go") { continue } @@ -149,15 +149,19 @@ func parseWithComments(fset *token.FileSet, filename string, src []byte) (*ast.F return parser.ParseFile(fset, filename, src, parser.ParseComments|parser.AllErrors) } -func syntaxPath(pkg *packages.Package, i int) string { - if i < len(pkg.CompiledGoFiles) && !strings.Contains(pkg.CompiledGoFiles[i], "go-build") { - return pkg.CompiledGoFiles[i] - } - if i < len(pkg.GoFiles) { - return pkg.GoFiles[i] +// syntaxPath returns the on-disk source behind one parsed file, read from the +// tree's own position information rather than by indexing a parallel file +// list — GoFiles and Syntax disagree in length and order for any cgo package, +// and a positional fallback spliced mutations into whatever file happened to +// sit at that index. A cgo-generated file lives in the build cache and +// describes text that exists nowhere in the tree, so it yields no mutants. +func syntaxPath(pkg *packages.Package, file *ast.File) string { + path := pkg.Fset.Position(file.FileStart).Filename + if path == "" || strings.Contains(path, "go-build") { + return "" } - return "" + return path } func mutantsInFile( From 9339d8a7250be97a53fb588c61fed739f5dd0aa5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 04:34:11 +0000 Subject: [PATCH 6/9] fix(flake): keep the newest appends when compaction ties on timestamps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Observations batched in one write share a clock reading, and the stable sort kept file order among ties — so a group trimmed at the cap kept its oldest evidence and discarded its newest, the opposite of the documented contract. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01211tSDyqM7KnfziwEjPzDF --- internal/flake/flake_test.go | 28 ++++++++++++++++++++++++++++ internal/flake/journal.go | 6 ++++++ 2 files changed, 34 insertions(+) diff --git a/internal/flake/flake_test.go b/internal/flake/flake_test.go index 678fdae..731c576 100644 --- a/internal/flake/flake_test.go +++ b/internal/flake/flake_test.go @@ -1,6 +1,7 @@ package flake import ( + "fmt" "os" "path/filepath" "sync" @@ -175,6 +176,33 @@ func TestCompactionKeepsTheNewestObservations(t *testing.T) { assert.Equal(t, int64(50), minBusy, "the newest observations are the ones kept") } +// TestCompactionKeepsTheNewestAppendsOnTimestampTies pins the tie order: a +// preflight stamps one clock reading across a whole batch, so a group over the +// cap can be all-ties — and compaction must keep the latest appends, not the +// order the file happened to serve them in. +func TestCompactionKeepsTheNewestAppendsOnTimestampTies(t *testing.T) { + j, err := OpenJournal(t.TempDir(), "/root") + require.NoError(t, err) + + var all []Observation + for i := range maxPerTest + 10 { + o := obs("p", "TestTied", "fp", VerdictPass, 7) + o.Detail = fmt.Sprintf("append-%03d", i) + all = append(all, o) + } + require.NoError(t, j.Record(all)) + + require.NoError(t, j.compactLocked()) + + got, err := j.Load() + require.NoError(t, err) + require.Len(t, got, maxPerTest) + for _, o := range got { + assert.NotEqual(t, "append-000", o.Detail, + "the oldest appends are the ones compaction must discard") + } +} + func TestParseVerboseOutputTakesTopLevelVerdictsOnly(t *testing.T) { output := []byte(`=== RUN TestAlpha --- PASS: TestAlpha (0.00s) diff --git a/internal/flake/journal.go b/internal/flake/journal.go index c0569b4..1601d9d 100644 --- a/internal/flake/journal.go +++ b/internal/flake/journal.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path/filepath" + "slices" "sort" "github.com/zeebo/blake3" @@ -162,6 +163,11 @@ func (j *Journal) compactLocked() error { kept := make([]Observation, 0, len(observations)) for _, group := range grouped { + // The group arrives in append order, oldest first, and observations + // batched in one write share a timestamp. Reversing before the stable + // sort makes the newest appends win ties; without it a group trimmed + // at the cap kept its stalest evidence and discarded its newest. + slices.Reverse(group) sort.SliceStable(group, func(i, j int) bool { return group[i].Unix > group[j].Unix }) if len(group) > maxPerTest { group = group[:maxPerTest] From 21f7dc7c4b5a3eae43065c3cfcdf13846be67a9c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 04:34:11 +0000 Subject: [PATCH 7/9] fix(vcs): key churn by root-relative paths, not repo-toplevel ones git prints numstat paths relative to the repository toplevel no matter where it runs, while risk keys its lookups relative to the analysis root. With --root pointing anywhere below the toplevel every lookup missed, every function counted as stable, and the report printed a green verdict produced by a key mismatch rather than by coverage. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01211tSDyqM7KnfziwEjPzDF --- internal/vcs/churn.go | 42 +++++++++++++++++++++++++++++++++++--- internal/vcs/churn_test.go | 15 ++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/internal/vcs/churn.go b/internal/vcs/churn.go index 4257944..79dce45 100644 --- a/internal/vcs/churn.go +++ b/internal/vcs/churn.go @@ -3,6 +3,7 @@ package vcs import ( "context" "fmt" + "path/filepath" "strconv" "strings" "time" @@ -20,9 +21,9 @@ type ChurnStat struct { const churnMarker = "\x01" // Churn reports per-file change activity over the trailing window, keyed by -// slash-separated path relative to the repository root. Only production Go -// files count: churn in tests is not production exposure, and churn in -// non-Go files is not this tool's business. +// slash-separated path relative to root. Only production Go files count: +// churn in tests is not production exposure, and churn in non-Go files is not +// this tool's business. func Churn(ctx context.Context, root string, window time.Duration) (map[string]ChurnStat, error) { since := fmt.Sprintf("--since=%d.seconds.ago", int64(window.Seconds())) out, err := runGit(ctx, root, @@ -31,6 +32,15 @@ func Churn(ctx context.Context, root string, window time.Duration) (map[string]C return nil, fmt.Errorf("read churn: %w", err) } + // git prints numstat paths relative to the repository toplevel no matter + // where it runs, while callers key lookups by root-relative paths. When + // root is a subdirectory of the repo the two disagree for every file, so + // every lookup missed and risk called everything stable. + prefix, err := gitPathPrefix(ctx, root) + if err != nil { + return nil, err + } + churn := make(map[string]ChurnStat) var commitUnix int64 seenThisCommit := make(map[string]struct{}) @@ -52,6 +62,10 @@ func Churn(ctx context.Context, root string, window time.Duration) (map[string]C if !ok || !isProductionGoFile(path) { continue } + path, inRoot := strings.CutPrefix(path, prefix) + if !inRoot { + continue + } stat := churn[path] if _, counted := seenThisCommit[path]; !counted { @@ -68,6 +82,28 @@ func Churn(ctx context.Context, root string, window time.Duration) (map[string]C return churn, nil } +// gitPathPrefix returns root's toplevel-relative prefix, slash-separated and +// "/"-terminated, or "" when root is the repository toplevel itself. Symlinks +// are resolved before comparing because rev-parse resolves them too. +func gitPathPrefix(ctx context.Context, root string) (string, error) { + toplevel, err := RepoRoot(ctx, root) + if err != nil { + return "", err + } + + resolved := root + if r, symErr := filepath.EvalSymlinks(root); symErr == nil { + resolved = r + } + + rel, err := filepath.Rel(toplevel, resolved) + if err != nil || rel == "." || strings.HasPrefix(rel, "..") { + return "", nil + } + + return filepath.ToSlash(rel) + "/", nil +} + // parseNumstat reads one "addeddeletedpath" line. Binary files // report "-" for both counts and still count as a change. func parseNumstat(line string) (added, deleted int, path string, ok bool) { diff --git a/internal/vcs/churn_test.go b/internal/vcs/churn_test.go index 4fd497d..05faf58 100644 --- a/internal/vcs/churn_test.go +++ b/internal/vcs/churn_test.go @@ -79,6 +79,21 @@ func TestChurnCountsProductionGoFilesOnly(t *testing.T) { assert.NotContains(t, churn, "README.md") } +// TestChurnKeysPathsRelativeToTheGivenRoot pins the contract the risk report +// depends on: git prints numstat paths relative to the repository toplevel, +// but callers analyzing a subdirectory key their lookups by root-relative +// paths. A mismatch made every lookup miss and reported all churn as zero. +func TestChurnKeysPathsRelativeToTheGivenRoot(t *testing.T) { + root := churnRepo(t) + + churn, err := vcs.Churn(t.Context(), filepath.Join(root, "pkg"), 30*24*time.Hour) + require.NoError(t, err) + + assert.Equal(t, 3, churn["hot.go"].Commits, + "paths must be relative to the analysis root, not the repo toplevel") + assert.NotContains(t, churn, "pkg/hot.go") +} + // TestChurnHonoursTheWindow pins the time bound: commits older than the window // contribute nothing, and an empty history is a result, not an error. func TestChurnHonoursTheWindow(t *testing.T) { From 68ac4e838f50c762f4b8032017ddea35d5687e1b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 04:34:11 +0000 Subject: [PATCH 8/9] fix(cli): decide color per stream, and tell the truth about --jobs Color was decided from stdout's TTY-ness while run, verify and the progress line write to stderr: 'assay run 2>log' wrote escapes into the log, and 'assay run >results' stripped color from a summary still heading to a terminal. Stderr-bound output now consults stderr. The --jobs help claimed a GOMAXPROCS default; the collector has defaulted to half of GOMAXPROCS since the inner build parallelism split was introduced. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01211tSDyqM7KnfziwEjPzDF --- internal/cli/index.go | 5 +++-- internal/cli/mutate.go | 2 +- internal/cli/root.go | 18 +++++++++++++++++- internal/cli/run.go | 2 +- internal/cli/verify.go | 2 +- 5 files changed, 23 insertions(+), 6 deletions(-) diff --git a/internal/cli/index.go b/internal/cli/index.go index d2d06fa..8d74351 100644 --- a/internal/cli/index.go +++ b/internal/cli/index.go @@ -96,7 +96,7 @@ func newIndexCommand(opts *options) *cobra.Command { Jobs: jobs, Timeout: timeout, Packages: targets, - Progress: newProgress(cmd.ErrOrStderr(), quiet, opts.painter()), + Progress: newProgress(cmd.ErrOrStderr(), quiet, opts.errPainter()), Legacy: legacy, }) if err != nil { @@ -108,7 +108,8 @@ func newIndexCommand(opts *options) *cobra.Command { }, } - cmd.Flags().IntVar(&jobs, "jobs", 0, "packages to index concurrently (default: GOMAXPROCS)") + cmd.Flags().IntVar(&jobs, "jobs", 0, + "packages to index concurrently (default: half of GOMAXPROCS; each worker builds with its share of the cores)") cmd.Flags().DurationVar(&timeout, "timeout", 0, "per-test timeout (default: 60s)") cmd.Flags().StringSliceVar(&packages, "packages", nil, "limit indexing to these import paths") cmd.Flags().BoolVar(&quiet, "quiet", false, "suppress progress output") diff --git a/internal/cli/mutate.go b/internal/cli/mutate.go index 1566eb6..f5b4b24 100644 --- a/internal/cli/mutate.go +++ b/internal/cli/mutate.go @@ -146,7 +146,7 @@ func runMutate(cmd *cobra.Command, opts *options, flags *mutateFlags) error { mutants = mutate.Exclude(mutants, preflight.Failing) } - execOpts.Progress = newProgress(cmd.ErrOrStderr(), flags.quiet, opts.painter()) + execOpts.Progress = newProgress(cmd.ErrOrStderr(), flags.quiet, opts.errPainter()) results, err := mutate.Execute(ctx, mutants, execOpts) if err != nil { diff --git a/internal/cli/root.go b/internal/cli/root.go index 8fea0a4..5093074 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -72,14 +72,30 @@ type options struct { noColor bool } +func (o *options) colorEnabled() bool { + return !o.noColor && os.Getenv("NO_COLOR") == "" +} + +// useColor answers for stdout-bound output and useColorErr for stderr-bound: +// the two can disagree under redirection — `assay run 2>log` must not write +// escapes into the log because stdout is still a terminal, and `assay run +// >results` must not strip color from a summary still heading to one. func (o *options) useColor() bool { - return !o.noColor && os.Getenv("NO_COLOR") == "" && isTerminal(os.Stdout) + return o.colorEnabled() && isTerminal(os.Stdout) +} + +func (o *options) useColorErr() bool { + return o.colorEnabled() && isTerminal(os.Stderr) } func (o *options) painter() ui.Painter { return ui.New(o.useColor()) } +func (o *options) errPainter() ui.Painter { + return ui.New(o.useColorErr()) +} + func NewRootCommand() *cobra.Command { opts := &options{} diff --git a/internal/cli/run.go b/internal/cli/run.go index f34097d..977c0a7 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -27,7 +27,7 @@ func newRunCommand(opts *options) *cobra.Command { return err } - printer := report.NewPrinter(cmd.ErrOrStderr(), opts.useColor()) + printer := report.NewPrinter(cmd.ErrOrStderr(), opts.useColorErr()) if err := printer.Summary(summary, opts.verbose); err != nil { return err } diff --git a/internal/cli/verify.go b/internal/cli/verify.go index 05d24f0..1b46334 100644 --- a/internal/cli/verify.go +++ b/internal/cli/verify.go @@ -31,7 +31,7 @@ func newVerifyCommand(opts *options) *cobra.Command { return err } - printer := report.NewPrinter(cmd.ErrOrStderr(), opts.useColor()) + printer := report.NewPrinter(cmd.ErrOrStderr(), opts.useColorErr()) if err := printer.Summary(summary, opts.verbose); err != nil { return err } From 090e831b4e4d7b6cdcd0f9c91d1e3e13389ef303 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 8 Aug 2026 04:34:39 +0000 Subject: [PATCH 9/9] docs: record the in-process covdata decoding measurement Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01211tSDyqM7KnfziwEjPzDF --- BENCHMARKS.md | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/BENCHMARKS.md b/BENCHMARKS.md index b4a91d5..42c7538 100644 --- a/BENCHMARKS.md +++ b/BENCHMARKS.md @@ -320,11 +320,30 @@ the win scales with init cost times test count. Durations also become honest in this mode: they are measured inside the harness around `m.Run`, not around a process that spends most of its life initialising. -The follow-up that would turn the wash into a win everywhere is parsing the -binary covcounters format directly instead of shelling out to covdata per -window. It is deliberately not attempted here: the format is internal to the Go -toolchain and version-sensitive, and a maintenance trap should be walked into -deliberately or not at all. +The follow-up that turns the wash into a win everywhere is parsing the binary +covmeta/covcounters formats directly instead of shelling out to covdata per +window — and it is now built, with the maintenance trap fenced rather than +walked into blind. Both formats are version-stamped and unchanged since Go +1.20; the decoder refuses anything it does not recognise — future version, +foreign magic, meta-hash mismatch, truncation — and any refusal falls back to +the covdata subprocess path wholesale, so an incompatible future toolchain +costs time, never correctness. A parity test drives the real toolchain end to +end and requires the decoded blocks to equal the textfmt path's exactly. + +What the subprocess path paid per window was one spawn plus a re-read of the +package's whole-closure meta plus a text profile dominated by zero-count +lines the parser then discarded — a cost that scales with tests × closure +size, which is exactly the product that buries large workspaces. Measured on a +synthetic workspace of 90 packages where each of 30 test packages closes over +61 instrumented packages (600 tests, warm build caches, 4 cores): + +| Path | Wall | user+sys CPU | +|---|---|---| +| covdata subprocess per window | 13.1s | 41.3s | +| in-process decoding | **8.5s** | **25.7s** | + +At a 201-package closure the same shape gives 8.7s against 5.7s. The remaining +time is compile, link and test execution; the per-test conversion tax is gone. **The full 353-package index was not measured here, and the reason is worth recording.** The first attempt drove load average to 30 on a 4-core box and was