fix: factory fetch NRE when [Fetch] returns false with [AuthorizeFactory]

docs/plans/completed/fix-factory-fetch-nre-on-not-found.memory/architect.md

@@ -0,0 +1,81 @@
+# Architect -- Fix Factory Fetch NRE on Not Found
+
+Last updated: 2026-03-21
+Current step: Post-implementation verification (Step 8A) complete -- VERIFIED
+
+## Key Context
+
+- Bug is in `ClassFactoryRenderer.cs` at two sites: read-style (~line 460) and write-style (~line 900)
+- Both emit `return default!;` on the bool-false path, which is null for `Authorized<T>`
+- Public wrappers then dereference `.Result` (read-style) or `.HasAccess` (save-style) on null
+- Fix: emit `new Authorized<T>()` instead of `default!` when `HasAuth` is true
+- Developer deviated from plan (`new Authorized<T>(default)` to `new Authorized<T>()`) due to CS0121 constructor ambiguity -- verified acceptable
+
+## Mistakes to Avoid
+
+- Original plan said `new Authorized<T>(default)` but this causes CS0121 ambiguity between `Authorized(Authorized)` and `Authorized(T?)` constructors when `T` is unconstrained and `default` is `null`
+- `new Authorized<T>()` is the correct choice -- it produces identical runtime state (`HasAccess = false`, `Result = null`) and matches existing save routing default pattern at `RenderSaveLocalMethod`
+- Plan design section referenced `AuthorizeFactoryOperation.Read` but the correct enum value for Fetch operations is `AuthorizeFactoryOperation.Fetch` -- developer correctly used `.Fetch`
+
+## User Corrections
+
+None.
+
+## Architectural Verification (Pre-Handoff)
+
+Scope table and evidence documented in plan file. No breaking changes. The fix replaces a crash (NRE) with the contractually correct return value (null).
+
+## Architect Verification (Post-Implementation)
+
+**Date:** 2026-03-21
+**Verdict:** VERIFIED
+
+### Independent Build Results
+
+- `dotnet build src/Neatoo.RemoteFactory.sln`: 0 errors, 2 warnings (pre-existing Blazor WASM NativeFileReference warnings)
+- `dotnet test src/Neatoo.RemoteFactory.sln`: ALL PASS
+  - Integration tests (net9.0): 502 passed, 3 skipped, 0 failed
+  - Integration tests (net10.0): 502 passed, 3 skipped, 0 failed
+  - Unit tests (net9.0): 490 passed, 0 skipped, 0 failed
+  - Unit tests (net10.0): 490 passed, 0 skipped, 0 failed
+  - Total: 1984 passed, 6 skipped (pre-existing ShowcasePerformanceTests), 0 failed
+
+### Design Match
+
+**Site 1 (read-style, ClassFactoryRenderer.cs lines 462-472):** Fix matches design. `method.HasAuth` conditional added before existing branches. When true, emits `new Authorized<{model.ServiceTypeName}>()`. Handles both async and Task.FromResult variants. Comments explain the rationale.
+
+**Site 2 (write-style, ClassFactoryRenderer.cs lines 904-914):** Identical fix pattern to Site 1. Comments reference `.HasAccess` (write-style wrapper) rather than `.Result` (read-style wrapper), showing awareness of both NRE sites.
+
+**Test targets (RoundTripTargets.cs lines 194-267):**
+- `FetchAuthAllow`: Authorization class with `AuthorizeFactoryOperation.Fetch` (correct enum value) returning `true` (allows access -- testing not-found path, not auth denial)
+- `RemoteFetchTarget_AuthBoolFalse`: Core bug scenario -- auth + bool-false Fetch
+- `RemoteFetchTarget_AuthBoolTrue`: Regression guard -- auth + bool-true Fetch
+- `RemoteFetchTarget_RemoteAuthBoolFalse`: Auth + bool-false + [Service] injection (proves server execution via remote transport)
+
+**Tests (RemoteFetchRoundTripTests.cs lines 141-197):**
+- Scenario 1: `RemoteFetch_AuthBoolFalse_ServerReturnsNull` -- server container, asserts null
+- Scenario 2: `RemoteFetch_AuthBoolFalse_ClientReturnsNull` -- client container round-trip, asserts null
+- Scenario 3: `RemoteFetch_AuthBoolTrue_ReturnsObject` -- client container, asserts non-null + state intact
+- Scenario 5: `RemoteFetch_RemoteAuthBoolFalse_ReturnsNull` -- client container with [Service], asserts null
+
+### Deviation Assessment
+
+Developer used `new Authorized<T>()` instead of plan's `new Authorized<T>(default)`. This is **acceptable and correct**:
+- `new Authorized<T>(default)` causes CS0121 ambiguity between `Authorized(Authorized)` and `Authorized(T?)` constructors
+- `new Authorized<T>()` parameterless constructor calls base `Authorized()` which sets `HasAccess = false`; `Result` property defaults to `null` (default for reference types via `T? Result { get; init; }`)
+- Runtime state is identical: `HasAccess = false`, `Result = null`
+- This pattern is already established in the codebase at `RenderSaveLocalMethod` (line ~1083)
+
+### Test Scenario Coverage
+
+| Scenario # | Test Method | Covered? |
+|------------|-------------|----------|
+| 1 | `RemoteFetch_AuthBoolFalse_ServerReturnsNull` | YES |
+| 2 | `RemoteFetch_AuthBoolFalse_ClientReturnsNull` | YES |
+| 3 | `RemoteFetch_AuthBoolTrue_ReturnsObject` | YES |
+| 4 | (Existing tests) | YES (no regression -- all 1984 tests pass) |
+| 5 | `RemoteFetch_RemoteAuthBoolFalse_ReturnsNull` | YES |
+
+### Issues Found
+
+None.

docs/plans/completed/fix-factory-fetch-nre-on-not-found.memory/developer.md

@@ -0,0 +1,58 @@
+# Developer -- Fix Factory Fetch NRE on Not Found
+
+Last updated: 2026-03-21
+Current step: Implementation complete, awaiting verification (Step 8)
+
+## Key Context
+
+- Bug is in `ClassFactoryRenderer.cs` at two sites: read-style line 460 and write-style line 900
+- Both emitted `return default!;` when `_succeeded == false` and `method.HasAuth` is true
+- `default!` for `Authorized<T>` (reference type) is `null`
+- Public wrapper at line 315 does `(await ...).Result` -- NRE when `.Result` is called on null
+- Save public wrapper at line 1009 does `.HasAccess` -- same NRE pattern
+- Fix: emit `new Authorized<T>()` instead of `default!` when `HasAuth` is true
+- Used parameterless constructor `new Authorized<T>()` NOT `new Authorized<T>(default)` due to CS0121 ambiguity
+- The save routing default at line 1083 already uses `new Authorized<T>()` -- established precedent
+
+## Mistakes to Avoid
+
+- Do NOT use `new Authorized<T>(default)` -- causes CS0121 ambiguity between `Authorized(Authorized)` and `Authorized(T?)` constructors
+- Use `new Authorized<T>()` (parameterless) instead -- unambiguous and matches existing pattern
+
+## User Corrections
+
+- None
+
+## Developer Review
+
+**Status:** Approved
+**Date:** 2026-03-21
+
+(See plan file for full assertion trace table)
+
+## Implementation Contract
+
+(See plan file for full contract)
+
+## Implementation Progress
+
+All milestones complete:
+1. Fixed read-style bool-false return (Site 1)
+2. Fixed write-style bool-false return (Site 2)
+3. Resolved CS0121 constructor ambiguity
+4. Added FetchAuthAllow auth class and 3 test targets
+5. Added 4 integration tests (scenarios 1, 2, 3, 5)
+6. All 1984 tests pass, 0 failures
+
+## Completion Evidence
+
+- All tests pass across net9.0 and net10.0
+- All 4 new tests pass
+- All existing tests pass (no regression)
+- Plan status set to "Awaiting Verification"
+
+### Files Modified
+
+1. `src/Generator/Renderer/ClassFactoryRenderer.cs` -- Fixed two bug sites
+2. `src/Tests/RemoteFactory.IntegrationTests/TestTargets/FactoryRoundTrip/RoundTripTargets.cs` -- Added FetchAuthAllow + 3 test targets
+3. `src/Tests/RemoteFactory.IntegrationTests/FactoryRoundTrip/RemoteFetchRoundTripTests.cs` -- Added 4 integration tests

docs/plans/completed/fix-factory-fetch-nre-on-not-found.memory/requirements-reviewer.md

@@ -0,0 +1,54 @@
+# Requirements Reviewer -- Fix Factory Fetch NRE on Not Found
+
+Last updated: 2026-03-21
+Current step: Post-implementation verification complete
+
+## Key Context
+
+This is a bug fix in the source generator's `ClassFactoryRenderer.cs`. The bug: when a `[Fetch]` method returns `false` (entity not found) and the factory has `[AuthorizeFactory<T>]`, the generated local method returned `default!` which is `null` for `Authorized<T>`. The public wrapper then dereferences `.Result` on null, causing NRE. The fix emits `new Authorized<T>()` instead of `default!` when `HasAuth` is true, at two sites (read-style line 462, write-style line 904).
+
+The developer deviated from the plan's `new Authorized<T>(default)` to `new Authorized<T>()` due to CS0121 constructor ambiguity between `Authorized(Authorized)` and `Authorized(T?)`. The parameterless constructor produces identical runtime state (`HasAccess = false`, `Result = default(T)`) and is already the established pattern in the save routing default at `RenderSaveLocalMethod` line 1087.
+
+## Mistakes to Avoid
+
+None encountered.
+
+## User Corrections
+
+None.
+
+## Requirements Verification
+
+**Verdict:** REQUIREMENTS SATISFIED
+**Date:** 2026-03-21
+
+### Compliance Table
+
+| # | Requirement | Source | Status | Notes |
+|---|------------|--------|--------|-------|
+| 1 | Bool-return Fetch contract: `false` means not found, factory returns `null` | `docs/attributes-reference.md:109`, `skills/RemoteFactory/references/factory-operations.md:150-166` | Satisfied | Fix ensures local method returns non-null `Authorized<T>` wrapper with null `Result`, so public wrapper's `.Result` returns `null` instead of throwing NRE |
+| 2 | Auth failure behavior: Create/Fetch return null when auth fails | `src/Design/Design.Domain/Aggregates/AuthorizedOrder.cs:54-58` | Satisfied | The not-found path now produces the same consumer-visible result (null) as the auth-denied path |
+| 3 | `Authorized<T>` constructor semantics | `src/RemoteFactory/Authorized.cs:80-82` (parameterless), base class lines 19-22 | Satisfied | `new Authorized<T>()` sets `HasAccess = false`, `Result = null` (default for reference types). Public wrapper `.Result` returns null correctly |
+| 4 | `[Remote]` only for aggregate root entry points | `src/Design/CLAUDE-DESIGN.md` Critical Rule 1 | Satisfied | New test targets use `[Remote]` correctly on their Fetch methods (they are aggregate root entry points) |
+| 5 | `[AuthorizeFactory<T>]` usage pattern | `src/Design/Design.Domain/Aggregates/AuthorizedOrderAuth.cs:84` | Satisfied | `FetchAuthAllow` uses `[AuthorizeFactory(AuthorizeFactoryOperation.Fetch)]` matching the Design project pattern |
+| 6 | Properties need public setters | `src/Design/CLAUDE-DESIGN.md` Critical Rule 5 | Satisfied | All test target properties use public setters |
+| 7 | `partial` keyword required | `src/Design/CLAUDE-DESIGN.md` Critical Rule 6 | Satisfied | All three new test targets are `partial` classes |
+| 8 | Existing non-auth bool-false tests unaffected | `RemoteFetchRoundTripTests.cs:53-109` | Satisfied | The `else` branches (lines 477-485 and 919-927) preserve the original `default!` behavior for non-auth cases |
+| 9 | Write-style path also fixed | `ClassFactoryRenderer.cs:894-928` | Satisfied | Identical `method.HasAuth` conditional applied at write-style site |
+| 10 | No Design Debt violation | `src/Design/CLAUDE-DESIGN.md` Design Debt table | Satisfied | This fix does not implement any deliberately deferred feature |
+
+### Unintended Side Effects
+
+1. **Write-style bool-false + auth path**: The fix causes `new Authorized<T>()` with `HasAccess = false` to flow through the save public wrapper, which throws `NotAuthorizedException` (line 1043-1045). This is a change from NRE to `NotAuthorizedException`. The plan documents this as a known semantic difference (Risks section) and explicitly scopes it as acceptable -- the save-with-bool-return + auth combination is untested and likely unused. The previous behavior (NRE crash) was strictly worse.
+
+2. **No effect on existing authorization tests**: All existing `[AuthorizeFactory<T>]` test targets use void-returning Fetch methods, not bool-returning. The fix only changes behavior when both `IsBool` and `HasAuth` are true, so existing auth tests are unaffected.
+
+3. **No effect on serialization contracts**: The `Authorized<T>` wrapper is created and consumed entirely within the generated local method and public wrapper on the server side. For Fetch operations, the public wrapper extracts `.Result` (null) before any serialization occurs. The null value serializes correctly through the existing transport.
+
+4. **No effect on Design project tests**: The Design projects use void-returning Fetch methods and are not affected by this change.
+
+5. **No effect on published documentation accuracy**: The documentation at `docs/attributes-reference.md:109` states "Returns `bool` or `Task<bool>` -- `false` means not found (factory returns `null`)." The fix restores compliance with this documented contract. No doc updates needed.
+
+### Issues Found
+
+None. The implementation correctly addresses both bug sites, follows the established `new Authorized<T>()` pattern already used in the save routing default, and adds comprehensive test coverage for the previously untested bool-false + authorization intersection.