docs(engine): state and pin the concurrent-use guarantee

compilers/compilers.go

@@ -60,6 +60,11 @@ type OptionSet struct {
 // returned as ir.Diagnostic values and the error return is reserved for
 // I/O-level and programmer errors.
 //
+// Purity is also the concurrency contract. One Compiler value must accept
+// overlapping Compile calls, which is what lets a caller share a single engine
+// across goroutines; a Compiler that memoizes into package state would break
+// that caller's guarantee without changing this signature.
+//
 // Detect and DecodeOptions are what keep the layers above format-agnostic: a
 // compiler says what its own input looks like and what its own options are
 // called, so registering one is the whole of adding a format. Both are required
@@ -102,6 +107,11 @@ type Compiler interface {
 // Registry maps source formats to compilers. It is a plain instance — there is
 // no package-level default and no init()-time self-registration; the engine
 // composes its registry explicitly. The zero value is a usable empty registry.
+//
+// Concurrent Lookup is safe once registration is complete. Register is not: it
+// writes an unsynchronized map, so every Register must happen before the first
+// concurrent Lookup. Compose a Registry fully before publishing it, the way
+// engine.NewWith registers into a fresh one and only then wraps it.
 type Registry struct {
 	byFormat map[SourceFormat]Compiler
 	ordered  []Compiler

engine/concurrency_test.go

@@ -0,0 +1,151 @@
+package engine_test
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"path/filepath"
+	"sync"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/require"
+
+	"github.com/dexpace/morphic/engine"
+)
+
+// concurrencyCorpus is the capability corpus the compiler's own conformance
+// suite drives. Reusing it here means the concurrent run covers every construct
+// the compiler can lower rather than one hand-picked spec, so shared state
+// hiding in a single lowering still gets exercised.
+const concurrencyCorpus = "../testdata/conformance/openapi"
+
+// concurrentWorkers is the bounded fan-out: enough goroutines to overlap on any
+// machine the suite runs on, few enough that a whole-corpus sweep per worker
+// stays well inside the gate's per-package timeout even under -race.
+const concurrentWorkers = 8
+
+// TestEngine_ConcurrentRunSharesOneEngine drives the whole conformance corpus
+// through a single *engine.Engine from several goroutines at once and requires
+// every document to be byte-identical to the one an unshared Engine produced.
+// One Engine, built outside the goroutines, is the whole point: a test that
+// constructs an Engine per goroutine shares nothing and proves nothing.
+//
+// Two properties are pinned, and only the first is the detector's. A data race
+// is not the only way concurrency corrupts output — a cache keyed on the wrong
+// thing can be perfectly synchronized and still hand one caller another's
+// answer — so the documents are compared, not merely produced, and the baseline
+// is built with a *fresh* Engine per spec: a baseline drawn from the shared
+// Engine would carry the same corruption and cancel it out. Matching it also
+// re-checks determinism (CLAUDE.md invariant 7). Run under -race for the first
+// property; the comparison holds either way.
+func TestEngine_ConcurrentRunSharesOneEngine(t *testing.T) {
+	t.Parallel()
+
+	specs := corpusSpecs(t)
+	ctx := t.Context()
+
+	// The baseline: each spec compiled sequentially through an Engine of its own,
+	// so nothing at all is shared and nothing can carry over between compiles.
+	want := make([]string, len(specs))
+	for i, spec := range specs {
+		fresh, err := engine.New()
+		require.NoError(t, err)
+		doc, err := compileJSON(ctx, fresh, spec)
+		require.NoError(t, err)
+		require.NotEmpty(t, doc, "baseline compile of %s produced nothing", spec)
+		want[i] = doc
+	}
+
+	// A comparison against a degenerate baseline would pass for the wrong reason,
+	// so require the corpus to have produced a distinct document per spec.
+	distinct := make(map[string]struct{}, len(want))
+	for _, doc := range want {
+		distinct[doc] = struct{}{}
+	}
+	require.Equal(t, len(specs), len(distinct), "baseline documents must all differ")
+
+	eng, err := engine.New()
+	require.NoError(t, err)
+
+	got := make([]workerRun, concurrentWorkers)
+	var wg sync.WaitGroup
+	for w := range concurrentWorkers {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			got[w] = runCorpus(ctx, eng, specs, w)
+		}()
+	}
+	wg.Wait()
+
+	for w, run := range got {
+		require.NoError(t, run.err, "worker %d", w)
+		// Bounds for the indexing below. It cannot fail while runCorpus returns
+		// either an error or a full-length slice, and it is here so that a later
+		// change to that shape is a failed assertion rather than a panic.
+		require.Len(t, run.docs, len(specs), "worker %d returned %d docs", w, len(run.docs))
+		for i, spec := range specs {
+			// A worker that stopped early leaves zero values behind, which the
+			// diff below would report as a document compiled differently. Say
+			// what actually happened instead.
+			if run.docs[i] == "" {
+				t.Errorf("worker %d never compiled %s", w, filepath.Base(spec))
+				continue
+			}
+			if diff := cmp.Diff(want[i], run.docs[i]); diff != "" {
+				t.Errorf("worker %d compiled %s differently (-unshared +shared):\n%s",
+					w, filepath.Base(spec), diff)
+			}
+		}
+	}
+}
+
+// workerRun is one goroutine's sweep of the corpus: the marshalled document per
+// spec, in the corpus order, or the first failure it hit.
+type workerRun struct {
+	docs []string
+	err  error
+}
+
+// runCorpus compiles every spec through eng, starting at offset so that workers
+// are on different specs at any instant and overlapping calls exercise
+// different lowerings rather than all crowding one.
+func runCorpus(ctx context.Context, eng *engine.Engine, specs []string, offset int) workerRun {
+	docs := make([]string, len(specs))
+	for i := range specs {
+		j := (offset + i) % len(specs)
+		doc, err := compileJSON(ctx, eng, specs[j])
+		if err != nil {
+			return workerRun{err: err}
+		}
+		docs[j] = doc
+	}
+	return workerRun{docs: docs}
+}
+
+// compileJSON runs one spec through eng and marshals the document. It returns
+// errors rather than calling t.Fatal because it runs off the test goroutine,
+// where require's FailNow is not legal.
+func compileJSON(ctx context.Context, eng *engine.Engine, path string) (string, error) {
+	res, err := eng.Run(ctx, path, engine.RunOptions{})
+	if err != nil {
+		return "", fmt.Errorf("run %s: %w", path, err)
+	}
+	if res.Document == nil {
+		return "", fmt.Errorf("run %s: nil document", path)
+	}
+	raw, err := json.Marshal(res.Document)
+	if err != nil {
+		return "", fmt.Errorf("marshal %s: %w", path, err)
+	}
+	return string(raw), nil
+}
+
+func corpusSpecs(t *testing.T) []string {
+	t.Helper()
+	specs, err := filepath.Glob(filepath.Join(concurrencyCorpus, "*.yaml"))
+	require.NoError(t, err)
+	require.NotEmpty(t, specs, "no specs under %s", concurrencyCorpus)
+	return specs
+}

engine/engine.go

@@ -47,6 +47,16 @@ type Result struct {
 
 // Engine orchestrates the detect → compiler → passes pipeline over a registry
 // of compilers.
+//
+// An Engine is safe for concurrent use by multiple goroutines, and concurrent
+// runs over one spec yield identical documents rather than merely uncorrupted
+// ones. NewWith finishes writing the registry before the Engine exists and Run
+// only reads it; Run keeps nothing between calls. The rest of the guarantee is
+// compilers.Compiler's purity requirement — a compiler holding package-level
+// mutable state would break it, which is why that requirement is part of the
+// contract and not advice. TestEngine_ConcurrentRunSharesOneEngine pins both
+// properties: the second on every run, the first only under -race, which the
+// gate's coverage step passes.
 type Engine struct {
 	registry *compilers.Registry
 }
@@ -99,6 +109,10 @@ func NewWith(fronts ...compilers.Compiler) (*Engine, error) {
 // Result, a source no compiler can lower included. A caller that treats a Go
 // error as "the pipeline was invoked wrongly" therefore stays correct, which is
 // what lets the CLI keep its usage exit code for actual misuse.
+//
+// Calls on one Engine may overlap. They share only the read-only registry, and
+// each call owns the document it returns — which is what makes appending the
+// validate pass's diagnostics into that document safe.
 func (e *Engine) Run(ctx context.Context, specPath string, opts RunOptions) (*Result, error) {
 	// Ahead of the read, because an engine that never went through a constructor
 	// is the caller's mistake whatever the path turns out to say, and reporting

pass/validate.go

@@ -18,6 +18,10 @@ const maxGroupDepth = 128
 // the diagnostics it finds, most-structural first. It is pure: it never mutates
 // doc and holds no package-level state. An empty result means the document is
 // internally consistent for every rule this pass enforces.
+//
+// Purity makes concurrent calls safe, including several over one document — but
+// Validate takes no lock, so nothing may be mutating that document meanwhile.
+// engine.Run satisfies this by validating a document no other run can reach.
 func Validate(doc *ir.Document) []ir.Diagnostic {
 	if doc == nil {
 		return nil