Test Generator: follow-ups deferred from #4323's review rounds (generated-tree audit)

Follow-up to **#4323** (*Test Generator: audit the generated tree against what the configuration
would produce*), which closes **#4305**.

#4323 drew eight rounds of automated review. Everything any reviewer marked *"would fix before
merge"* has been actioned on that branch; this issue collects what was consciously deferred so it
is tracked rather than lost in a PR thread. Nothing here blocks #4323, and nothing here is known
to be live — every item is latent, cosmetic, or a hardening step.

Source comments: [5584805486](https://github.com/BrighterCommand/Brighter/pull/4323#issuecomment-5584805486),
[5587572596](https://github.com/BrighterCommand/Brighter/pull/4323#issuecomment-5587572596),
[5609053450](https://github.com/BrighterCommand/Brighter/pull/4323#issuecomment-5609053450),
[5614318515](https://github.com/BrighterCommand/Brighter/pull/4323#issuecomment-5614318515),
[5615001894](https://github.com/BrighterCommand/Brighter/pull/4323#issuecomment-5615001894),
[5616054404](https://github.com/BrighterCommand/Brighter/pull/4323#issuecomment-5616054404),
[5616225556](https://github.com/BrighterCommand/Brighter/pull/4323#issuecomment-5616225556) /
[5616230360](https://github.com/BrighterCommand/Brighter/pull/4323#issuecomment-5616230360),
[5616382748](https://github.com/BrighterCommand/Brighter/pull/4323#issuecomment-5616382748),
[5617735115](https://github.com/BrighterCommand/Brighter/pull/4323#issuecomment-5617735115).

---

## A. Do these before, or as part of, the #4240 / #4297 merge

That merge adds a third suite (`MessagingGateway/Shared`) to `MessagingGatewayGenerator.SuitesFor(...)`.
Two of the items below are latent *only* because there are currently two suites and every planned
path happens to contain a `Generated` segment.

**A1. `IsUnderAGeneratedFolder` and `Collect` disagree about where "under `Generated/`" starts.**
`IsUnderAGeneratedFolder` splits the **absolute** path, while `Collect` starts at `testsRoot` with
`underGeneratedFolder: false` and so only considers segments below it. Two consequences, both
currently invisible because the filter is a no-op today: a checkout under a directory literally
named `Generated` makes the expected-side filter admit everything; and **any future suite that
writes outside `Generated/` enters `Expected` but never `OnDisk`** — a permanent false *missing*
whose message tells the reader to run the thing they just ran. Fix: relativise against `testsRoot`
before segmenting.

**A2. A guard against the fold silently shrinking the expected set.**
Folding the Shared suite into `SuitesFor(...)` incorrectly does **not** break the build or the
generator. It quietly removes files from `Expected`, and the audit goes green while owning less
than it should — the one failure mode the audit cannot self-report. A count assertion, or an
explicit *"this configuration plans the Shared suite"* test, makes that specific mistake visible.

**A3. Make `BaseGenerator.GenerateAsync` non-virtual.**
It is still `protected virtual` with **no override left in the codebase** — removing both overrides
was the point of the refactor. Leaving it virtual re-opens exactly the drift that was closed: an
override can change what is *written* without changing what is *planned*, and the audit would then
be quietly wrong rather than loud. Non-virtual turns "do not do this again" into a compile error.

---

## B. Correctness, all latent

**B1. Config discovery is one level deep; the disk walk is not.** Configurations are read from
direct children of `tests/`, while `GeneratedFilesUnder` walks the whole tree. That matches
`generate-test.sh` exactly today. Nest a test project one level deeper and the audit reports that
project's **entire** generated tree as orphans — under a message advising the reader to confirm the
configuration and then delete by hand. Given the configuration was the wrong side 37 times out of
39 at `f16a0c896`, this is the one failure mode where the tool's advice could destroy real tests.
Either walk configurations as deep as files, or fail loudly on a `Generated/` tree in a project
carrying no configuration.

**B2. `ApplySharedDefaults` is reachable only through the loader.** A `TestConfiguration` built in
code and handed straight to a generator renders an empty `MessageBuilder`/`MessageAssertion`
**silently, into a `.cs` file**. Every real caller goes through the loader and the docs now say so,
but `GenerationPlanPurityTests` constructs one directly, which shows how available the wrong path
is. It is idempotent, so having the generators call it too costs nothing.

**B3. Ordinal path comparison.** The two sets compare `StringComparer.Ordinal` while macOS and
Windows filesystems do not. A configuration key whose casing differs from the on-disk folder
reports every file in that suite as **both** an orphan and missing — two unrelated-looking
failures for something that is actually fine. Verified safe today: every `Outboxes` /
`MessagingGateways` key matches its folder name exactly, including case, and CI is
`ubuntu-latest`. A comparer chosen by `OperatingSystem.IsLinux()`, or a note in the failure
message when the two sets differ only by case, would close it.

---

## C. What the audit still cannot see

**C1. Content drift.** Both directions ask whether a file is *present*. Edit a `.liquid` template
and forget to regenerate, and every file is present, every name correct, and the audit is green —
arguably the most common drift of all, and the shape of the #4300 story itself. This is stated
honestly in `GeneratedTreeAudit`'s remarks and in `.agent_instructions/generated_tests.md`, but the
caveat should not become permanent by default. Needs a render-to-string seam on `Parser`; with
`PlannedFile` already carrying `(TemplatePath, DestinationPath)` the comparison itself is nearly
free.

**C2. The `Missing` direction for the four shared root files.** Scoping *orphans* out of project
roots is right — the directory is not generator-owned, so "on disk and not planned" says nothing
there. That argument does **not** apply to *missing*: "the generator would write this and it is not
there" is well-formed regardless of who else owns the directory, and would catch a project the
shared generator never ran over. Needs a `SharedGenerator.Plan`.

**C3. A canary pinning the documented scope.** No canary runs `SharedGenerator`, so nothing
confirms that project-root shared files are neither orphan nor missing. It holds today only because
`Collect` refuses to descend outside `Generated/`; widen the walk and the documented scope breaks
with nothing failing.

**C4. Convert the `MemberwiseClone` remark into a check.** `WithPrefix` / `WithDefaultsFrom` are
deep copies only while every property on both configuration types is a string or a value type. The
XML remarks say so — but this PR's whole thesis is that a statement kept beside the code drifts
from it. A ~10-line reflection test over the public properties of both types would fail the day
someone adds `List Tags`.

---

## D. CI

**D1. The audit gates package publication and suppresses all later tests in the `build` job.**
The step sits between `Build` and `Core Tests`, and steps fail-fast — so a stale generated file now
blocks `Upload packages as artifacts` and `Push to GitHub Feed` on `master`, and means **no
unit-test feedback at all** from that job. Running first was a deliberate choice (drift reported
before the long suites rather than after them), and the inverse cost is real. The audit needs no
infrastructure and takes seconds, so a standalone `generator-audit` job would give the same gate,
report in parallel, and leave both other concerns unblocked. Worth a deliberate decision rather
than a consequence of step ordering.

**D2. `net9.0` compiles the generator tests but never runs them.** `-f net10.0` was chosen because
neither the walk nor the generator is framework-sensitive. Noting the consequence, not disputing
the reasoning.

---

## E. Hygiene — three of these were introduced by the review fixes themselves

**E1. ⚠️ `TestConfigurationLoader` carries two sibling `` elements.** Only the first
survives IntelliSense and DocFX, and the one that disappears is the *"the returned
`DestinationFolder` is always absolute"* contract — the interesting half. This is the **same defect
that was fixed on `WithPrefix` earlier in the same PR** and then reintroduced here. Merge into one
`` with `` blocks.

**E2. ⚠️ A test name that now contradicts what it asserts.**
`When_a_configuration_names_a_relative_destination_folder_should_resolve_it_against_the_default`
asserts resolution against the **configuration file's own folder** — the two bases coincide in that
particular test, so the name states the one thing the behaviour is *not*.
`..._should_resolve_it_against_the_configuration_folder` says what it checks.

**E3. ⚠️ `using System;` is now unused** in `MessagingGatewayGenerator.cs` — the deleted override was
its only consumer, and no `Func`, `StringComparison` or other `System` type remains. `OutboxGenerator.cs`
still needs its own.

**E4.** `file.Name.Replace(".liquid", string.Empty)` replaces *every* occurrence rather than the
extension. `Path.GetFileNameWithoutExtension` is exact and says what it means. Pre-existing.

**E5.** `GenerationSuite.Prefix` is a relative destination path, not a prefix — and it sits a few
lines from `OutboxConfiguration.Prefix` / `MessagingGatewayConfiguration.Prefix`, which mean a
namespace/folder token. `DestinationRelativePath` removes the collision.

**E6.** `WithPrefix(...).WithDefaultsFrom(...)` clones twice. Irrelevant at this scale; the reason
to mention it is that a single `WithRenderingDefaults(configuration, modelPrefix)` would make it
impossible to apply one and forget the other, which is the failure mode these methods exist to
prevent.

**E7.** `TestConfigurationLoader.Load` throws `JsonException` without naming the file it was
reading, and `ExpectedFilesUnder` walks 14 configurations. Wrapping with the path makes the failure
attributable. Related: `RepositoryTreeAudit` builds the audit in a property initializer, which
defeats `GeneratedTreeAudit.Of`'s stated purpose of making a failure attributable to a named
operation.

**E8.** `Collect` follows directory symlinks; a cycle under `tests/` recurses forever. Very
unlikely, but a visited-set on resolved paths is cheap.

**E9.** `When_outbox_configuration_missing_namespace_...` now pins
`namespace MyApp.Tests.OutboxSqlServer.Sync;`. That is faithfully what the singular branch does — it
does not dot-qualify where the plural branch does — but the test is the first thing to record
`OutboxSqlServer` as *intended*. Real singular configs all leave `Prefix` empty, so it never
manifests; an empty prefix in the test would match how the form is actually written.

**E10.** `PlannedFile`'s XML says both paths are absolute. `DestinationPath` is absolute via the
loader, but not for a directly-constructed `TestConfiguration` as several tests use. Doc precision.

**E11.** The canary file name (`When_a_file_the_generator_would_not_write_is_planted_should_report_an_orphan.cs`)
now holds five behaviours. Understandable given the shared temp-tree fixture, but a reader looking
for the deleted-configuration canary will not find it by file name.

---

## F. Process notes, recorded not actionable

- The change making the `ignore` predicate take a file name rather than the full path is
**behavioural**, and it landed in a commit labelled `refactor:`. It is inert on this tree (no path
segment matches any `SkipTest` substring) and "`./generate-test.sh` changed no generated file"
does not establish it — the two behaviours diverge only for checkout paths nobody has. Per
CLAUDE.md's Tidy First rule it should have been separated. A cleaner end state would make the
comment unnecessary by having the signature state the contract, e.g. taking a `FileInfo`.
- #4323's history reads fix-then-test: the audit (commit 3) is the regression test for the defect
fixed in commit 2. The PR body records that this was verified by removing the three files
afterwards, which is the same evidence — but the ordering is what the workflow is for.