[fuad-daoud]

Summary

make coverage ran the whole suite once under -race -covermode=atomic. That run was the slowest step of every pull request, and atomic coverage under -race can hide a data race: #1271 has a planted race that -race reports 6 of 6 times and -race -covermode=atomic reports 0 of 6. This splits the run in two.

• make coverage runs go test ./... -covermode=set without -race. cmd/morphic-covergate reads only whether a block ran, so its verdict is unchanged: 100% of 16,833 statements on this branch.
• make race runs -race alone, on the packages listed in scripts/race-packages.txt: the ones whose tests run morphic code on more than one goroutine (load, irfacts, engine/..., cmd/morphic, barrier, leakcheck, ctxtest). It skips TestLoad_ACancelAtAnyCheckEndsTheLoadCleanly and TestLoad_DoesNextToNothingOnceItRefusesACancel, which test sequential cancellation and still run at full size in make coverage.
• make race proves it reaches: scripts/race-reach.sh runs the race command on a planted race under internal/archtest/testdata/racereach and fails unless it is reported. The plant's racing accesses sit in a non-test file, so both regressions fail the gate: dropping -race, and merging coverage back into the race run (the plant is reported 6 of 6 under -race and 0 of 6 with -covermode=atomic added). ci: run coverage in set mode, and -race only where code is concurrent #1271 records that the hiding depends on layout, so a later toolchain could weaken the coverage half; it can never redden a correct recipe.
• The list cannot drift. TestRaceList_NamesEveryConcurrentPackage fails when a package whose production code takes a lock, an atomic or a go statement, or whose tests start goroutines, is neither listed nor exempt with its reason, and when an entry or exemption is stale. It reads the same file the Makefile does.
• CI's suite job runs coverage and race as separate steps, under a name that says so. Its id stays suite, so its cache key is unchanged.
• Comments, documents and skills that described the combined run now describe this one.

What the race run gives up, deliberately:

• Race checking of tests that run morphic code on one goroutine. Every test still runs, without -race, in make coverage.
• internal/harness and internal/census: tooling whose tests call harness.Check and (*Census).Add from parallel goroutines. A race there goes unreported. The list file says so.
• TestEngine_ConcurrentRunSharesOneEngine passes RunOptions{}, which leaves Verify off. It race-checks the compile and the passes; other listed tests reach Verify.
• CI no longer meets the go/types race in archtest's loads (test(archtest): the recursion pin's load can trip a go/types data race #855).

A scripts/vet-rules.sh fix that #1271 neither needs nor exposes was moved out to #1305.

Closes #1271.

Before and after

flowchart LR
  subgraph before["Before: one combined run"]
    direction TB
    b1["make coverage<br/>go test ./... -race -covermode=atomic"] --> b2["every package under the race detector<br/>284–386s on a one-file change"]
    b1 -. "atomic counters can act as<br/>synchronisation the detector trusts" .-> b3["some races go unreported"]
  end
  subgraph after["After: two runs"]
    direction TB
    a1["make coverage<br/>go test ./... -covermode=set"] --> a2["100% gate, same verdict<br/>77–85s"]
    a3["make race<br/>-race on scripts/race-packages.txt"] --> a4["only packages that run code concurrently<br/>24–95s"]
    a3 --> a5["race-reach.sh: a planted race<br/>must be reported"]
    a6["TestRaceList_NamesEveryConcurrentPackage"] -. "holds the list to the tree" .-> a3
  end
  before ~~~ after

Loading

What it brings: the race detector is no longer silenced by coverage and is proven to report a planted race, and the suite job stops paying for it on packages whose code never runs concurrently. On the fork's measurements, the race run fell from 284–386s (combined) to 24–95s, with coverage alone at 77–85s.

Test plan

• make gate passes on this branch, pinned to 4 cores (GOMAXPROCS=4 taskset -c …), on the tree merged with main at 39afce9. main at 7cf3f95 is merged in since; on that merge the race-list rule passes, main adds no race-detector mention and no production package that takes a lock or starts a goroutine, and CI on this head runs the gate.
• The workflow change ran on Actions in a test pull request on a fork (check: #1271 workflow on CI fuad-daoud/morphic#1): run 41360252466 on an earlier head and 41365106637 on this one. The coverage step ends Coverage gate passed: all 16833 statements covered.. The race step runs exactly the seven listed patterns (11 packages) with the two tests skipped, then the reach check.
• make race's reach: removing -race from the recipe, adding -covermode=atomic to it, or making the plant race-free, each fails scripts/race-reach.sh.
• TestRaceList_* and TestRaceListViolations_* each fail under 12 planted defects: an entry dropped, an unlisted sync import, an unlisted go statement, a stale exemption, the / boundary in matching removed, test-only imports counted, and others. Each exemption's reason was read against every goroutine site in its package.
• TestRun_JudgesAProfile gained a mode: set row, which reddens when set-mode blocks are judged missed.
• Every -race, race detector and covermode mention outside testdata was read and classified: those in listed packages stay, and those in packages that left the race run were reworded.
• After merge: judge timing on a run once main has saved a cache under the new steps. The first pull-request run restores a cache built for the combined run.

[OmarAlJarrah]

issue: the race list leaves out pipeline code that only parallel tests run concurrently, and every place that states the gap names internal/harness and internal/census alone.

concurrentDirs counts a non-test sync import or a goroutine statement, and deliberately not t.Parallel (racelist_test.go#L262-L268). But parallel tests do run morphic code on many goroutines at once, and plain -race finds races through them that the race list misses. On this head merged with main (3188471), 48 directories with parallel tests are off the list, 32 of them pipeline packages under ir/, pass and compilers/:

git grep -l 't\.Parallel()' -- '*_test.go' ':!**/testdata/**' | xargs -n1 dirname | sort -u |
  grep -vxE 'barrier|cmd/morphic|compilers/openapi/internal/load|engine(/.*)?|internal/ctxtest|internal/leakcheck|ir/irfacts' | wc -l

I planted one unsynchronized package-level int increment in each of two branches that only their own package's parallel tests run concurrently:

• pass: inside checkPropIDRefs' loop over dangling prop references (validate_refs.go#L54). Validate's doc says concurrent calls are safe (validate.go#L41), and the engine runs it as a default pass, so this would be a production race whenever two runs validate broken documents at once.
• ir/irverify: the ir/bigval-not-numeric branch of appendBigVal (bigval.go#L76).

var danglingProps int // above checkPropIDRefs

	for _, ref := range dangling {
		danglingProps++

Run, both plants in	Result
go test -race ./pass ./ir/irverify	reported in 3 of 3 runs of each package (1–3 reports in pass, 13–15 in irverify)
this PR's make race command	passes, 0 reports
this PR's make coverage command on both packages	passes, 100%
TestRaceList_NamesEveryConcurrentPackage	passes
main's -race -covermode=atomic	0 reports in 3 of 3 runs of each package

So on these two, head is no worse than main: atomic coverage hid both before, as #1271 describes. But hidden races are what this PR sets out to fix, and outside the seven listed patterns the split leaves them as hidden as before, now because those packages never run under the detector. A plant at a function's entry cannot show this: one at the top of pass.validate was reported 291 times by this PR's make race, because the engine's concurrent tests reach every entry. The plant has to sit on a branch only the unlisted package's own tests take.

Where the gap is stated too narrowly:

• scripts/race-packages.txt#L3: "A package belongs here when its code runs on more than one goroutine". pass and irverify meet that rule. L8-L11 then names two exceptions.
• CLAUDE.md#L314: "Parallel tests alone do not count, so internal/harness and internal/census are off the list". architecture.md#L708 says the same.
• racelist_test.go#L34-L38: an exemption is for a directory where "no morphic code runs on more than one goroutine", and compilers/openapi/e2e is exempt because its compile "runs on that goroutine alone". That package calls t.Parallel() 275 times.
• The body: what the race run gives up is "race checking of tests that run morphic code on one goroutine".

Rules: CLAUDE.md's "Correctness comes first", and "A limitation that is deliberately out of scope must be stated in the code and in the PR body".

Direction, as a draft:

1. Run make race over every package the engine runs (./ir/... ./pass ./compilers/... beside the current list), or over ./..., still skipping the two cancellation tests, and measure it on CI. The list and its archtest rule then shrink to the tooling deliberately left off.
2. If that costs more than the split is worth, keep the list and state the gap as a class everywhere it is stated now: every package whose only concurrency is parallel tests is off the race run, pipeline packages included, so a race on a path only those tests take goes unreported, as it did under atomic coverage.

The first fixes the gap. The second makes it a stated trade.

[OmarAlJarrah]

nit: two counts in the body are stale on this head.

• "100% of 16,833 statements on this branch": the suite job on a5330ca (run 41486584623) prints Coverage gate passed: all 17947 statements covered.
• "runs exactly the seven listed patterns (11 packages)": the same job's race step runs 12 packages (barrier, cmd/morphic, load, six under engine/..., ctxtest, leakcheck, irfacts).

Rule: CLAUDE.md's "Derive counts; never maintain them … If a claim about code must be written down, name the revision it was true at."

Fix: name the run and head each count was read from, or drop the counts. The gate already derives the first one on every run.