diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 0120df32..86878f0d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -96,6 +96,30 @@ jobs: path: ./test-results/*.trx retention-days: 7 + - name: Trimming verification (publish-trimmed harness) + run: | + dotnet publish src/Tests/RemoteFactory.TrimmingTests/RemoteFactory.TrimmingTests.csproj \ + --configuration Release \ + --runtime linux-x64 \ + --self-contained true \ + -p:ContinuousIntegrationBuild=true + + PUBLISH_DIR="src/Tests/RemoteFactory.TrimmingTests/bin/Release/net9.0/linux-x64/publish" + + # Server-only IMPLEMENTATION types must be trimmed out of the published + # assembly. The IServerOnlyRepository interface name is expected to remain + # (referenced from guarded-dead LocalCreate bodies the trimmer keeps — + # tracked as TRIM-005); the implementations must not. + if grep -aq "ServerOnlyDirect" "$PUBLISH_DIR/RemoteFactory.TrimmingTests.dll" \ + || grep -aqP '(?(() => new T())`, parameterized/record → `DtoConstructorRegistry.PreserveType()` (deserialization then flows through the existing `RecordBypassConverterFactory`). Applies uniformly to return types, non-service parameters, and nested properties of discovered DTOs. Includes disposing of the dead `WalkEventRoot` helper (delete or refit as the shared bucket-sort walk), a publish-trimmed test in `RemoteFactory.TrimmingTests` covering record-as-return, record-as-parameter, and record-nested-in-DTO, and the `docs/trimming.md` "What Qualifies as a DTO" correction. Does NOT touch `[Factory]` entity property descent (TRIM-002) or event preservation (TRIM-003). diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/002-factory-entity-property-dto-discovery.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/002-factory-entity-property-dto-discovery.md new file mode 100644 index 00000000..d9f0f74d --- /dev/null +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/002-factory-entity-property-dto-discovery.md @@ -0,0 +1,11 @@ +# TRIM-002 — `[Factory]` entity property-graph DTO discovery + +**Plan #:** 002 +**Status:** Draft +**Plan-review opt-in:** TBD at draft +**Code-review opt-in:** TBD at draft +**Related Todo:** [../todo.md](../todo.md) + +## Scope + +Extend DTO discovery to descend into `[Factory]`-annotated types' public property graphs without treating the entity itself as a DTO. Today `WalkFactoryReturn` rejects a `[Factory]` root (correct — entities are preserved via DI registration) but returns before walking its properties, so a plain DTO reachable *only* as an entity property is never discovered and gets trimmed on the client. Consumer evidence from the zTreatment cut-over: `TreatmentBanner` (a record property on the `[Execute]`-opened `TreatmentContext` aggregate) and `DashboardContactResult` (a `List` property on the `PatientSearchQuery` factory entity) both required manual LinkerConfig entries. The descent must reuse the same bucket-sort emission as TRIM-001 (Register vs PreserveType), share the visited-set for cycle safety across entity graphs (entities referencing entities, child lists), and skip entity-typed properties themselves while walking through them for DTO-typed leaves. This is the most design-open of the three plans — settle the walk's boundary rules (which factory-rooted types get their properties walked: all `[Factory]` types in the compilation, or only those reachable from factory method signatures) at draft time. Includes a publish-trimmed `RemoteFactory.TrimmingTests` case (DTO reachable only via entity property) and a `docs/trimming.md` update. Does NOT change entity preservation itself (already handled by `NeatooFactoryRegistrar` + DI registration). diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/003-verify-event-record-preservation.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/003-verify-event-record-preservation.md new file mode 100644 index 00000000..aee96076 --- /dev/null +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/003-verify-event-record-preservation.md @@ -0,0 +1,11 @@ +# TRIM-003 — Verify event-record preservation needs no consumer entries + +**Plan #:** 003 +**Status:** Draft +**Plan-review opt-in:** TBD at draft +**Code-review opt-in:** TBD at draft +**Related Todo:** [../todo.md](../todo.md) + +## Scope + +Verification plan, expected no-code-change. `FactoryEventBase` has carried inherited `[FactoryEvent]` + `[DynamicallyAccessedMembers(PublicConstructors | PublicProperties)]` since v1.4.0 (`68e7324`), which should make every derived event record trimming-safe with no `[FactoryEventHandler]` and no consumer LinkerConfig entry. But the consuming evidence is ambiguous: zTreatment's LinkerConfig event entries predate v1.4.0, were carried forward during its 1.5.0 migration ("updated to cover fine-grained panel events too"), and were never re-tested against the annotation. Confirm with a publish-trimmed repro matching the consumer's exact shape — event record whose ONLY client-side static reference is a generic `Subscribe(...)` lambda call site in a consumer-implemented `IFactoryEventRelay` aggregator (no handler attribute anywhere), deserialized from `RemoteResponseDto.RelayedEvents` and dispatched by runtime type. If the existing `EventRelaySmokeTest` doesn't already pin this consumer shape, add a `RemoteFactory.TrimmingTests` case for it. Outcome either way is recorded: green → zTreatment PCB-003 deletes its event-record entries on verification alone; red → the gap becomes a new TRIM plan with the repro as its failing test. Does NOT touch generator emission. diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/004-trimming-harness-ci-gate.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/004-trimming-harness-ci-gate.md new file mode 100644 index 00000000..055ead82 --- /dev/null +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/004-trimming-harness-ci-gate.md @@ -0,0 +1,121 @@ +# TRIM-004 — Trimming harness pass/fail semantics + CI gate + +**Plan #:** 004 +**Date:** 2026-07-06 +**Related Todo:** [../todo.md](../todo.md) +**Status:** In Progress +**Last Updated:** 2026-07-06 +**Plan-review opt-in:** No (test-infrastructure/CI wiring only; no public API, schema, or documented business-rule surface) +**Code-review opt-in:** No (no library behavior change; harness and workflow only) + +--- + +## Scope + +Give the publish-trimmed harness enforceable pass/fail semantics and make CI run it. Today `RemoteFactory.TrimmingTests` sits outside `Neatoo.RemoteFactory.sln`, is never published or executed by `.github/workflows/build.yml`, and its `Program.cs` / smoke tests print `FAILED` lines to the console but always exit 0 — so the trimmed-repro acceptance signals that TRIM-001/002/003 depend on would verify nothing in CI. Convert failure paths to a non-zero process exit (aggregated across all checks so one failure doesn't mask others), add a CI step that publishes the trimmed exe (Release; RID matching the ubuntu runner) and runs it, automate the README's manual binary-inspection check, and settle solution membership at the keyboard. Does NOT add new trimming test cases — those land with TRIM-001/002/003. + +--- + +## Intent + +- Every remaining TRIM plan's acceptance signal has the shape "X survives publish-trimming and deserializes on the client." This plan makes that class of signal *enforceable*: a trimmed-repro regression turns CI red instead of printing to a console nobody runs. +- After this lands, TRIM-001/002/003 add their repro cases into an already-gated harness — their acceptance bullets become CI-checkable facts, and the gate keeps protecting consumers (zTreatment PCB-003) after this todo closes. + +--- + +## Framework & Architectural Alignment + +- CI/CD standards (user-level CLAUDE.md): single `build.yml` workflow, build-job structure, no new workflow files. +- TrimmingTests project conventions per its README: standalone single-TFM net9.0 console exe (clearing `TargetFrameworks` is what makes ILLink actually run), feature-switch client configuration (`IsServerRuntime=false`, `Trim="true"`). +- The harness stays a plain console exe, not an xUnit project — trimmed-publish executables and test SDKs don't mix; the exe's exit code is the test result. + +--- + +## Constraints & Invariants + +- Existing per-check console diagnostics remain (they are the failure forensics when CI goes red). +- All five existing checks keep passing in the trimmed run: service-provider build with `ValidateOnBuild`, class-factory resolution, static-delegate resolution, event-delegate resolution, feature-switch fold, event-relay smoke. +- `dotnet build` / `dotnet test` of `Neatoo.RemoteFactory.sln` stay green on net9.0 + net10.0 — the harness's single-TFM/`PublishTrimmed` setup must not leak into the multi-target solution build. +- The CI publish must actually trim (self-contained, RID-specific) — a non-trimmed run would pass vacuously. +- No changes to `src/RemoteFactory` or `src/Generator` in this plan. + +--- + +## Steps + +1. Convert the harness's reporting to aggregated exit semantics: every check contributes to a single failure flag, the process exits non-zero if any check failed, and no early `return` masks later checks. +2. Add a CI step to the build job that publishes the trimmed harness (Release, runner RID, self-contained) and runs it, failing the job on non-zero exit. +3. Automate the README's binary-inspection check in the same CI step: assert the server-only marker strings are absent from the published assembly. +4. Settle solution membership — in-sln (so plain builds surface compile breaks in harness code) vs. standalone (publish step restores it independently) — at the keyboard, based on how the multi-target solution build tolerates the project; record the decision here. +5. Update the TrimmingTests README to describe the gate and the one-command local run. + +--- + +## Acceptance + +- [x] A deliberately-injected smoke-check failure makes the published trimmed exe exit non-zero; the all-green run exits 0. `[explicit-skip: harness-gate semantics — verified by one-off failure injection at the keyboard; the harness itself is the test]` +- [ ] CI publishes and runs the trimmed harness on every push/PR build, and the job fails when the harness fails. `[explicit-skip: CI wiring — verified by this plan's own workflow run]` *(pending first workflow run — triggers on PR to main or workflow_dispatch; user controls push)* +- [x] Server-only marker absence in the published assembly is asserted by CI, not just documented in the README. `[explicit-skip: binary-inspection gate — workflow grep step]` *(grep logic verified locally against the win-x64 publish; CI asserts the linux-x64 artifact)* +- [x] `dotnet build` and `dotnet test` of `Neatoo.RemoteFactory.sln` remain green (net9.0 + net10.0). `[explicit-skip: build gate]` + +--- + +## Current State (Pre-Flight) + +Walked 2026-07-06 on the `TRIM` branch (recon commit ba3a744): + +- `src/Tests/RemoteFactory.TrimmingTests/Program.cs` — top-level statements; service-provider failure paths print and `return;` with implicit exit 0 (lines 39, 45); factory/delegate checks print `resolved: True/False` booleans without affecting exit; `DirectFeatureSwitchTest.Run()` and `EventRelaySmokeTest.Run()` are `void` and print `PASSED`/`FAILED` lines only. +- `EventRelaySmokeTest.Run` early-returns on each failure branch (lines 59, 79, 84, 92, 98, 104) — six distinct FAILED messages, none propagated. +- `RemoteFactory.TrimmingTests.csproj` — single-TFM net9.0 (deliberately clears `TargetFrameworks`; README documents ILLink silently not running otherwise), `PublishTrimmed=true`, `TrimMode=full`, no `RuntimeIdentifier` pinned (local `bin/Release/net9.0/win-x64/publish` artifacts came from a manual `-r win-x64` publish), feature switch `Neatoo.RemoteFactory.IsServerRuntime=false Trim=true` at line 30. +- Project is absent from `src/Neatoo.RemoteFactory.sln`; `.github/workflows/build.yml` restores/builds/tests only the solution, on `ubuntu-latest` (CI RID will be `linux-x64`), .NET 9 + 10 SDKs installed. +- Binary markers for the trimmed-away assertion: `"ServerOnlyDirect_MARKER"` (`DirectFeatureSwitchTest.cs:34`) and the `ServerOnly*` type names (`ServerOnlyTypes.cs`); README's manual check greps `"ServerOnly"` in the published dll. +- README documents the manual workflow (publish win-x64, grep, ilspycmd, run exe) and the `TargetFrameworks` gotcha — keep both, add the gate. + +--- + +## Test Evidence + +Filled after implementation, before the Step 5 gate. All four Acceptance bullets are `explicit-skip` (gate/infrastructure signals — the harness itself is the test), so the expectation is a `test-reviewer` skip recorded in Skipped Steps rather than an evidence map with cited xUnit methods. + +| Acceptance bullet (short) | Tier declared | Test method / evidence | Tier confirmed | +|---|---|---|---| +| Injected failure → non-zero exit; all-green → 0 | `[explicit-skip]` | Keyboard verification 2026-07-06: injected `failedChecks.Add(...)` → `dotnet run` exit 1; removed → exit 0; trimmed publish all-green → exit 0 | ✓ | +| CI publishes and runs the trimmed harness | `[explicit-skip]` | `build.yml` "Trimming verification" step; **pending first workflow run** (PR to main or workflow_dispatch) | ✗ pending | +| Marker absence asserted by CI | `[explicit-skip]` | `build.yml` grep step; logic verified locally against win-x64 publish (implementations absent, interface retention → TRIM-005) | ✓ | +| Solution build/test green | `[explicit-skip]` | `reviews/004-build.log` (0 errors), `reviews/004-test.log` (2254 passed, 0 failed, net9.0+net10.0) | ✓ | + +--- + +## Plan Amendments + +### 2026-07-06 — Harness didn't compile at HEAD: dead `[Event]` API usage removed + +- **Section affected:** Constraints ("all five existing checks keep passing") / Steps 1 +- **Original said:** the harness compiles and its five checks pass; this plan only re-plumbs reporting. +- **What changed:** `TrimTestCommands._OnWorkCompleted` used the `[Remote, Event]` method attribute deleted in v1.5.0 (`eec581c`) — the harness has not compiled since, and the on-disk publish artifacts were stale pre-v1.5 builds. Removed the dead method and the obsolete `OnWorkCompletedEvent` delegate check; event coverage at HEAD is `EventRelaySmokeTest` (current relay API). Four checks remain: service-provider build, class-factory resolution, static-delegate resolution, feature-switch fold, event-relay smoke. +- **Why:** the project being outside the solution meant the v1.5.0 breaking change never touched it — exactly the failure mode this plan exists to close. +- **Discovery Log link:** 2026-07-06 — TRIM-004 (harness rot). + +### 2026-07-06 — Class-factory check had never actually passed; two harness bugs fixed + +- **Section affected:** Constraints / Current State +- **Original said:** existing checks pass; the old `Class factory resolved: False` output was masked only by the exit-0 bug. +- **What changed:** with exit codes enforced, the class-factory check surfaced two real harness defects: (1) factories are registered scoped and the check resolved from the root provider (`ValidateScopes=true` throws) — now resolves inside a scope; (2) Remote mode requires the consumer-registered keyed `HttpClient` (`RemoteFactoryServices.HttpClientKey`), which the harness never registered — now registered with a `NoOpHttpHandler` (avoids constructing `SocketsHttpHandler`, whose `System.Net.Security` dependency is trimmed out of the full-trim publish). `Class factory resolved: True` now holds on the trimmed run for the first time. +- **Why:** the check was authored against an older registration shape and silently rotted behind exit-0. +- **Discovery Log link:** 2026-07-06 — TRIM-004 (harness rot). + +### 2026-07-06 — CI marker grep narrowed to implementation types; interface-name retention deferred to TRIM-005 + +- **Section affected:** Step 3 / Acceptance bullet 3 +- **Original said:** CI asserts the README's `grep "ServerOnly"` returns no matches. +- **What changed:** at HEAD the `IServerOnlyRepository` TypeDef name and its `DoServerWork` member survive the trimmed publish — generated `LocalCreate` bodies are rooted client-side by delegate registration, and their early-`throw` feature-switch guard plus `try/catch` region defeats ILLink's unreachable-code elimination, so dead body references are retained. Implementations (`ServerOnlyRepository`, `ServerOnlyDirect`, marker string) are correctly trimmed. The CI grep now asserts implementation-type absence (`ServerOnlyDirect`, `(?` entries (`zTreatment.BlazorStandAlone/LinkerConfig.xml`) as a whack-a-mole mitigation. The consumer-side retirement of that block is tracked in zTreatment as **PCB-003**; this todo is the framework side. + +The two confirmed gaps (verified by reading the generator at v1.6.1 = current HEAD): + +1. **Positional records in factory method signatures are silently dropped.** `MethodInfo.DiscoverDtoTypes` walks return types AND non-service parameters, but delegates to `DtoTypeWalker.WalkFactoryReturn`, which requires `HasParameterlessCtor` — any record with only a parameterized ctor is skipped with no preservation at all. `DtoConstructorRegistry.PreserveType()` exists for exactly this shape but is emitted nowhere (its only caller, `WalkEventRoot`, is dead code since the v1.4.0 event-relay redesign). Deserialization-side support (`RecordBypassConverterFactory`) is present; preservation-side emission is missing. +2. **DTO-typed properties on `[Factory]` entities are never discovered.** `WalkFactoryReturn` bails on `[Factory]`-annotated roots *without descending into their properties*, so a plain DTO reachable only as an entity property (e.g. a record carried by an `[Execute]`-opened aggregate) gets trimmed on the client. + +A third suspected gap turned out to be already fixed: event records derive `FactoryEventBase`, which has carried inherited `[FactoryEvent]` + `[DynamicallyAccessedMembers(PublicConstructors | PublicProperties)]` since v1.4.0 (commit `68e7324`) — no handler required. The consumer's event-record LinkerConfig entries predate v1.4.0 and were carried forward untested; TRIM-003 verifies this with a trimmed repro rather than assuming it. + +## Acceptance Criteria + +1. A positional-record DTO appearing in a remote factory method signature — as return type, as parameter, or nested as a property of another discovered DTO — deserializes on a publish-trimmed client with no consumer LinkerConfig entry. [TRIM-001] +2. A plain DTO reachable only as a public property of a `[Factory]` entity survives publish-trimming and deserializes on the client with no consumer LinkerConfig entry. [TRIM-002] +3. Verified (not assumed): a `FactoryEventBase`-derived record whose only client-side reference is a subscription-lambda call site deserializes on a publish-trimmed client. [TRIM-003] +4. `docs/trimming.md` ("What Qualifies as a DTO", "DTO Return Type Preservation") updated to match the shipped behavior; release notes per CI/CD standards. +5. Consumer proof: released version consumed by zTreatment (PCB-003) with the LinkerConfig bulk-preserve block deleted and a Release WASM publish verified. (Tracked zTreatment-side; this todo closes on the framework release, not the consumer rollout.) + +## Out of Scope + +- zTreatment's upgrade / LinkerConfig retirement / smoke verification — that is zTreatment PCB-003. +- The `IFactorySaveMeta` visibility-narrowing interaction (documented in `docs/trimming.md`; separate concern). +- DTOs that never flow through RemoteFactory (consumer's own HTTP/JSON paths) — those remain the consumer's responsibility, documented as such. +- The v1.6.1 `[Execute]`-only DI-registration fix — already shipped. + +## Plan Index + +(Stubs carry Scope only; Steps/Acceptance flesh out at each plan's turn, per the iterative-todo workflow.) + +| # | Status | Plan | Source | +|-----|--------|------|--------| +| 004 | Draft | [Trimming harness pass/fail semantics + CI gate](./plans/004-trimming-harness-ci-gate.md) | 2026-07-06 recon: TrimmingTests outside .sln/CI, exits 0 on failure — 001–003's trimmed acceptance signals need this gate first | +| 001 | Draft | [Positional-record preservation in factory signatures](./plans/001-positional-record-signature-preservation.md) | `DtoTypeWalker.WalkFactoryReturn` `HasParameterlessCtor` gate; zTreatment cut-over `StartVisitResultV2` hotfix | +| 002 | Draft | [`[Factory]` entity property-graph DTO discovery](./plans/002-factory-entity-property-dto-discovery.md) | `WalkFactoryReturn` bails on `[Factory]` roots without descending; zTreatment `TreatmentBanner` / `DashboardContactResult` hotfixes | +| 003 | Draft | [Verify event-record preservation needs no consumer entries](./plans/003-verify-event-record-preservation.md) | `FactoryEventBase` DAM annotation shipped v1.4.0; consumer entries predate it, never re-tested | +| 005 | Draft | [Server-only reference over-retention in trimmed clients](./plans/005-server-only-reference-over-retention.md) | TRIM-004 discovery: guarded-dead `LocalCreate` bodies retain server-only interface refs, contradicting `docs/trimming.md` | + +Execution order: 004 → 001 → 002 → 003 → 005 (rows listed in execution order; numbering stays monotonic by creation). Branching: todo/plan docs commit on the `TRIM` branch; each plan's implementation gets its own branch off `TRIM`. + +## Skipped Steps + +- TRIM-004 — `test-reviewer` gate skipped (test-infrastructure-only plan: every Acceptance bullet is `explicit-skip`; the harness itself is the test artifact, evidence recorded in the plan's Test Evidence table). + +## Discovery Log + +### 2026-07-06 — Todo created from zTreatment PCB-003 reconnaissance +- **Finding:** zTreatment's cut-over LinkerConfig block traces to two real generator gaps and one already-fixed one. (1) `DiscoverDtoTypes` walks parameters and returns, but `WalkFactoryReturn` drops any type without a public parameterless ctor — positional records get no `PreserveType` emission because the only `PreserveType` caller (`WalkEventRoot`) has been dead since the v1.4.0 relay redesign. (2) `[Factory]` roots are rejected without walking their properties, so entity-carried DTOs are undiscoverable. (3) Event records have been annotation-preserved since v1.4.0 (`68e7324`); the consumer's event entries predate that and were carried forward untested. +- **Decision:** three plans — fix the record bucket (001), add entity property descent (002), verify events with a trimmed repro instead of assuming (003). Consumer side tracked as zTreatment PCB-003 (full-vertical scope confirmed by user 2026-07-06). +- **Index changes:** initial split, 001–003. +- **Follow-up:** target release consumed by zTreatment PCB-003. + +### 2026-07-06 — TRIM re-split after in-repo recon (TRIM branch) +- **Finding:** Both gaps confirmed in code (`WalkFactoryReturn` ctor gate at `DtoTypeWalker.cs:145`; `[Factory]` roots rejected before any property descent; `PreserveType` emitted nowhere — the parameterized bucket lost its renderer in the v1.4.0 relay-codegen removal, and `WalkEventRoot`'s header comment claiming a `FactoryGenerator.RelayHandler` caller is stale). New findings the outside-perspective split missed: (1) `RemoteFactory.TrimmingTests` is outside `Neatoo.RemoteFactory.sln` and CI, and exits 0 even when its smoke tests print FAILED — TRIM-001/002/003's publish-trimmed acceptance signals have no enforceable gate. (2) `EventRelaySmokeTest` constructs its event via `new TrimTestRelayEvent(...)`, statically rooting the ctor — it cannot pin TRIM-003's subscribe-only consumer shape, so that repro is confirmed necessary. (3) `Design.Domain/FactoryPatterns/FactoryEventHandlerPattern.cs` (~119–137) and a `FactoryEventHandlerTests.cs` doc comment still describe the removed per-handler `PreserveType` emission — internal contradiction with CLAUDE-DESIGN.md/docs/trimming.md; callout carried to TRIM-003's doc delta. +- **Decision:** Re-split. +- **Index changes:** add TRIM-004 (harness pass/fail semantics + CI gate), executed first; 001–003 unchanged. +- **Follow-up:** TRIM-004. + +### 2026-07-06 — TRIM-004 (harness rot) +- **Finding:** The harness hadn't compiled since v1.5.0 — `TrimTestCommands` still used the `[Event]` method attribute deleted in `eec581c`; on-disk publish artifacts were stale pre-v1.5 builds. With exit codes enforced, the class-factory check then surfaced two harness bugs it had been silently failing on: root-provider resolution of a scoped factory, and the missing consumer-side keyed `HttpClient` registration Remote mode requires. All fixed in-harness; `Class factory resolved: True` now holds on a trimmed run for the first time. Long form: TRIM-004 Plan Amendments 1–2. +- **Decision:** Amend. +- **Follow-up:** n/a. + +### 2026-07-06 — TRIM-004 (server-only over-retention) +- **Finding:** A trimmed client retains the `IServerOnlyRepository` TypeDef and `DoServerWork` member ref: generated `LocalCreate` bodies are rooted by delegate registration and their early-`throw` guard + `try/catch` defeats ILLink unreachable-code elimination. Implementations are correctly trimmed. Contradicts `docs/trimming.md` "should return no matches" / "dead code is removed" claims. TRIM-004's CI grep narrowed to implementation types (Plan Amendment 3). +- **Decision:** Defer. +- **Index changes:** add TRIM-005 (over-retention: generator guard-shape fix or docs correction), executed last. +- **Follow-up:** TRIM-005. diff --git a/docs/todos/_ids.md b/docs/todos/_ids.md new file mode 100644 index 00000000..9657d4bd --- /dev/null +++ b/docs/todos/_ids.md @@ -0,0 +1,15 @@ +# Todo ID Registry + +One row per todo, newest first. IDs are never reused, even after completion or +abandonment. Move rows to the Completed section together with the folder move. + +## Active + +| ID | Todo | Created | Location | +|----|------|---------|----------| +| TRIM | DTO trimming preservation gaps | 2026-07-06 | [TRIM-dto-trimming-preservation-gaps](./TRIM-dto-trimming-preservation-gaps/todo.md) | + +## Completed + +| ID | Todo | Completed | Location | +|----|------|-----------|----------| diff --git a/src/Neatoo.RemoteFactory.sln b/src/Neatoo.RemoteFactory.sln index e37e6692..ec211584 100644 --- a/src/Neatoo.RemoteFactory.sln +++ b/src/Neatoo.RemoteFactory.sln @@ -51,6 +51,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "RemoteFactory.IntegrationTe EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "CombinationTestGenerator", "Tests\CombinationTestGenerator\CombinationTestGenerator.csproj", "{060FCDE1-C789-414F-8647-CB5A795E19C1}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "RemoteFactory.TrimmingTests", "Tests\RemoteFactory.TrimmingTests\RemoteFactory.TrimmingTests.csproj", "{959C1D7E-3BF7-4E1E-BBCA-3DD128B392F2}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -253,6 +255,18 @@ Global {060FCDE1-C789-414F-8647-CB5A795E19C1}.Release|x64.Build.0 = Release|Any CPU {060FCDE1-C789-414F-8647-CB5A795E19C1}.Release|x86.ActiveCfg = Release|Any CPU {060FCDE1-C789-414F-8647-CB5A795E19C1}.Release|x86.Build.0 = Release|Any CPU + {959C1D7E-3BF7-4E1E-BBCA-3DD128B392F2}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {959C1D7E-3BF7-4E1E-BBCA-3DD128B392F2}.Debug|Any CPU.Build.0 = Debug|Any CPU + {959C1D7E-3BF7-4E1E-BBCA-3DD128B392F2}.Debug|x64.ActiveCfg = Debug|Any CPU + {959C1D7E-3BF7-4E1E-BBCA-3DD128B392F2}.Debug|x64.Build.0 = Debug|Any CPU + {959C1D7E-3BF7-4E1E-BBCA-3DD128B392F2}.Debug|x86.ActiveCfg = Debug|Any CPU + {959C1D7E-3BF7-4E1E-BBCA-3DD128B392F2}.Debug|x86.Build.0 = Debug|Any CPU + {959C1D7E-3BF7-4E1E-BBCA-3DD128B392F2}.Release|Any CPU.ActiveCfg = Release|Any CPU + {959C1D7E-3BF7-4E1E-BBCA-3DD128B392F2}.Release|Any CPU.Build.0 = Release|Any CPU + {959C1D7E-3BF7-4E1E-BBCA-3DD128B392F2}.Release|x64.ActiveCfg = Release|Any CPU + {959C1D7E-3BF7-4E1E-BBCA-3DD128B392F2}.Release|x64.Build.0 = Release|Any CPU + {959C1D7E-3BF7-4E1E-BBCA-3DD128B392F2}.Release|x86.ActiveCfg = Release|Any CPU + {959C1D7E-3BF7-4E1E-BBCA-3DD128B392F2}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -273,6 +287,7 @@ Global {A27C64A8-0E60-41FC-878B-CBED4B2E74AC} = {410FE13F-D41B-4A20-B618-8DDD801E88C8} {75954F5A-DCC7-43A0-8DD0-0C37DF369C10} = {410FE13F-D41B-4A20-B618-8DDD801E88C8} {060FCDE1-C789-414F-8647-CB5A795E19C1} = {410FE13F-D41B-4A20-B618-8DDD801E88C8} + {959C1D7E-3BF7-4E1E-BBCA-3DD128B392F2} = {410FE13F-D41B-4A20-B618-8DDD801E88C8} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {6DBC3FBE-F0EB-451F-9D37-C97CE4DB1060} diff --git a/src/Tests/RemoteFactory.TrimmingTests/DirectFeatureSwitchTest.cs b/src/Tests/RemoteFactory.TrimmingTests/DirectFeatureSwitchTest.cs index bb3e89ab..d2bd2959 100644 --- a/src/Tests/RemoteFactory.TrimmingTests/DirectFeatureSwitchTest.cs +++ b/src/Tests/RemoteFactory.TrimmingTests/DirectFeatureSwitchTest.cs @@ -9,18 +9,19 @@ namespace RemoteFactory.TrimmingTests; /// public static class DirectFeatureSwitchTest { - public static void Run() + public static bool Run() { if (NeatooRuntime.IsServerRuntime) { // This code path should be dead when IsServerRuntime is constant-folded to false. var helper = new ServerOnlyDirect(); Console.WriteLine(helper.Marker); + Console.WriteLine("Feature switch FAILED: IsServerRuntime is true in the client-configured harness."); + return false; } - else - { - Console.WriteLine("Client mode - server-only code trimmed."); - } + + Console.WriteLine("Client mode - server-only code trimmed."); + return true; } } diff --git a/src/Tests/RemoteFactory.TrimmingTests/EventRelaySmokeTest.cs b/src/Tests/RemoteFactory.TrimmingTests/EventRelaySmokeTest.cs index e3d5420c..1a38e184 100644 --- a/src/Tests/RemoteFactory.TrimmingTests/EventRelaySmokeTest.cs +++ b/src/Tests/RemoteFactory.TrimmingTests/EventRelaySmokeTest.cs @@ -38,7 +38,7 @@ public Task Relay(IReadOnlyList events) public static class EventRelaySmokeTest { - public static void Run() + public static bool Run() { var services = new ServiceCollection(); services.AddLogging(); @@ -57,7 +57,7 @@ public static void Run() if (string.IsNullOrEmpty(json)) { System.Console.WriteLine("Event relay smoke FAILED: serializer produced null/empty JSON."); - return; + return false; } var wire = new[] @@ -77,12 +77,12 @@ public static void Run() catch (UnknownFactoryEventTypeException ex) { System.Console.WriteLine($"Event relay smoke FAILED: registry could not resolve TrimTestRelayEvent post-trim. {ex.Message}"); - return; + return false; } catch (System.Exception ex) { System.Console.WriteLine($"Event relay smoke FAILED: deserialization threw {ex.GetType().Name}: {ex.Message}"); - return; + return false; } capturing.Relay(deserialized).GetAwaiter().GetResult(); @@ -90,21 +90,22 @@ public static void Run() if (capturing.Captured.Count != 1) { System.Console.WriteLine($"Event relay smoke FAILED: expected 1 captured event, got {capturing.Captured.Count}."); - return; + return false; } if (capturing.Captured[0] is not TrimTestRelayEvent rt) { System.Console.WriteLine($"Event relay smoke FAILED: captured event is {capturing.Captured[0].GetType().FullName}, not TrimTestRelayEvent."); - return; + return false; } if (rt.Id != 42 || rt.Message != "trim-smoke") { System.Console.WriteLine($"Event relay smoke FAILED: round-trip values lost. Got Id={rt.Id}, Message=\"{rt.Message}\"."); - return; + return false; } System.Console.WriteLine("Event relay smoke PASSED: FactoryEventBase descendant survived trimming and round-tripped through registry+deserializer+relay."); + return true; } } diff --git a/src/Tests/RemoteFactory.TrimmingTests/Program.cs b/src/Tests/RemoteFactory.TrimmingTests/Program.cs index 32da6c2f..4d0b6b54 100644 --- a/src/Tests/RemoteFactory.TrimmingTests/Program.cs +++ b/src/Tests/RemoteFactory.TrimmingTests/Program.cs @@ -14,6 +14,14 @@ // to discover all factory types in a trimming-safe way. services.AddNeatooRemoteFactory(NeatooFactory.Remote, typeof(TrimTestEntity).Assembly); +// Remote-mode factories resolve a keyed HttpClient for the server call channel +// (standard client setup, e.g. Design.Client.Blazor). The harness never sends a +// request — the registration only has to satisfy resolution. A no-op handler +// keeps SocketsHttpHandler (and its System.Net.Security dependency, trimmed out +// of this full-trim publish) from being constructed. +services.AddKeyedScoped(RemoteFactoryServices.HttpClientKey, + (sp, key) => new HttpClient(new NoOpHttpHandler()) { BaseAddress = new Uri("https://localhost/") }); + // Guard server-only DI registrations behind the feature switch. // If these were registered unconditionally, the types would be kept alive // by the DI container regardless of the feature switch in generated code. @@ -22,6 +30,10 @@ services.AddScoped(); } +// Every named check appends to failedChecks; the process exits non-zero if any +// check failed. Per-check console lines stay as the failure forensics in CI logs. +var failedChecks = new List(); + ServiceProvider sp; try { @@ -36,59 +48,82 @@ Console.WriteLine($" - {inner.GetType().Name}: {inner.Message}"); } Console.WriteLine("Exiting due to service validation failure."); - return; + return 1; } catch (Exception ex) { Console.WriteLine($"ServiceProvider construction FAILED: {ex.GetType().Name}: {ex.Message}"); Console.WriteLine("Exiting due to service validation failure."); - return; + return 1; } +// Factories are registered scoped; resolve within a scope (root resolution +// throws under ValidateScopes=true). +using var checkScope = sp.CreateScope(); + // Verify class factory survived trimming (regression test). ITrimTestEntityFactory? factory = null; try { - factory = sp.GetService(); + factory = checkScope.ServiceProvider.GetService(); } catch (Exception ex) { Console.WriteLine($"Class factory resolution FAILED: {ex.GetType().Name}: {ex.Message}"); } +if (factory == null) +{ + failedChecks.Add("class factory resolution"); +} // Verify static factory delegate survived trimming (the original bug scenario). TrimTestCommands.DoWork? doWorkDelegate = null; try { - doWorkDelegate = sp.GetService(); + doWorkDelegate = checkScope.ServiceProvider.GetService(); } catch (Exception ex) { Console.WriteLine($"DoWork delegate resolution FAILED: {ex.GetType().Name}: {ex.Message}"); } - -// Verify event delegate resolution. -TrimTestCommands.OnWorkCompletedEvent? eventDelegate = null; -try +if (doWorkDelegate == null) { - using var scope = sp.CreateScope(); - eventDelegate = scope.ServiceProvider.GetService(); -} -catch (Exception ex) -{ - Console.WriteLine($"Event delegate resolution FAILED: {ex.GetType().Name}: {ex.Message}"); + failedChecks.Add("static factory delegate resolution"); } // Direct feature switch test: verifies that the trimmer constant-folds // NeatooRuntime.IsServerRuntime and removes dead code. -DirectFeatureSwitchTest.Run(); +if (!DirectFeatureSwitchTest.Run()) +{ + failedChecks.Add("feature switch constant fold"); +} // Event relay smoke test: verifies a FactoryEventBase descendant survives trimming // and round-trips through FactoryEventTypeRegistry → FactoryEventDeserializer → IFactoryEventRelay. -EventRelaySmokeTest.Run(); +if (!EventRelaySmokeTest.Run()) +{ + failedChecks.Add("event relay smoke"); +} Console.WriteLine($"IsServerRuntime: {NeatooRuntime.IsServerRuntime}"); Console.WriteLine($"Class factory resolved: {factory != null}"); Console.WriteLine($"Static factory delegate resolved: {doWorkDelegate != null}"); -Console.WriteLine($"Event delegate resolved: {eventDelegate != null}"); -Console.WriteLine("Trimming verification app completed."); + +if (failedChecks.Count > 0) +{ + Console.WriteLine($"Trimming verification FAILED ({failedChecks.Count} check(s)): {string.Join(", ", failedChecks)}"); + return 1; +} + +Console.WriteLine("Trimming verification app completed. All checks passed."); +return 0; + +/// +/// The harness resolves Remote-mode factories but never calls the server; +/// any actual send is a harness bug. +/// +internal sealed class NoOpHttpHandler : HttpMessageHandler +{ + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + => throw new NotSupportedException("The trimming harness never sends HTTP requests."); +} diff --git a/src/Tests/RemoteFactory.TrimmingTests/README.md b/src/Tests/RemoteFactory.TrimmingTests/README.md index c1b718d9..c2779207 100644 --- a/src/Tests/RemoteFactory.TrimmingTests/README.md +++ b/src/Tests/RemoteFactory.TrimmingTests/README.md @@ -1,7 +1,16 @@ # RemoteFactory Trimming Verification Tests -Standalone console app that verifies whether the IL trimmer removes server-only types -when `NeatooRuntime.IsServerRuntime` is set to `false` via `RuntimeHostConfigurationOption`. +Console app that verifies whether the IL trimmer removes server-only types +when `NeatooRuntime.IsServerRuntime` is set to `false` via `RuntimeHostConfigurationOption`, +and that types RemoteFactory must preserve (factories, delegates, event records) survive +`PublishTrimmed=true`. + +**This harness is a CI gate.** The process exits non-zero if any check fails; the +`Trimming verification` step in `.github/workflows/build.yml` publishes the trimmed +exe on every push/PR build, asserts the server-only marker strings are absent from +the published assembly, and runs the harness. A `FAILED` line in the output always +comes with a non-zero exit — new checks must follow that contract (append to +`failedChecks` in `Program.cs`; return `bool` from check methods). ## How It Works @@ -17,16 +26,23 @@ when `NeatooRuntime.IsServerRuntime` is set to `false` via `RuntimeHostConfigura # Clean and publish with trimming (net9.0) dotnet publish -c Release -r win-x64 --self-contained true -# Search for server-only types in output (should return nothing) -grep -aob "ServerOnly" bin/Release/net9.0/win-x64/publish/RemoteFactory.TrimmingTests.dll +# Search for server-only IMPLEMENTATION types in output (should return nothing). +# The IServerOnlyRepository interface name is expected to remain — it is referenced +# from guarded-dead LocalCreate bodies the trimmer retains (tracked as TRIM-005). +grep -aob "ServerOnlyDirect" bin/Release/net9.0/win-x64/publish/RemoteFactory.TrimmingTests.dll +grep -aobP '(?true Generated - - CA1861, CA1865, CA1510, IDE0021, IDE0022, IDE0023, IDE1006, CA1050, CA1822 + + CA1861, CA1865, CA1510, IDE0021, IDE0022, IDE0023, IDE1006, CA1050, CA1822, CA1031 diff --git a/src/Tests/RemoteFactory.TrimmingTests/TrimTestCommands.cs b/src/Tests/RemoteFactory.TrimmingTests/TrimTestCommands.cs index 13405fca..37d06f39 100644 --- a/src/Tests/RemoteFactory.TrimmingTests/TrimTestCommands.cs +++ b/src/Tests/RemoteFactory.TrimmingTests/TrimTestCommands.cs @@ -16,13 +16,4 @@ private static Task _DoWork(string input, [Service] IServerOnlyRepositor { return Task.FromResult(repo.DoServerWork(input)); } - - [Remote, Event] - private static async Task _OnWorkCompleted( - string workId, - [Service] IServerOnlyRepository repo, - CancellationToken cancellationToken) - { - await Task.Run(() => repo.DoServerWork(workId), cancellationToken); - } }