fix(core): a composed Delegate carries its concrete type (#2271, SR-AUD-118)

Combine and Remove accepted operands of unrelated concrete types and always
produced a base Delegate. .NET refuses the mismatch with ArgumentException
('Delegates must be of the same type.') and preserves the type.

THE FINDING'S TWO HALVES REALLY WERE INSEPARABLE, for the reason it gave: a
same-type guard alone breaks Combine(Combine(a, b), c), because step one returns
a multicast whose own typeid is Delegate rather than the operands' type, so step
two would compare Delegate against C and reject a combination .NET accepts.

BUT NO DATA MEMBER WAS NEEDED. The design the ticket priced assumed the type had
to be stored; it does not. A multicast delegate's type IS the type of its
entries, and Combine itself guarantees they are uniform because it refuses to
build a mixed list -- so the type is read from the invocation list, and
sizeof(Delegate) is unchanged. This is not an SA-3 change.

The null ordering is .NET's and is asserted: it checks types inside CombineImpl,
which a null 'a' never reaches, so Combine(nullptr, b) returns b unchecked.
RemoveAll inherits the check because it is defined in terms of Remove, here as
in .NET.

The ticket expected two green fixtures to need rewriting and NONE did -- the
gate was green before a single test was added. What the derivation-removing
mutation breaks is three pre-existing MulticastDelegateTests, which is the
evidence it is load-bearing rather than decorative.

Three mutations, all caught. Gate 17,291 run, 0 failed. Downstream: zero
Combine/Remove sites in either consumer.

Author: robertvokac

CLAUDE.md

@@ -0,0 +1,70 @@
+<!-- SPDX-License-Identifier: MIT -->
+<!-- Copyright (c) Robert Vokac and contributors -->
+
+# Migration — a composed `Delegate` carries its concrete type (ticket #2271)
+
+*2026-08-18.* `Delegate::Combine` and `Delegate::Remove` accepted operands of unrelated concrete
+types and always produced a base `Delegate`. .NET refuses the mismatch and preserves the type.
+
+Landed under SA-5 on the user's decision of the same date. A **narrowing**: two spellings that
+compiled and ran now throw. **No signature, layout, vtable or `noexcept` change** — and, notably,
+**no data member**: `sizeof(Delegate)` is unchanged.
+
+---
+
+## 1. What changed
+
+| Call | Was | Is |
+|---|---|---|
+| `Combine(alpha, beta)` — different derived types | accepted, produced a base `Delegate` | **`ArgumentException`**, *"Delegates must be of the same type."* |
+| `Remove(combined, foreign)` | returned the source unchanged | **`ArgumentException`** |
+| `RemoveAll(combined, foreign)` | returned the source unchanged | **`ArgumentException`** |
+| `Combine(Combine(a, b), c)` — all one type | worked | **works** |
+| `Combine(nullptr, b)`, `Combine(b, nullptr)` | returned `b` | **unchanged** |
+| `Remove(nullptr, v)`, `Remove(s, nullptr)` | — | **unchanged** |
+| anything using plain `Delegate` on both sides | — | **unchanged** |
+
+The message is `MulticastDelegate.CoreCLR.cs:212-220` and `Delegate.cs:158-169` transcribed;
+`Strings.resx:310-312` gives the sentence verbatim.
+
+## 2. Why both halves had to land together
+
+The ticket recorded that a same-type guard **alone** would break the chained form
+`Combine(Combine(a, b), c)`: step one returns a multicast whose own `typeid` is `Delegate` rather
+than the operands' type, so step two would compare `Delegate` against `C` and reject a
+combination .NET accepts. That is why the finding could not be split.
+
+## 3. No data member was needed
+
+A multicast delegate's type **is** the type of its entries — and `Combine` itself guarantees they
+are uniform, because it refuses to build a mixed list. So the type can be **read** from the
+invocation list rather than stored beside it:
+
+```cpp
+const std::type_info& effectiveDelegateType(const Delegate& d, const InvocationList& list) {
+    return list.empty() ? typeid(d) : typeid(*list.front());
+}
+```
+
+`sizeof(Delegate)` is unchanged and this is not an SA-3 change. A mutation that makes a multicast
+report its own `typeid` instead breaks the chained-form test **and three pre-existing
+`MulticastDelegateTests`**, which is what shows the derivation is load-bearing rather than
+decorative.
+
+## 4. The null ordering is .NET's, deliberately
+
+.NET checks the types *inside* `CombineImpl`, which a null `a` never reaches — so
+`Combine(nullptr, b)` returns `b` **unchecked**. `Remove`'s check likewise runs after both null
+tests. Both orderings are asserted so they cannot drift.
+
+## 5. To migrate
+
+If two delegates in your code have different concrete types and you were combining them, that was
+never meaningful — the result could only be invoked through the base. Give them a common type, or
+keep them in separate delegates. Plain `Delegate` instances all share one type, so the narrowing
+bites only across two **different derived** types.
+
+## 6. Downstream, measured
+
+Neither `cna` nor `mobile-eggbert` calls `Delegate::Combine`, `Delegate::Remove` or
+`Delegate::RemoveAll` — **zero sites in both**. Neither repository was modified.

audit/AUDIT_FINDINGS_INDEX.md

@@ -2,8 +2,10 @@
 // Copyright (c) Robert Vokac and contributors
 // Portions based on .NET runtime API (MIT License, Copyright .NET Foundation and Contributors)
 #include "System/Delegate.hpp"
+#include "System/ArgumentException.hpp"
 #include "System/NotImplementedException.hpp"
 #include <functional>
+#include <typeinfo>
 #include <optional>
 
 namespace System {
@@ -122,11 +124,40 @@ std::any Delegate::DynamicInvoke(const std::vector<std::any>&) {
     throw NotImplementedException("DynamicInvoke is not supported in sharp-runtime");
 }
 
+namespace {
+
+// #2271 / SR-AUD-118. .NET's Delegate carries its own runtime type, and both CombineImpl
+// (`MulticastDelegate.CoreCLR.cs:212-220`) and Remove (`Delegate.cs:158-169`) refuse operands
+// whose types differ, with ArgumentException(SR.Arg_DlgtTypeMis).
+//
+// THE TWO HALVES OF THE FINDING ARE INSEPARABLE, and this function is why. A same-type guard
+// alone would break the chained form Combine(Combine(a, b), c): step one returns a multicast
+// Delegate, whose own `typeid` is `Delegate` and not the operands' type, so step two would
+// compare `Delegate` against `C` and reject a combination .NET accepts.
+//
+// NO DATA MEMBER IS NEEDED TO CARRY THE TYPE. A multicast delegate's type is the type of its
+// entries -- which Combine itself guarantees are all the same -- so it can be READ from the list
+// rather than stored beside it. A leaf delegate's type is simply its own. `sizeof(Delegate)` is
+// unchanged and this is not an SA-3 change.
+const std::type_info& effectiveDelegateType(const Delegate& d,
+                                            const std::vector<std::shared_ptr<Delegate>>& list) {
+    return list.empty() ? typeid(d) : typeid(*list.front());
+}
+
+}  // namespace
+
 std::shared_ptr<Delegate> Delegate::Combine(
         std::shared_ptr<Delegate> a, std::shared_ptr<Delegate> b) {
+    // .NET checks the types inside CombineImpl, which a null `a` never reaches -- so
+    // Combine(nullptr, b) returns b unchecked, and this ordering is deliberate.
     if (!a) return b;
     if (!b) return a;
 
+    if (effectiveDelegateType(*a, a->invocationList_) !=
+        effectiveDelegateType(*b, b->invocationList_)) {
+        throw System::ArgumentException("Delegates must be of the same type.");
+    }
+
     std::vector<std::shared_ptr<Delegate>> combined;
 
     const auto& la = a->invocationList_;
@@ -152,6 +183,14 @@ std::shared_ptr<Delegate> Delegate::Remove(
     if (!source) return nullptr;
     if (!value)  return source;
 
+    // `Delegate.cs:166-167`: the SAME check as Combine's, and it runs AFTER the two null tests.
+    // RemoveAll inherits it, because it is defined in terms of Remove here exactly as it is in
+    // .NET (`Delegate.cs:172-183`).
+    if (effectiveDelegateType(*source, source->invocationList_) !=
+        effectiveDelegateType(*value, value->invocationList_)) {
+        throw System::ArgumentException("Delegates must be of the same type.");
+    }
+
     const auto& sl = source->invocationList_;
     if (sl.empty()) {
         // Single-target delegate

docs/Migration-DelegateTypeIdentity.md

@@ -2,6 +2,7 @@
 // Copyright (c) Robert Vokac and contributors
 // Portions based on .NET runtime API (MIT License, Copyright .NET Foundation and Contributors)
 #include <gtest/gtest.h>
+#include "System/ArgumentException.hpp"
 #include "System/Delegate.hpp"
 #include "System/NotImplementedException.hpp"
 
@@ -304,3 +305,111 @@ TEST(DelegateTests, GetTargetProperty_AlwaysNull) {
     auto d = std::make_shared<Delegate>([]{});
     EXPECT_EQ(d->getTargetProperty(), nullptr);
 }
+
+// ---------------------------------------------------------------------------
+// #2271 / SR-AUD-118 — a composed delegate carries its concrete type
+// ---------------------------------------------------------------------------
+//
+// .NET's CombineImpl (`MulticastDelegate.CoreCLR.cs:212-220`) and Delegate.Remove
+// (`Delegate.cs:158-169`) both refuse operands whose runtime types differ, with
+// ArgumentException(SR.Arg_DlgtTypeMis) — "Delegates must be of the same type."
+//
+// THE FINDING'S TWO HALVES ARE INSEPARABLE, and these tests are why. A same-type guard alone
+// would break the chained form Combine(Combine(a, b), c): step one returns a multicast Delegate,
+// whose own typeid is `Delegate` rather than the operands' type, so step two would compare
+// `Delegate` against `C` and reject a combination .NET accepts. The repair reads a multicast's
+// type from its ENTRIES — which Combine itself guarantees are uniform — so no data member was
+// needed and sizeof(Delegate) is unchanged.
+
+namespace {
+
+class AlphaDelegate : public Delegate {
+public:
+    using Delegate::Delegate;
+};
+
+class BetaDelegate : public Delegate {
+public:
+    using Delegate::Delegate;
+};
+
+}  // namespace
+
+TEST(DelegateTypeIdentityTests, Fix2271_CombiningDifferentConcreteTypesIsRejected) {
+    auto alpha = std::make_shared<AlphaDelegate>([] {});
+    auto beta  = std::make_shared<BetaDelegate>([] {});
+
+    EXPECT_THROW((void)Delegate::Combine(alpha, beta), System::ArgumentException);
+    EXPECT_THROW((void)Delegate::Combine(beta, alpha), System::ArgumentException);
+
+    try {
+        (void)Delegate::Combine(alpha, beta);
+        ADD_FAILURE() << "expected ArgumentException";
+    } catch (const System::ArgumentException& e) {
+        // .NET's own sentence, transcribed rather than paraphrased.
+        EXPECT_STREQ(e.what(), "Delegates must be of the same type.");
+    }
+}
+
+TEST(DelegateTypeIdentityTests, Fix2271_TheChainedFormStillWorks) {
+    // THE ROW THE FINDING SAID COULD NOT BE SATISFIED BY A GUARD ALONE. Step one returns a
+    // multicast whose own typeid is `Delegate`; if that were the type compared in step two, this
+    // would throw.
+    int calls = 0;
+    auto a = std::make_shared<AlphaDelegate>([&] { ++calls; });
+    auto b = std::make_shared<AlphaDelegate>([&] { ++calls; });
+    auto c = std::make_shared<AlphaDelegate>([&] { ++calls; });
+
+    std::shared_ptr<Delegate> ab;
+    ASSERT_NO_THROW(ab = Delegate::Combine(a, b));
+    std::shared_ptr<Delegate> abc;
+    ASSERT_NO_THROW(abc = Delegate::Combine(ab, c));
+    ASSERT_NE(nullptr, abc);
+    abc->Invoke();
+    EXPECT_EQ(3, calls);
+
+    // ...and the composed delegate really does carry the type, rather than merely tolerating the
+    // second step: adding a foreign operand to it is still refused.
+    auto foreign = std::make_shared<BetaDelegate>([] {});
+    EXPECT_THROW((void)Delegate::Combine(abc, foreign), System::ArgumentException);
+    EXPECT_THROW((void)Delegate::Combine(foreign, abc), System::ArgumentException);
+}
+
+TEST(DelegateTypeIdentityTests, Fix2271_RemoveAndRemoveAllRefuseAForeignType) {
+    // `Delegate.cs:166-167` applies the SAME check to Remove, and RemoveAll inherits it because
+    // it is defined in terms of Remove — in .NET and here alike.
+    auto a = std::make_shared<AlphaDelegate>([] {});
+    auto b = std::make_shared<AlphaDelegate>([] {});
+    auto combined = Delegate::Combine(a, b);
+    auto foreign = std::make_shared<BetaDelegate>([] {});
+
+    EXPECT_THROW((void)Delegate::Remove(combined, foreign), System::ArgumentException);
+    EXPECT_THROW((void)Delegate::RemoveAll(combined, foreign), System::ArgumentException);
+
+    // The same-type removal is untouched.
+    auto afterRemove = Delegate::Remove(combined, b);
+    ASSERT_NE(nullptr, afterRemove);
+    EXPECT_TRUE(afterRemove->getHasSingleTargetProperty());
+}
+
+TEST(DelegateTypeIdentityTests, Fix2271_TheNullOrderingIsDotNetsAndTheBaseTypeIsUnaffected) {
+    // .NET checks the types INSIDE CombineImpl, which a null `a` never reaches, so
+    // Combine(nullptr, b) returns b unchecked. Remove's check likewise runs after both null
+    // tests. The ordering is deliberate and asserted so it cannot drift.
+    auto beta = std::make_shared<BetaDelegate>([] {});
+    EXPECT_EQ(beta, Delegate::Combine(nullptr, beta));
+    EXPECT_EQ(beta, Delegate::Combine(beta, nullptr));
+    EXPECT_EQ(nullptr, Delegate::Remove(nullptr, beta));
+    EXPECT_EQ(beta, Delegate::Remove(beta, nullptr));
+
+    // Plain Delegate instances all share one type, so every pre-existing combination keeps
+    // working. That is the whole of the narrowing: it bites only across two DIFFERENT derived
+    // types, which is exactly .NET's rule.
+    int calls = 0;
+    auto p = std::make_shared<Delegate>([&] { ++calls; });
+    auto q = std::make_shared<Delegate>([&] { ++calls; });
+    auto pq = Delegate::Combine(p, q);
+    ASSERT_NE(nullptr, pq);
+    pq->Invoke();
+    EXPECT_EQ(2, calls);
+}