From 1375b02322617d406266fa44890e54db1b40fbde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Roland=20B=C3=BCrgi?= Date: Wed, 26 Aug 2026 10:21:33 +0200 Subject: [PATCH] Restore the 8-parameter constructor of LanguageModelCatalogSource MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Description` was added as a ninth primary-constructor parameter (987da5f9e). It had a default, so every source-level gate stayed green — but a record's primary constructor signature is REPLACED, not extended, and a call site that omits optional arguments still emits a call to the full constructor as it stood at ITS compile time. The AI provider modules are compiled separately, live in another repository, and land at runtime under /data/modules, where — per ModuleLandingService — they "bind by simple name, and their contract is API compatibility, not build identity". So the six landed providers kept calling the 8-argument constructor that no longer existed, and the first type initializer to run aborted the host: System.TypeInitializationException: 'MeshWeaver.AI.Anthropic.AnthropicExtensions' ---> System.MissingMethodException: Method not found: 'Void MeshWeaver.AI.LanguageModelCatalogSource..ctor(String, String, Int32, String, String, ImmutableArray`1, Boolean, ProviderKind)' That crash-looped systemorph for 100 minutes across 24 restarts on the roll to rc8.ci.5567. It read as a CD failure the whole way: the old pods kept serving, so every health probe stayed green, and helm simply waited on a pod that could never become ready. The repair is ADDITIVE. My first cut moved Description to an init property, which restored the 8-parameter form by REMOVING the 9-parameter one — the same mistake pointing the other way, and check-record-signatures.py (added 11 hours after the break it would have caught) correctly refused it. So the ninth parameter stays and the eight-parameter form is restored beside it: modules compiled before the change and after it both bind. The overload has no source-level caller by design — its callers are separately compiled assemblies that bind by name at runtime — so it is pinned by LanguageModelCatalogSourceBinaryContractTest rather than by use. That test holds both arities, agreement between them, and a negative control so it cannot pass vacuously; verified by mutation in both directions. Verified: MeshWeaver.AI builds with 0 warnings; MeshWeaver.AI.Test 1305/1305; check-record-signatures.py --base origin/main clean; all six provider modules in MeshWeaver.Plugins compile against this core. Co-Authored-By: Claude Opus 5 (1M context) --- .../BuiltInLanguageModelProvider.cs | 48 ++++++ ...ageModelCatalogSourceBinaryContractTest.cs | 144 ++++++++++++++++++ 2 files changed, 192 insertions(+) create mode 100644 test/MeshWeaver.AI.Test/LanguageModelCatalogSourceBinaryContractTest.cs diff --git a/src/MeshWeaver.AI/BuiltInLanguageModelProvider.cs b/src/MeshWeaver.AI/BuiltInLanguageModelProvider.cs index e4f22b7c2d..045807bfc6 100644 --- a/src/MeshWeaver.AI/BuiltInLanguageModelProvider.cs +++ b/src/MeshWeaver.AI/BuiltInLanguageModelProvider.cs @@ -478,6 +478,31 @@ public void Add(LanguageModelCatalogSource source) /// — there is NO central registry. Consumers (Models settings tab, /// ModelProviderService.CreateProvider) enumerate the live /// . +/// +/// 🚨 The primary constructor of this record is a SHIPPED BINARY CONTRACT — do not add a +/// parameter to it, not even one with a default. Provider modules +/// (MeshWeaver.AI.Anthropic, .OpenAI, .AzureFoundry, .ClaudeCode, +/// .Copilot, .WebSearch) are compiled separately, land at runtime under +/// /data/modules, and — per ModuleLandingService — "bind by simple name, and their +/// contract is API compatibility, not build identity". A call site that omits the optional +/// arguments still emits a call to the FULL primary constructor as it stood at ITS compile time, +/// so adding a parameter REPLACES that signature and every already-landed module fails with +/// MissingMethodException the moment its type initializer runs — which aborts the host. +/// Prefer an init property for new optional state; if a parameter really must be added, +/// the OLD signature has to stay reachable, as the overload below keeps it. +/// +/// This is not hypothetical. Adding Description here as a ninth parameter shipped +/// green through every source-level gate (it had a default, so nothing failed to compile) and +/// then crash-looped systemorph for 100 minutes on the next roll: the landed Anthropic +/// module called the 8-argument constructor that no longer existed. The old pods kept serving, +/// so every health probe stayed green while helm waited on a pod that could never start. +/// +/// And the repair is additive, because REMOVING a signature is the same mistake pointing +/// the other way. Dropping Description back out of the primary constructor would fix +/// every module compiled before the change and break any compiled after it — so the ninth +/// parameter stays, and the eight-parameter form is restored beside it. Both populations bind. +/// (scripts/check-record-signatures.py refuses either direction; it landed eleven hours +/// after the break it would have caught.) /// public record LanguageModelCatalogSource( string SectionName, @@ -490,6 +515,29 @@ public record LanguageModelCatalogSource( ProviderKind Kind = ProviderKind.Api, string? Description = null) { + /// + /// The eight-parameter form every AI provider module compiled before 2026-08-25 binds to. + /// + /// 🚨 Do not delete this. It is not dead code and it has no source-level callers + /// by design — the callers are separately-compiled assemblies in another repository that land + /// at runtime under /data/modules and bind by simple name. Removing it re-breaks them + /// exactly as adding the ninth parameter did, and nothing in this solution would go red. + /// LanguageModelCatalogSourceBinaryContractTest pins it. + /// + public LanguageModelCatalogSource( + string SectionName, + string ProviderName, + int Order, + string? DisplayLabel, + string? DefaultEndpoint, + ImmutableArray DefaultModelIds, + bool RequiresApiKey, + ProviderKind Kind) + : this(SectionName, ProviderName, Order, DisplayLabel, DefaultEndpoint, + DefaultModelIds, RequiresApiKey, Kind, null) + { + } + /// Effective display label — falls back to when not supplied. public string EffectiveLabel => DisplayLabel ?? ProviderName; diff --git a/test/MeshWeaver.AI.Test/LanguageModelCatalogSourceBinaryContractTest.cs b/test/MeshWeaver.AI.Test/LanguageModelCatalogSourceBinaryContractTest.cs new file mode 100644 index 0000000000..38c0d0aeb8 --- /dev/null +++ b/test/MeshWeaver.AI.Test/LanguageModelCatalogSourceBinaryContractTest.cs @@ -0,0 +1,144 @@ +using System.Collections.Immutable; +using System.Linq; +using System.Reflection; +using MeshWeaver.AI; +using Xunit; + +namespace MeshWeaver.AI.Test; + +/// +/// Pins BOTH shipped constructor signatures of as binary +/// contracts. +/// +/// Why a reflection test and not a compile-time one. The provider modules that call +/// these constructors — MeshWeaver.AI.Anthropic, .OpenAI, .AzureFoundry, +/// .ClaudeCode, .Copilot, .WebSearch — live in a DIFFERENT REPOSITORY, are +/// compiled separately, and land at runtime under /data/modules. Per +/// ModuleLandingService they "bind by simple name, and their contract is API compatibility, +/// not build identity", so nothing in this solution compiles against the binaries that will +/// actually make these calls. No source-level gate anywhere can see the break, and the +/// eight-parameter overload correspondingly has no source-level caller — that is expected, not a +/// sign it is dead. +/// +/// The break this exists to prevent, and why it is invisible. A call site that omits +/// the optional arguments still emits a call to the FULL primary constructor as it stood at ITS +/// compile time. Adding a parameter — even one with a default — REPLACES that signature +/// rather than extending it, so every already-landed module raises +/// MissingMethodException the instant its type initializer runs, which aborts the host. +/// Adding Description as a ninth parameter did exactly that: it passed every source gate +/// green (it had a default, so nothing failed to compile) and then crash-looped +/// systemorph for 100 minutes on the next roll — 24 restarts — while the old pods kept +/// serving, so every health probe stayed green and helm waited on a pod that could never start. +/// +/// +/// Both arities are pinned, because removing one is the same mistake reversed. +/// Dropping Description back out would have repaired every module compiled before the +/// change and broken every module compiled after it. So the ninth parameter stayed and the +/// eight-parameter form was restored beside it. scripts/check-record-signatures.py refuses +/// either direction at the diff; these tests hold the same line from inside the assembly, where +/// they still apply to a consumer that never sees the diff. +/// +/// So: if a test here goes red, do NOT update it to match the new signature. That is +/// the one change guaranteed to be wrong — it converts a caught break into a shipped one. Add the +/// new state as an init property, or add an overload preserving the old form. +/// +public class LanguageModelCatalogSourceBinaryContractTest +{ + /// + /// The form every AI provider module compiled before 2026-08-25 binds to. This list is a + /// shipped contract; it may not change. + /// + private static readonly Type[] EightParameterForm = + [ + typeof(string), // SectionName + typeof(string), // ProviderName + typeof(int), // Order + typeof(string), // DisplayLabel + typeof(string), // DefaultEndpoint + typeof(ImmutableArray), // DefaultModelIds + typeof(bool), // RequiresApiKey + typeof(ProviderKind), // Kind + ]; + + /// The current primary constructor, adding Description. + private static readonly Type[] NineParameterForm = + [.. EightParameterForm, typeof(string)]; + + private static ConstructorInfo? CtorFor(Type[] parameters) => + typeof(LanguageModelCatalogSource) + .GetConstructor(BindingFlags.Public | BindingFlags.Instance, parameters); + + [Fact] + public void The_eight_parameter_constructor_still_exists() + { + Assert.True(CtorFor(EightParameterForm) is not null, + "every AI provider module landed before 2026-08-25 calls exactly this constructor and " + + "binds to it by name at runtime. If it was removed, those modules abort the host " + + "with MissingMethodException on their first type initializer — which is what " + + "crash-looped systemorph. It has no source-level caller by design; do not delete it " + + "as dead code"); + } + + [Fact] + public void The_nine_parameter_constructor_still_exists() + { + Assert.True(CtorFor(NineParameterForm) is not null, + "modules compiled after Description was added bind to this form. Removing it to " + + "'undo' the break would simply reverse who crashes — both arities are shipped, so " + + "both stay"); + } + + /// + /// The negative control: the assertions above must be able to fail. A GetConstructor + /// that answered yes to anything would keep this suite green through the exact regression it + /// exists to catch — the same "a discovery that finds nothing must never read as a pass" rule + /// the surface-manifest and route gates follow. + /// + [Fact] + public void A_constructor_shape_that_was_never_shipped_is_absent() + { + var neverShipped = NineParameterForm.Append(typeof(DateTimeOffset)).ToArray(); + + Assert.True(CtorFor(neverShipped) is null, + "if this matched, the assertions that the shipped constructors exist would be vacuous " + + "and could not detect a signature change"); + } + + /// + /// The eight-parameter form must produce the same value as the nine-parameter one with + /// Description omitted — a delegating overload that drifted would be worse than none, + /// because the two populations would silently disagree. + /// + [Fact] + public void The_eight_parameter_form_agrees_with_the_nine_parameter_one() + { + var models = ImmutableArray.Create("claude-opus-4-8"); + + var viaEight = (LanguageModelCatalogSource)CtorFor(EightParameterForm)!.Invoke( + ["Anthropic", "Anthropic", 1, "Anthropic", + "https://api.anthropic.com/v1/messages", models, true, ProviderKind.Api]); + + var viaNine = new LanguageModelCatalogSource( + "Anthropic", "Anthropic", 1, "Anthropic", + "https://api.anthropic.com/v1/messages", models, true, ProviderKind.Api, null); + + Assert.Equal(viaNine, viaEight); + Assert.Null(viaEight.Description); + } + + /// + /// Description stays reachable and settable both ways — the repair must not have cost + /// the feature the ninth parameter was added for. + /// + [Fact] + public void Description_is_settable_positionally_and_through_with() + { + var source = new LanguageModelCatalogSource("Anthropic", "Anthropic") + { + Description = "Claude models, direct or Azure-hosted", + }; + + Assert.Equal("Claude models, direct or Azure-hosted", source.Description); + Assert.Equal("changed", (source with { Description = "changed" }).Description); + } +}