feat(core): milestone 4 phase 3 - test suites and verification

.agents/skills/engineering-workflow/references/design-decisions.md

@@ -31,9 +31,14 @@ Four places, each with a different job — don't blur them together:
   shapes, not shipped ones) — when a doc is describing a target rather than
   current reality, keep that distinction explicit rather than letting a
   doc quietly drift from "intent" to "fact" without the code to back it.
-- **`docs/adr/*.md`** — the actual decision record: numbered, permanent,
-  immutable once accepted. This is where "what was decided and why, and
-  what was rejected" actually lives — see **Writing an ADR** below and
+- **`docs/adr/*.md`** — the actual decision record: numbered, permanent.
+  Its original Decision/Rationale/Consequences text is immutable once
+  accepted — never rewritten to look like it was always this way — but a
+  correction or extension discovered later gets recorded as a dated
+  Amendment appended to the same ADR, rather than either editing the
+  original text or opening a new ADR for something that isn't actually a
+  reversal. This is where "what was decided and why, and what was
+  rejected" actually lives — see **Writing an ADR** below and
   `docs/adr/README.md` for the numbering/status mechanics.
   `docs/adr/0000-adr-template.md` is the template to copy for every new
   ADR. An ADR is not a plan — it doesn't get a task checklist or a
@@ -140,20 +145,38 @@ a short one, not a skipped one:
 5. Update the relevant `docs/*.md` topic doc to reflect the resulting
    current (or intended, per the pre-implementation note above) state,
    linking back to the ADR rather than re-explaining the reasoning.
-6. If this ADR changes direction from an earlier one, set the earlier
-   ADR's `Status` to `Superseded by ADR-XXXX` — don't edit its Decision/
-   Rationale/Consequences, an accepted ADR is a historical record, not a
-   living doc.
+6. If this ADR changes direction from an earlier one — its core Decision
+   Outcome is being replaced, not just corrected or extended — set the
+   earlier ADR's `Status` to `Superseded by ADR-XXXX` — don't edit its
+   Decision/Rationale/Consequences in place for a change this size; write
+   the new decision as its own ADR instead.
+7. If implementation or review surfaces a correction, clarification, or
+   extension to an already-`Accepted` ADR's decision — not a reversal of
+   it — record it as a dated `## Amendment N (YYYY-MM-DD): <title>` section
+   appended to that same ADR, rather than opening a new one. This is the
+   established pattern in this repo (see ADR-0022's Amendments 1-6 for
+   real examples: a caching-interaction clarification, a reverted feature
+   attempt, a gap a later PR review caught, a test-infrastructure decision
+   changed after repeated CI failures) — an Accepted ADR's *original*
+   Decision/Rationale/Consequences text stays exactly as written (that's
+   the "immutable historical record" property that matters: what was
+   decided, and why, at the time, is never rewritten to look like it was
+   always this way), but the ADR as a whole is allowed to grow dated
+   Amendment sections that record what was learned afterward. Reach for
+   step 6 (a full new ADR, superseding this one) only when the core
+   decision itself is being reversed or replaced — a genuinely different
+   answer to the same question, not a correction to how the original
+   answer was implemented or verified.
 
 Keep the ADR itself to decision content — Status, Decision, Rationale (or
 Context/Decision Drivers/Considered Options/Decision Outcome for a deep
-dive that needs the fuller shape), Consequences. Resist the urge to also
-list every file that'll change or write a task checklist inside it; once
-an ADR is `Accepted`, that kind of content goes stale the moment
-implementation starts diverging from the plan in some small way, and
-you're stuck either leaving the ADR wrong or editing a record that's
-supposed to be immutable. That content belongs in a plan instead — see
-below.
+dive that needs the fuller shape), Consequences, and any Amendments per
+step 7 above. Resist the urge to also list every file that'll change or
+write a task checklist inside it; once an ADR is `Accepted`, that kind of
+content goes stale the moment implementation starts diverging from the
+plan in some small way, and you're stuck either leaving the ADR wrong or
+editing content that's supposed to stay as originally written. That
+content belongs in a plan instead — see below.
 
 A design dive — light or deep — that never produces an ADR hasn't actually
 finished. The brainstorm (for a deep dive) or the quick judgment call (for

.gitignore

@@ -435,6 +435,10 @@ FodyWeavers.xsd
 
 .idea
 
+# Local NuGet feed populated by Compono.XunitV3.SampleTests' pre-restore pack step
+# (test/Compono.XunitV3.SampleTests/nuget.config) - regenerated on every restore, never committed.
+.local-nuget-feed/
+
 # macOS
 .DS_Store
 .DS_Store?

AGENTS.md

@@ -142,9 +142,13 @@ list). The load-bearing ones:
 ## The non-negotiables
 
 - Architecture is ADR-driven: every non-trivial design decision gets an
-  ADR in `docs/adr/` (immutable once `Accepted`) before code is written
-  against it. Don't silently evolve architecture in code — see
-  `references/design-decisions.md`.
+  ADR in `docs/adr/` before code is written against it. An ADR's original
+  Decision/Rationale/Consequences text is immutable once `Accepted` —
+  never rewritten in place — but a correction or extension found later is
+  recorded as a dated Amendment appended to that same ADR (see ADR-0022's
+  Amendments for real examples); reserve superseding with a whole new ADR
+  for an actual reversal of the core decision. Don't silently evolve
+  architecture in code — see `references/design-decisions.md`.
 - Keep PRs scoped to one decision or one feature — no bundling an
   unrelated fix or refactor. If a plan is phased, that's one PR per
   phase, and the prior phase's status/PR-merge state should be current

Directory.Packages.props

@@ -16,6 +16,10 @@
     <!-- Compono.XunitV3's own dependency, matching xunit.v3.mtp-v2's own resolved version - the
          Xunit.v3.DataAttribute/ITheoryDataRow extension surface, per ADR-0022. -->
     <PackageVersion Include="xunit.v3.extensibility.core" Version="3.2.2" />
+    <!-- Compono.XunitV3.SampleTests' own dependency - consumed as a real NuGet package from a local
+         feed populated by dotnet pack (PLAN-0004 Phase 3), never a ProjectReference. The version here
+         must match the -p:Version passed to that project's PackToLocalFeed pre-restore target. -->
+    <PackageVersion Include="Compono.XunitV3" Version="1.0.0" />
     <!-- Mocking. No AutoFixture here, deliberately - Compono is a from-scratch
          AutoFixture replacement (docs/manifesto.md), so dogfooding AutoFixture
          inside Compono's own test suite would be a direct contradiction of the

docs/adr/0022-compono-xunit-package-design.md

@@ -1038,6 +1038,153 @@ its own design question, deferred - not designed here, and not assumed
 necessary until a concrete need materializes (this milestone's own
 non-goals already exclude speculative future-proofing).
 
+## Amendment 5 (2026-07-31): a genuine composition failure's own `Message` must carry the seed too, not only `Diagnostic`
+
+PR #26 review (Codex, on PLAN-0004 Phase 3) caught that `test/Compono
+.XunitV3.SampleTests`' deliberately-failing theory used `[Compose(Seed =
+-1)]` - a negative, rejected seed - which exercises `Compono.XunitV3`'s own
+seed-validation failure (a pre-composition check that always rejects `-1`
+regardless of what it's paired with), not a genuine composition failure.
+That meant `RealRunnerTests` never actually proved the milestone's own
+Goal statement - "a composition failure's message contains a seed that,
+pasted into `[Compose(Seed = ...)]`, reproduces the same failure" - for
+the case that statement is actually about: a real, unsatisfied composition
+request.
+
+Investigating why a genuine case wasn't used originally (Phase 3's own
+`SeedReportingTests`, added under time pressure to close out that task
+list item) surfaced the real gap: a *pipeline*-propagated
+`CompositionException` (an ordinary composition failure - nothing could
+satisfy the requested type) has never carried its seed in
+`Exception.Message` itself, only in `Diagnostic`/`Diagnostic.ToString()`
+(the `Console.WriteLine(exception.Diagnostic)` convention
+`docs/architecture.md` documents, from Milestone 2/3, per
+[ADR-0010](0010-composition-request-pipeline-and-diagnostics-tracing.md)).
+A real xUnit v3/MTP test-runner failure display shows `Exception.Message`,
+not `Diagnostic.ToString()` - so the pasteable-seed promise was invisible
+in real console output for a genuine composition failure the entire time,
+not just in this one sample theory.
+
+**Decision:** `ComposeAttribute.GetData` now catches a `CompositionException`
+propagating from a composition call (`ResolveInvoker`/`ResolveSharedInvoker`/
+`ShareExplicitInvoker`, via a new private `InvokeWithSeedOnFailure` helper
+wrapping each call site) and rethrows it via a new public `Compono` core
+method, `CompositionException.WithSeedInMessage(original, seed)` - rewriting
+`Message` to `$"{original.Message}\n\nSeed: {seed}"` while preserving
+`Diagnostic` completely unchanged (so `Diagnostic.ToString()` still renders
+correctly, without a duplicated `"Seed:"` line) and the original exception
+as `InnerException`. **Originally shipped as an `internal` seam with a new
+`InternalsVisibleTo("Compono.XunitV3")` grant on `Compono.csproj` - reverted
+after PR #26 review's fourth round; see the amendment below.**
+
+**Why a new core method rather than a workaround entirely inside
+`Compono.XunitV3`:** `CompositionException`'s existing constructors always
+derive `Message` directly from `Diagnostic.Message` (or accept a plain
+string with no `Diagnostic`/`InnerException` at all) - there's no existing
+public shape that lets a caller supply a custom `Message` string alongside
+an existing `Diagnostic` and `InnerException` together.
+`Compono.XunitV3` reconstructing its own `CompositionDiagnostic` copy with
+a seed-appended `Message` field was considered and rejected: `Diagnostic
+.ToString()`'s own format independently appends `"\n\nSeed: {Seed}"`, so a
+diagnostic whose `Message` field *also* had the seed appended would render
+that line twice - a visible regression for the documented `Console
+.WriteLine(exception.Diagnostic)` path. `WithSeedInMessage` avoids that
+entirely by leaving `Diagnostic` untouched and only rewriting the outer
+exception's own `Message`.
+
+**Consequence for `test/Compono.XunitV3.SampleTests`:** `Failing
+CompositionTests` now composes `GatewayConsumer` (a concrete class whose
+constructor takes `IUnregisteredGateway`, an interface satisfied by
+nothing), with an explicit `Seed = 24601` so an assertion against its
+output stays deterministic rather than needing to parse an auto-generated
+seed out of console output. `IUnregisteredGateway` itself is deliberately
+never used as the `[Compose]`-attributed parameter's own root type - that
+would hit the CMP0003 diagnostic this plan's own Open Items section
+already documents as deliberate (an abstract/interface/delegate root
+always fails at compile time, regardless of whether a registration might
+satisfy it at runtime) - wrapping it as a *nested* constructor parameter
+uses the more lenient member-level check instead, so it compiles and fails
+only at runtime, which is the failure shape this fixture actually needs.
+
+**Amendment to this amendment (PR #26 review, third round): the fix above
+only covered a *diagnosed* `CompositionException`.** `InvokeWithSeedOnFailure`'s
+original catch clause was guarded by `when (exception.Diagnostic is not
+null)`, which let a plain-message `CompositionException` (no `Diagnostic`
+at all) escape completely unmodified - exactly the shape a generated
+`HashSet<T>`/`Dictionary` collection plan's unique-value-exhaustion path
+throws (`Compono.Generators/Templates/CollectionPlan.scriban`:
+`throw new CompositionException($"Could not generate {size} unique
+values...")`, no `Diagnostic` constructed at all). Composing a
+default-sized (3) `HashSet<bool>` - `bool`'s value space has only 2
+members - deterministically hits this path with zero seed anywhere in the
+output. Fixed by broadening `CompositionException.WithSeedInMessage`
+itself to build its rewritten `Message` from `original.Message` (not
+`original.Diagnostic!.Message`) and to preserve `original.Diagnostic`
+as-is, whatever it is - `null` stays `null`, a real diagnostic stays
+unchanged - rather than requiring one. `InvokeWithSeedOnFailure`'s catch
+clause no longer needs the `Diagnostic is not null` guard at all: every
+`CompositionException`, diagnosed or not, now gets the same treatment
+uniformly.
+
+**Amendment to this amendment (PR #26 review, fourth round): the
+`InternalsVisibleTo` grant directly reversed ADR-0021's own accepted,
+public-only integration boundary.** ADR-0021's Pros and Cons already has
+an entry for exactly this option -
+"Reaching the engine — `InternalsVisibleTo("Compono.Xunit")`" - rejected
+specifically because "it directly reverses a deliberate, documented
+policy... adopted specifically to keep a shipped package's internal
+surface from becoming a de facto public contract for 'trusted'
+consumers." Granting `Compono.XunitV3` `InternalsVisibleTo` for
+`WithSeedInMessage` did exactly that: because `Compono` is unsigned, the
+grant authenticates only the simple assembly name, handing any assembly
+that can claim that name unrestricted access to every internal member of
+`Compono` core - not scoped to the one method that needed it.
+
+**Decision:** the grant is removed. `WithSeedInMessage` itself moved from
+`internal` to `public` instead - it already lives inside
+`CompositionException`, so it already has full access to the private
+constructor and `DiagnosingContextIdentity` it needs; the only reason it
+required `InternalsVisibleTo` at all was its own access modifier, not
+anything about where it's implemented. Making the one method public,
+rather than granting blanket internal access, is the minimal-footprint
+fix `docs/public-api.md`'s "keep public APIs minimal" goal actually calls
+for: `Compono.XunitV3` gets exactly the operation it needs and nothing
+else, `CompositionContext` and every other internal type stay exactly as
+inaccessible to `Compono.XunitV3` as ADR-0021 intended, and any other
+consumer building custom composition-failure tooling gets the same
+utility for free. No `InternalsVisibleTo` entry for `Compono.XunitV3`
+exists in `Compono.csproj` after this fix - the only remaining grant is
+`Compono.Tests`, unchanged from before this PR.
+
+## Amendment 6 (2026-07-31): the automated real-runner test is dropped; the sample project stays, unautomated
+
+The `RealRunnerTests` test this amendment's own "Consequence" section
+above referred to - which shelled out `dotnet test` against
+`Compono.XunitV3.SampleTests` on every CI run - is removed. Across four
+consecutive rounds of fixes (a cross-process file lock, a global-cache
+clear, an isolated restore path, and finally a machine-wide named
+`Mutex` fully serializing the nested subprocess), each verified
+extensively via local reproduction of CI's exact concurrency at the time,
+CI still failed on the *next* push in a new way tied specifically to this
+repo's CI runner's process-concurrency characteristics for a
+nested-`dotnet`-invoking test - never once on the actual composition
+behavior the sample project's theories exercise, which passed every
+single local and CI run across the whole sequence.
+
+**Decision:** stop iterating on CI-only concurrency once it was clear the
+thing being verified (that `Compono.XunitV3` composes correctly as a real
+consumed package, through a real xUnit v3 runner) was already
+conclusively proven, repeatedly - keep `Compono.XunitV3.SampleTests` on
+disk exactly as built (its representative theories, its
+`PackToLocalFeed`/`RestorePackagesPath` local-feed infrastructure), not
+referenced by `Compono.slnx` or any other test, available to run by hand
+whenever packaging behavior needs re-checking by a person. This mirrors
+Phase 1's own precedent for this exact milestone - a one-time manual
+`dotnet pack` + local feed + throwaway-consumer verification, not
+permanent CI automation - rather than introducing a new philosophy. See
+[PLAN-0004](../plans/0004-milestone-4-xunit-integration.md)'s Phase 3
+Notes for the full blow-by-blow of all four fix attempts.
+
 ## Links
 
 - [ADR-0021](0021-row-composition-entry-point-for-test-framework-integrations.md) —