fix(core): ResolveEventHandler can decline (#2325, SR-AUD-123)

The alias returned a plain std::string -- a TOTAL function, with no way for a
handler to say 'I could not resolve this'. .NET's returns Assembly?
(ResolveEventHandler.cs:8) and null means exactly that, after which the runtime
tries the next handler. Landed under SA-8.

THE EMPTY STRING COULD NOT BE BORROWED FOR IT, and that is the finding rather
than an aesthetic preference: empty ALREADY means 'absent requesting assembly'
elsewhere in ResolveEventArgs, so a documented sentinel would have been
unenforceable by the type -- which is the defect itself, not a repair for it. A
test asserts all three states are distinguishable: resolved to a name, resolved
to an EMPTY name, and unresolved.

THE REVIEW'S OPTION C (decide SR-AUD-103 first and revisit) IS DELIBERATELY NOT
TAKEN, and its own reasoning is why. It objected that this 'would fix the
signature of a delegate nothing calls'. That is true and is the reason to do it
NOW: the shape is decided by the reference, not by what will eventually call it,
and a wrong signature in a shipped alias is harder to change once callers exist.
Measured cost today: one first-party site and zero downstream.

The break is asymmetric, which is what makes it cheap: a WIDENING on the handler
side, because std::string converts implicitly to the optional, so an existing
lambda still binds unedited. Only a caller assigning to std::string breaks.

Fixture set: 24 fixtures / 158 sites. Downstream #2371: zero sites in either
consumer. Gate 17,302 run, 0 failed.

Author: robertvokac

CLAUDE.md

@@ -134,7 +134,7 @@ implementation ticket.
 | [SR-AUD-120](modules/core/src/System/Delegate.cpp.audit.md#sr-aud-120--medium--delegateremove-cannot-remove-a-multicast-values-last-matching-invocation-list-subsequence) | medium | remediated | `Delegate.cpp`, `DelegateTests.cpp` | Remove scans one entry at a time and cannot remove a multi-entry value's final matching subsequence; RemoveAll inherits the behavior. **REMEDIATED (#2270 review, #2273, 2026-08-11, `docs/CoreDelegateCompositionContractPlan.md` §6).** Reproduced exactly, and the *reason* the entry loop cannot match was measured rather than assumed: a single-target entry compared against a multi-entry value is rejected by `Equals` on length alone, so the answer is always "not found". That also fixes the repair's regression surface at **empty** — every reachable multicast-value removal shape returns the source unchanged today, including `remove_same_pointer_subsequence_size=4`, where the source's entries are the *identical* `shared_ptr`s the value holds, so the new branch can only fire where the current answer is "unchanged" and **no existing outcome can change**. `Remove` now branches on whether the value is itself multicast, as `MulticastDelegate.RemoveImpl` does, scanning candidate start positions from the last one backwards and deleting the **last** matching contiguous subsequence. Measured after: `remove_multicast_subsequence_size` and `remove_same_pointer_subsequence_size` both `4 → 2`, `remove_entire_list_null` `0 → 1`, `remove_leaving_one_size`/`is_a` `3/0 → 1/1`, `removeall_subsequence_size`/`null` `4/0 → 0/1` — so `RemoveAll` inherits the fix exactly as the finding says it inherits the fault — while `longer_value_unchanged` and the entire single-target path stay put. Boundaries: a longer value leaves the source unchanged, emptying the list returns `nullptr`, and a one-entry remainder is returned as that entry itself, the convention the existing single-entry path already used. The `vl.size() > sl.size()` guard is load-bearing (without it the unsigned start index underflows); its boundary is pinned by a mutation rather than by removing it, which would be undefined behaviour rather than a measurement. **Independent of SR-AUD-118**: the multi-entry result uses the same base-`Delegate` construction the existing removal path already used, so no new type-loss debt is added against #2271. +17 tests; 4 mutations executed and all caught, 1 documented as deliberately not executed; no signature, layout, vtable, `noexcept` or symbol change. |
 | [SR-AUD-121](modules/core/include/System/EventHandler.hpp.audit.md#sr-aud-121--medium--eventhandler-stores-an-empty-handler-and-defers-failure-to-stdbad_function_call-during-raise) | medium | remediated | `EventHandler.hpp`, `EventHandlerTests.cpp` | An empty `std::function` is stored and later invoked as `std::bad_function_call`, whereas adding a null .NET event delegate is a no-op. |
 | [SR-AUD-122](modules/core/include/System/EventHandler.hpp.audit.md#sr-aud-122--medium--eventhandler-exposes-event-arguments-as-const-and-cannot-represent-net-handlers-that-mutate-their-event-data-object) | medium | confirmed | `EventHandler.hpp`, `EventHandlerTests.cpp` | The callback's `const TEventArgs&` rejects a handler needing mutable event data even though .NET EventHandler passes a mutable event object. **Reviewed 2026-08-11 (#2324); STILL CONFIRMED, nothing implemented — `needs_user`.** Live: `using HandlerType = std::function<void(Object* sender, const TEventArgs& e)>` (`EventHandler.hpp:88`). **Premise correction: the break is on the raise side, not the subscriber side.** Dropping the `const` is asymmetric — existing subscribers taking `const TEventArgs&` keep compiling, because a `T&` argument binds to a `const T&` parameter; what stops compiling is every **raiser** that passes a `const` event-args object to `Raise()`/`Invoke()`, and the args object must become non-`const` there. That is a public source break in a template used by shipped types: `System::Timers::Timer::Elapsed` is `EventHandler<ElapsedEventArgs>`, and `ObservableCollection`, `ReadOnlyObservableCollection` and `XObject` carry aliases of the same shape. The .NET behaviour named (a subscriber mutating event data for acknowledgement/cancellation) is real, but enabling it changes a published API's const-correctness contract. Options priced in #2324. `docs/CoreOwnedFindingsReviews2317.md`. |
-| [SR-AUD-123](modules/core/include/System/ResolveEventHandler.hpp.audit.md#sr-aud-123--medium--resolveeventhandler-has-no-representation-for-nets-null-not-resolved-result) | medium | confirmed | `ResolveEventHandler.hpp`, `SystemEventTests.cpp` | A required `std::string` result cannot represent .NET's nullable Assembly “not resolved” outcome or distinguish it from an empty string without an undocumented sentinel. **Reviewed 2026-08-11 (#2325); STILL CONFIRMED, nothing implemented — `needs_user`.** Live: `std::function<std::string(void*, ResolveEventArgs&)>`, a total function into `std::string`, while empty already means "absent requesting assembly" elsewhere in `ResolveEventArgs`, so empty cannot also mean "unresolved". **Measured consumer surface: none** — outside its own header the alias appears only in `Batch4Tests.cpp`, and it is **not wired to any `AppDomain` resolve API** because those are stubs under SR-AUD-103. Adding the missing state is a **public representation change** to a published alias: `std::optional<std::string>` breaks every handler's return statement, a documented sentinel cannot be enforced by the type (which is the defect itself), and leaving it means documenting the reduction. Which is right depends on whether the assembly-resolution surface will ever be more than a stub, so it is sequenced behind SR-AUD-103 rather than fixing the signature of a delegate nothing calls. `docs/CoreOwnedFindingsReviews2317.md`. |
+| [SR-AUD-123](modules/core/include/System/ResolveEventHandler.hpp.audit.md#sr-aud-123--medium--resolveeventhandler-has-no-representation-for-nets-null-not-resolved-result) | medium | remediated | `ResolveEventHandler.hpp`, `SystemEventTests.cpp` | A required `std::string` result cannot represent .NET's nullable Assembly “not resolved” outcome or distinguish it from an empty string without an undocumented sentinel. **Reviewed 2026-08-11 (#2325); STILL CONFIRMED, nothing implemented — `needs_user`.** Live: `std::function<std::string(void*, ResolveEventArgs&)>`, a total function into `std::string`, while empty already means "absent requesting assembly" elsewhere in `ResolveEventArgs`, so empty cannot also mean "unresolved". **Measured consumer surface: none** — outside its own header the alias appears only in `Batch4Tests.cpp`, and it is **not wired to any `AppDomain` resolve API** because those are stubs under SR-AUD-103. Adding the missing state is a **public representation change** to a published alias: `std::optional<std::string>` breaks every handler's return statement, a documented sentinel cannot be enforced by the type (which is the defect itself), and leaving it means documenting the reduction. Which is right depends on whether the assembly-resolution surface will ever be more than a stub, so it is sequenced behind SR-AUD-103 rather than fixing the signature of a delegate nothing calls. `docs/CoreOwnedFindingsReviews2317.md`. **REMEDIATED (#2325, 2026-08-18) under SA-8.** The alias returned a plain `std::string` — a **total** function, with no way for a handler to say *"I could not resolve this"*; .NET's returns `Assembly?` and null means exactly that. **The empty string could not be borrowed for it**, and that is the finding rather than an aesthetic preference: empty already means *absent requesting assembly* elsewhere in `ResolveEventArgs`, so a documented sentinel would have been unenforceable by the type — the defect itself, not a repair for it. A test asserts all three states are now distinguishable. **The review's option C (decide SR-AUD-103 first) is deliberately not taken, and its own reasoning is why**: it objected that this would fix the signature of a delegate nothing calls, which is exactly the reason to do it now — the shape is decided by the reference, not by what will eventually call it, and one first-party site plus zero downstream is the cheapest moment it will ever have. The break is **asymmetric**: a widening on the handler side, since `std::string` converts implicitly, so only a *caller* assigning to `std::string` breaks. `docs/Migration-ResolveEventHandlerOptional.md`. |
 | [SR-AUD-124](modules/core/include/System/ApplicationId.hpp.audit.md#sr-aud-124--medium--applicationid-replaces-binarynull-aware-identity-fields-with-undocumented-strings-and-skips-required-name-validation) | medium | confirmed | `ApplicationId.hpp`, `ApplicationIdTests.cpp` | Mandatory strings replace a cloned byte[] token and nullable components without an encoding/sentinel policy, and an empty name is accepted instead of rejected. **Reviewed 2026-08-11 (#2290); STILL CONFIRMED - approval boundary recorded (#2291, needs_user).** **A genuine pair with SR-AUD-125**, unlike the adjacency slices earlier in the same batch, and the dependency runs one way: SR-AUD-125's faithful repair needs this finding's representation decision, so it was deliberately not split out. **Both findings reproduce exactly as filed; no premise correction was needed** - worth recording because the two slices before this one each corrected something. Consumer inventory measured: **zero** production consumers (`ApplicationIdentity` is a different type that neither includes nor mentions this one); two test files, 18 cases, in two **different** executables, so the shared suite name is not the one-binary duplication seen on `Void` and `UnitySerializationHolder`. **Routes priced, none selected - the selection is the decision.** Token: a `byte[]`-shaped parameter and getter (source break on the constructor and the getter), an uppercase-hex contract on the existing `std::string` (no signature change but the two tokens the tests use are not hex, so it is a behaviour break dressed as documentation), or opaque text (no break, closes nothing). Optional components: `std::optional` (source break on two getters and the constructor), added predicates (additive, but the constructor still cannot receive the distinction), or one documented representation (no break, closes nothing). **The separable third half - rejecting an empty name - needs no representation decision**, uses the port's existing `ArgumentException::ThrowIfNullOrEmpty` so no message would be invented, and needs no in-repo migration; it was still left to #2291 because it turns an input that currently succeeds into a runtime throw for every consumer while closing nothing, this finding being a conjunction. **What was done anyway:** the header now states that the token is text stored verbatim with **no encoding applied or assumed** so binary key material cannot be round-tripped, that there is **no null/empty distinction** (the empty string represents both), that the name is **not validated**, and what `ToString`/`GetHashCode` actually promise - **no signature, stored type, emitted text or validation changed, so the finding is not closed**. **No test was added**: every assertion the report asks for is an assertion about the disputed model and each would be retired by whichever option is approved (the #2281 reason). A separate latent defect found while reviewing - `GetHashCode()` is `noexcept` yet calls `Version::ToString()`, which allocates, so an allocation failure calls `std::terminate` - is ticket #2292 with no `SR-AUD-*` identifier. No CCF minted; **not** marked design-complete, because the routes were priced but not selected. `docs/CoreApplicationIdIdentityModelDesign.md`. |
 | [SR-AUD-125](modules/core/include/System/ApplicationId.hpp.audit.md#sr-aud-125--medium--applicationidtostring-omits-the-public-key-token-and-uses-a-different-identity-grammar) | medium | confirmed | `ApplicationId.hpp`, `ApplicationIdTests.cpp` | ToString drops the token and differs from .NET's quoted lowercase optional identity grammar, so unequal token values can serialize identically. **Reviewed 2026-08-11 (#2290); STILL CONFIRMED - approval boundary recorded (#2291, needs_user).** **Not independently compatible, and that is measured rather than assumed:** .NET writes `publicKeyToken` as **uppercase hex bytes**, which only has meaning if the token is a `byte[]` - while it is an encoding-less `std::string`, the raw text, hex of the storage bytes and hex of a decoded value are three different strings and nothing in the port says which the caller supplied - and .NET **omits components that are null**, which only has meaning if Culture and ProcessorArchitecture can *be* null, which this port cannot express. Both prerequisites are SR-AUD-124, so the two are held under one decision; splitting this one out would mean either changing the emitted text without reaching parity (a break that buys nothing) or waiting for #2291 anyway. The dependency is one-way - SR-AUD-124 can be decided without touching `ToString` - which is why #2291 separates the options. Finding reproduces exactly as filed: `publicKeyToken_` does not appear in the `ToString` expression, and the emitted grammar is `<name>, Version=<v>, Culture=<c>, ProcessorArchitecture=<a>` with unquoted capitalized keys and both optional components always present. **What was done anyway:** `ToString`'s comment now says it is a counterpart **in role only**, spells out both grammars, and states the consequence plainly - *two ApplicationIds differing only by token produce identical text, so this string does not identify an ApplicationId and must not be used as a manifest identity or an equality proxy*, while `Equals` compares all five fields and does distinguish them. **The emitted text is unchanged, so the finding is not closed.** No test was added, because every assertion the report asks for (exact text, token distinction, optional omission, escaping, byte-to-hex) pins the disputed grammar and would be retired by whichever option is approved. No CCF minted; **not** marked design-complete, because the routes were priced but not selected. `docs/CoreApplicationIdIdentityModelDesign.md`. |
 | [SR-AUD-126](modules/core/include/System/Func.hpp.audit.md#sr-aud-126--medium--func-and-converter-permit-void-result-specializations-that-collapse-the-separate-net-action-category) | medium | confirmed (design-complete) | `Func.hpp`, `Converter.hpp`, their direct tests | Unconstrained result aliases accept `Func<void>` and `Converter<T, void>`, making them type-identical to `Action` forms that .NET deliberately keeps separate because `void` cannot be a generic argument. **Design complete, approval-bound (#2296 review, #2299 `needs_user`, 2026-08-11); still open.** **The report's second prescription is structurally impossible**: "preventing APIs that require .NET parity from accepting the substitute aliases" cannot be satisfied by any alias-based design, since an alias introduces no type - there is only one type, so no declaration can accept `Action` while rejecting `Func<void>`; preserving the category would mean replacing every alias with a distinct class type, a whole-API break. **The first prescription is mechanically available, measured:** a constrained alias-template parameter (`template<NonVoid R> using Func = std::function<R()>;`) is legal C++23 here and makes `Func<void>` fail with `error: template constraint failure ... constraints not satisfied` - and it is a **compile-domain public source break** at every `Func<void>`/`Converter<T, void>` downstream, across all 17 `Func` aliases and both `Converter` declarations. Consumers measured: **zero** production sites name the `Func`/`FuncT*` aliases and **zero** sites anywhere spell a `void` result, which licenses nothing since the headers are public in `Core.Base`. Measured and **not** a defect: `Converter` is declared identically in `System/Converter.hpp` and `System/Action.hpp`, and including both in one translation unit compiles clean under `-Wall -Wextra -Wpedantic -Werror`. **Taken anyway (#2300), true under every outcome:** `Func.hpp` gains the arity-spelling note the report asks for, the `std::bad_function_call` note and a `void` warning carrying the structural reason above; `Converter.hpp` the matching warning and `Action.hpp` a cross-reference. **This does not close the finding** - `Func<void>` still compiles. **Five tests added, none retired**, closing three of the four "other missing assertions" bullets - arities 5-7 and 9-15, empty-target `std::bad_function_call`, target-exception propagation and reference-result preservation; dropping `T11` from `FuncT11`'s signature is caught at compile time by the arity case and by nothing else. **No test asserts `Func<void>` is `Action`**, deliberately, since #2299 may remove it. **Not a family with SR-AUD-128 or SR-AUD-129** - three causes, not one; no CCF minted. `docs/CoreMarshalSlotAndFuncShapePlan.md` §4. |

audit/AUDIT_FINDINGS_INDEX.md

@@ -0,0 +1,72 @@
+<!-- SPDX-License-Identifier: MIT -->
+<!-- Copyright (c) Robert Vokac and contributors -->
+
+# Migration — `ResolveEventHandler` can decline (ticket #2325)
+
+*2026-08-18.* `System::ResolveEventHandler` returned a plain `std::string` — a **total**
+function, with no way for a handler to say *"I could not resolve this"*. .NET's returns
+`Assembly?`.
+
+Landed under `docs/StandingApprovals.md` **SA-8** with SA-2's five conditions. A public
+**signature** change on a published alias.
+
+---
+
+## 1. What changed
+
+| | Was | Is |
+|---|---|---|
+| the alias | `std::function<std::string(void*, ResolveEventArgs&)>` | `std::function<std::optional<std::string>(void*, ResolveEventArgs&)>` |
+| a handler that always succeeds | — | **compiles unchanged** — see §3 |
+| a handler that cannot resolve | had to invent a sentinel | **`return std::nullopt;`** |
+| `std::string s = handler(…);` | compiled | **rejected** |
+
+## 2. Why the empty string could not be used instead
+
+The obvious cheap answer — *let `""` mean unresolved* — was not available, and that is the point
+of the finding rather than an aesthetic preference. **`""` already means something else in this
+API**: `ResolveEventArgs` uses it for an *absent requesting assembly*. A documented sentinel would
+therefore have been unenforceable by the type, which is the defect itself rather than a repair
+for it.
+
+A test asserts all three states are distinguishable: resolved-to-a-name, resolved-to-an-empty-name,
+and unresolved.
+
+## 3. Handlers need no edit; callers might
+
+This is a **widening on the handler side**. `std::string` converts implicitly to
+`std::optional<std::string>`, so an existing lambda still binds:
+
+```cpp
+System::ResolveEventHandler h = [](void*, System::ResolveEventArgs& a) -> std::string {
+    return a.getNameProperty() + ".dll";        // still compiles
+};
+```
+
+What breaks is a **caller**:
+
+```cpp
+// before
+std::string resolved = handler(nullptr, args);
+
+// after
+const auto resolved = handler(nullptr, args);
+if (!resolved.has_value()) { /* the handler declined */ }
+// or, for the old collapsed answer:
+const std::string collapsed = handler(nullptr, args).value_or("");
+```
+
+## 4. Sequencing — deliberate, against the review's suggestion
+
+The review offered option C: *decide SR-AUD-103 first and revisit*, on the grounds that fixing
+this "would fix the signature of a delegate nothing calls".
+
+That is true and is the reason to do it **now**. The shape is decided by the reference, not by
+what will eventually call it, and a wrong signature in a shipped alias is harder to change once
+callers exist. Nothing calls it today, so this is the cheapest moment it will ever have — measured
+at **one** first-party site and **zero** downstream.
+
+## 5. Downstream, measured
+
+Neither `cna` nor `mobile-eggbert` mentions `ResolveEventHandler` — **zero sites in both**. Neither
+repository was modified. The downstream ticket is **#2371**.