diff --git a/docs/research/2026-08-12-fa2-review-mechanism.md b/docs/research/2026-08-12-fa2-review-mechanism.md new file mode 100644 index 00000000..a0f91e6c --- /dev/null +++ b/docs/research/2026-08-12-fa2-review-mechanism.md @@ -0,0 +1,478 @@ +# Campaign FA slice FA2 — mechanism-faithfulness review + +**Reviewer lens:** mechanism faithfulness (retail behavior vs. shipped code). +**Target:** `1c401048` (Core.Net), `369729f0` (Runtime), `cced83b4` (App/Headless), +`12053e61` (docs/ledger), against +`docs/plans/2026-08-11-fellowship-allegiance-campaign.md` slice FA2 + D2/D4, +lanes B/C/D (`2026-08-11-fa-fellowship-wire.md`, +`2026-08-11-fa-allegiance-wire.md`, `2026-08-11-fa-acdream-seams.md` incl. its +FA1 fix-round addenda), and the J4.x precedent closeouts. +**Date:** 2026-08-12. + +## Verdict + +**APPROVE-WITH-FIXES — 2 MUST-FIX, 6 SHOULD-FIX.** + +Both MUST-FIX findings are on the allegiance owner and they **compound**: the +owner is seeded from a message retail never seeds from (MF-2), and it is never +cleared at the boundary retail clears it at (MF-1). Every fellowship mechanism +in the slice re-derived clean against the decomp. Nothing in the wiring, +command, view, teardown, or register work is wrong. + +Neither MUST-FIX is user-visible today — no FA2 command or view has a +production consumer yet (verified below). They are must-fix because FA3–FA5 +build the panel directly on this owner, and MF-1 is already pinned by a +passing test, which is the "wrong behavior locked in by test" shape FA1's own +review called out when it deleted `AllegianceTree`. + +--- + +## 1. MUST-FIX + +### MF-1 — Allegiance surviving a generation reset contradicts retail, contradicts its own cited precedent, and is pinned by a test + +`RuntimeAllegianceState` participates in no reset stage +(`src/AcDream.Runtime/RuntimeGenerationReset.cs:39-41` says so explicitly; +`src/AcDream.Runtime/Gameplay/RuntimeAllegianceState.cs:18-27` documents the +intent), and `tests/AcDream.Runtime.Tests/RuntimeGenerationResetTests.cs` +(`FellowshipClearsAtResetButAllegianceSurvivesReconnect`) asserts the profile, +the name and `HasServerSeed` all survive `ResetGeneration`. + +**(a) Retail clears it at exactly this boundary.** + +``` +00569fa0 void __fastcall ClientAllegianceSystem::OnEndCharacterSession(...) +00569fa3 return AllegianceProfile::Clear(&this->m_allegianceProfile); +``` +(`docs/research/named-retail/acclient_2013_pseudo_c.txt:375000-375004`, +`symbols.json` `0x00569FA0`.) `AllegianceProfile::Clear @0x005B6A60` +(`:453700-453707`) zeroes `_total_members`, `_total_vassals` and tail-calls +`AllegianceHierarchy::Clear` — the whole tree. + +The sibling hook is the control that proves the boundary is the right one: +``` +005690a0 void __fastcall ClientFellowshipSystem::OnEndCharacterSession(...) +005690a8 if (m_pFellowship != 0) m_pFellowship->vtable->__vecDelDtor(1); +005690b0 this->m_pFellowship = nullptr; +``` +(`:373778-373790`.) **FA2's fellowship half is byte-faithful; the allegiance +half is the exact inverse of what retail does at the same hook.** + +The hook is a per-character-session pair, and it maps 1:1 onto acdream's +generation: `UIQueueManager::OnBeginCharacterSession` fires on +`PlayerDescription` (`0x13`, `@0x0055B173`, `:359247`) and +`UIQueueManager::OnEndCharacterSession` fires from `gmGamePlayUI::~gmGamePlayUI` +(`@0x004EA339`, `:238072`), fanning out to every client system's vtable slot +(`UIQueueManager::OnEndCharacterSession @0x0055A6D0`, `:358492-358501`). + +Corroboration that the cached profile really is per-session state: +`gmAllegianceUI::RecvNotice_AllegianceLogin @0x00492220` (`:158415-158426`) +gates every logon/logoff line on +`AllegianceProfile::GetData(&eax->m_allegianceProfile, guid, …)` — the cached +profile, i.e. the thing `Clear` empties. + +**(b) The cited precedent CLEARS and re-latches — the citation is inverted.** + +`RuntimeAllegianceState.cs:24-27` and lane D §1.3 +(`2026-08-11-fa-acdream-seams.md:144-148`) both justify persistence as "same +class as `RuntimeCharacterOptionsState`'s `HasServerSeed` latch". That +precedent does the opposite: + +```csharp +// src/AcDream.Runtime/Gameplay/RuntimeCharacterState.cs:1007-1017 +public void ResetSession() +{ + Volatile.Write(ref _options1, DefaultOptions1); + Volatile.Write(ref _options2, DefaultOptions2); + Interlocked.Increment(ref _revision); + lock (_dirtyGate) { _isDirty = false; _hasServerSeed = false; } +} +``` + +and its own doc comment (`:999-1006`) names the hazard FA2 is now exposed to +verbatim: *"a stale seed surviving a session boundary could let a flush ship +the PRIOR character's words over the new one's."* It runs every generation via +reset stage `Character = 7` → `RuntimeCharacterState.ResetSession` → +`Try(Options.ResetSession, …)` (`RuntimeCharacterState.cs:442`). + +**(c) The process model does NOT preclude a cross-character reset.** + +The graphical host constructs its connect options with **no character +selector**: + +```csharp +// src/AcDream.App/Composition/SessionPlayerComposition.cs:1127-1132 +new LiveSessionConnectOptions( + d.Options.LiveMode, d.Options.LiveHost, d.Options.LivePort, + d.Options.LiveUser ?? string.Empty, d.Options.LivePass ?? string.Empty) +``` + +`LiveSessionConnectOptions.Character` therefore defaults to `null` +(`src/AcDream.Runtime/Session/LiveSessionContracts.cs:16`), so +`LiveSessionController.StartCore` (`:611-618`) falls through +`TrySelectCharacter` (`:841-852`) to `CharacterList.TrySelectFirstAvailable`, +whose only filter is +`IsAvailableActiveIdentity(c) => c.Id != 0 && c.SecondsGreyedOut == 0` +(`src/AcDream.Core.Net/Messages/CharacterList.cs:111-112`) — and ACE derives +`secondsGreyedOut` from `character.DeleteTime` +(`references/ACE/.../GameMessageCharacterList.cs:28`). **Which character enters +world at generation N+1 is resolved fresh from a server-supplied list, and +nothing in `RuntimeAllegianceState` keys on character identity.** The reset +genuinely runs between the two: `StopCore` (`LiveSessionController.cs:676-701`) +always drains the retired scope, whose teardown calls +`Host.ResetSessionState(Generation)` (`:231`), which is the graphical host's +`_domain.Runtime.ResetGeneration(...)` hop +(`src/AcDream.App/Net/LiveSessionRuntimeFactory.cs:193`). + +Headless is *not* exposed: `HeadlessSessionHost` requires exactly one selector +field (`MapCharacterSelector`, `:830-835`; a fully-null selector makes +`selectorCount != 1` and `TrySelectCharacter` returns false), so each headless +root is character-pinned for its whole life. The exposure is the graphical +host only — but the graphical host is the one FA3–FA5 target. + +**(d) There is no re-seed that would mask it.** + +ACE pushes `0x0020 AllegianceUpdate` on a tree change +(`AllegianceManager.cs:398`, `Allegiance.cs:361`) or in reply to `0x001F` +(`GameActionAllegianceUpdateRequest.cs:9-22`). FA2 wires no `0x001F` sender to +any lifecycle event (verified in SF/clean §3.4). Retail's own re-seed is +`gmAllegianceUI::RecvNotice_PlayerDescReceived @0x00490D59` → +`Event_UpdateRequest(1)` (lane C §1.2), which acdream does not have yet. So for +a second character with **no** allegiance, character A's `AllegianceName`, +monarch, vassal list, `HasProfile` and `HasServerSeed` stay readable +indefinitely. + +**Recommended fix.** Add `RuntimeGenerationResetStage.Allegiance` calling a new +`RuntimeAllegianceState.ResetSession()` that clears the profile **and** +`_hasServerSeed` (mirroring the options precedent exactly); invert the test; +correct plan §2 D2 and lane D §1.3's inverted precedent citation. Anchor the +missing PlayerDesc-triggered `0x001F(1)` resubscribe to FA5 as a TS row so the +clear does not silently mean "blank until the panel is opened". +If the campaign still wants persistence after seeing this, it needs BOTH a +divergence-register row (CLAUDE.md: same commit as the deviation) AND a +character-identity key on the retained data — persistence with no identity +guard is the failure mode, not the persistence itself. + +### MF-2 — `0x027C AllegianceInfoResponse` must not seed the cached profile; retail's handler is print-only + +`RuntimeAllegianceState.ApplyInfoResponseSelf` +(`RuntimeAllegianceState.cs:101-116`) writes monarch, records, name, totals, +`_hasProfile` and `_hasServerSeed` from `0x027C`. Retail does not. + +The dispatcher unpacks `0x027C` into a **stack-local** profile and destroys it +at return: +``` +006a7470 CM_Allegiance::DispatchUI_AllegianceInfoResponseEvent(...) +006a74cd void var_118; CAllegianceProfile::CAllegianceProfile(&var_118); +006a74f1 AllegianceProfile::UnPack(&var_118, &arg2, …); +006a74ff ClientAllegianceSystem::Handle_Allegiance__AllegianceInfoResponseEvent( + arg1->m_pAllegianceSystem, edi, &var_118); +006a750a AllegianceHierarchy::~AllegianceHierarchy(&var_114); +``` +(`acclient_2013_pseudo_c.txt:686294-686313`.) The handler +(`@0x0056A1D0`, `:375144-375210`) reads only `arg3` and calls +`ClientSystem::AddTextToScroll` — **it never touches +`this->m_allegianceProfile`.** + +Contrast the only path that does seed: +``` +0056a120 ClientAllegianceSystem::Handle_Allegiance__AllegianceUpdate(...) +0056a129 AllegianceProfile::operator=(&this->m_allegianceProfile.vtable, arg2); +``` +(`:375103-375109`.) `0x0020` is the single inbound writer of the cached +profile, which is why lane C describes `0x0020` as the subscribed push and +`0x027C` as a query response. + +**Second-order defect the divergence already carries.** `0x027C` has no rank +field, so `ApplyInfoResponseSelf` deliberately retains the previous `_rank` +(`RuntimeAllegianceState.cs:97-99`). On a client where the only allegiance +message ever received is a self `@allegiance info` response, +`Snapshot.HasProfile == true` with `Snapshot.Rank == 0` — FA5's monarch/self +block would render a ranked character as rank 0 (no title). That state is +unreachable in retail precisely because retail never seeds from `0x027C`. + +**Recommended fix.** Drop `ApplyInfoResponseSelf`, its +`onAllegianceInfoResponseSelf` delegate hole, and the self-gate, leaving the +`@allegiance info` chat path exactly as it is today (the fold itself is +correct — see §3.2). If it is kept deliberately, it needs a register row naming +the rank hazard. + +--- + +## 2. SHOULD-FIX + +**SF-1 — The nine new delegate holes are passed unconditionally, so nine event +types silently leave the unhandled-count diagnostic.** +`LiveSessionEventRouter.cs:238-264` passes non-null lambdas +(`update => social.Fellowship?.ApplyFullUpdate(update)` etc.) regardless of +whether `social.Fellowship`/`Allegiance` is supplied, so +`GameEventWiring`'s `if (onX is not null)` guards always pass and the handlers +always register. `GameEventDispatcher.Dispatch` (`:95-117`) only increments +`_unhandledCounts` when there is no registration, so +`GetUnhandledCount(FellowshipFullUpdate)` and its eight siblings now read 0 for +any caller that did not supply an owner (bare-`ChatLog` tests, future +partial hosts) even though the parse result is discarded. Pass the delegates +conditionally, or hoist the null check to the binding site. + +**SF-2 — Disposed check is outside the lock in both new owners (TOCTOU).** +Every mutator does `ObjectDisposedException.ThrowIf(IsDisposed, this)` and then +takes `_gate` separately (`RuntimeFellowshipState.cs:69-71, 94-96, 112-114, +133-135, 148-150, 207-208`; `RuntimeAllegianceState.cs:80-82, 104-106, 132-134, +139-141, 146-148`). `IsDisposed` takes and releases the lock itself, so an +inbound event racing `Dispose` can repopulate `_members`/`_records` after +`_disposed = true`, permanently falsifying `CaptureOwnership().IsConverged` and +failing the J-slice ownership ledger at teardown. The established precedent +checks inside the lock (`RuntimeCommunicationState.cs:377-379`). Inbound events +run on the decode thread and `Dispose` on the host thread, so this is a real +window, currently masked by the router's `accepting` gate. + +**SF-3 — Retail's client-side `RecalculateEvenXPSplitting` is not modelled.** +`Fellowship::UpdateFellow @0x005B9730` calls +`Fellowship::RecalculateEvenXPSplitting(this)` at `@0x005B9785` on every +upsert (`:457071-457098`), as do `AddFellow @0x005B94D5` and +`RemoveFellow @0x005B96EE` (lane B §2.10). FA2 stores `_evenXpSplit` from the +full update and never recomputes it on `ApplyUpdateFellow`/`ApplyQuit`/ +`ApplyDismiss`. FA4's share-percentage column branches on exactly this field +(lane B §2.9). Against ACE it is largely masked — `RemoveFellowshipMember` and +`AddFellowshipMember` both finish with `UpdateAllMembers()`, a FullUpdate to +everyone (`references/ACE/Source/ACE.Server/Entity/Fellowship.cs:179,219-227`) +— but the retail mechanism is absent, and the owner is where retail keeps it. + +**SF-4 — Retail's locked/departed admission gate on the upsert path is not +modelled.** `Fellowship::UpdateFellow` falls through to +`Fellowship::AddFellow @0x005B9480` when the guid is absent (`@0x005B976F`), +and `AddFellow` refuses a guid while `_locked` unless it is in +`_fellows_departed` within 900 s (lane B §2.11, `@0x005B94A5`). FA2's +`ApplyUpdateFellow` always inserts, and `ApplyFullUpdate` discards +`update.Departed` entirely (`RuntimeFellowshipState.cs:67-84` — field 8 of +`0x02BE` per lane B §3.9). The insert-a-new-member branch IS live against ACE +(`AddFellowshipMember` sends `UpdateFellow` to existing members *before* the +FullUpdate, `Fellowship.cs:161-163`), so this is reachable, not theoretical. + +**SF-5 — `AllegianceProfileLookups` is re-implemented rather than reused.** +`RuntimeAllegianceState`'s `TryGetMember`/`TryGetPatron`/`GetVassals` +(`:214-290`) duplicate FA1's `ClientCommandResponses.AllegianceProfileLookups` +(`:250-294`) line for line. They agree today — I diffed both against +`AllegianceHierarchy::GetPatron @0x005B6DD0` (`:453991-454011`) and the §4.4 +reversal rule and both are correct — but two copies of a retail walk is the +shape that drifts. `AllegianceProfileLookups` is `private static`; promoting it +to `internal` (or public) lets Runtime call the one implementation. + +**SF-6 — `RuntimeStateCheckpoint`'s new defaulted snapshots carry null +strings.** `GameRuntimeViews.cs:241-245` defaults `Fellowship`/`Allegiance` to +`default`, whose `Name`/`AllegianceName` are `null` (not `string.Empty` — the +owners' own initialisers are correct, `RuntimeFellowshipState.cs:41`, +`RuntimeAllegianceState.cs:49`). Only test-constructed checkpoints hit it +(`GameRuntimeContractTests.cs:150` is the sole positional site), and the trace +recorder only reads scalars (`GameRuntimeEvents.cs:184-189`), so nothing NREs +today. Worth a non-null default. + +**Observations (no action requested).** `RuntimeFellowMemberSnapshot.ShareLoot` +is exposed as `bool` while lane B §4.1's recommendation is "model as a raw +`uint`" — the predicate is right (`!= 0`, never `== 1`, +`RuntimeFellowshipState.cs:291-293`, pinned by +`ShareLoot_ReadsTheRawWireBitAsNonZero_NeverEqualsOne`) and the raw value is +retained in the owner, so this is a view-shape preference, not a defect. +`IRuntimeFellowshipView` has no member enumeration (only +`TryGetMember(guid)` + `MemberCount`), which FA4's roster list box will need. + +--- + +## 3. What verified clean + +### 3.1 Fellowship lifecycle rules — every rule re-derived against the decomp + +| Rule | FA2 | Retail | +|---|---|---| +| Full update REPLACES | `ApplyFullUpdate` clears then refills, sets `_isInFellowship` (`:67-84`) | `Handle_Fellowship__FullUpdate @0x005693C0`: allocate `CFellowship` if absent, then `Fellowship::operator=` (`:374037-374071`) | +| Incremental UPSERTS by guid | `_members[update.MemberGuid] = update.Member` (`:92-101`) | `Fellowship::UpdateFellow @0x005B9730` → `Fellow::operator=` on hit, `AddFellow` on miss (`:457071-457098`) | +| SELF quit clears the WHOLE snapshot | `ApplyQuit`, `quitterGuid == selfGuid` → `ClearLocked()` (`:110-124`) | `Handle_Fellowship__Quit @0x00569E90`: `arg2 != player_id` → `RemoveFellow`, else `DeleteFellowship` (`:374914-374950`) | +| Other-quit removes ONE | `_members.Remove(quitterGuid)` | same site | +| Dismiss identical to quit | `ApplyDismiss` (`:130-144`) | `Handle_Fellowship__Dismiss @0x00569F10` is byte-identical (`:374953-374985`) | +| Disband always clears | `ApplyDisband` (`:147-151`) | `Handle_Fellowship__Disband @0x00569E70` → `DeleteFellowship` (`:374903-374910`) | + +The `if (!_isInFellowship) return;` guards on the three incremental mutators +are strictly defensive: retail passes a possibly-null `m_pFellowship` straight +into `Fellowship::IsFellow`/`RemoveFellow` with no guard, so the equivalent +retail state faults rather than doing something different. ACE never reaches +it (`Player.LogOut_Inner` → `FellowshipQuit(false)`, +`references/ACE/.../WorldObjects/Player.cs:531-532`, so a reconnect is never +mid-fellowship; and `HandleFellowshipUpdateRequest` sends a FullUpdate before +any vitals stream, `Player_Fellowship.cs:142-149`). + +**The leader hand-off rule is exact.** `RequiresLeaderHandoffBeforeQuit` +(`RuntimeFellowshipState.cs:172-193`) returns true only when +`!disband && _isInFellowship && _leaderGuid == selfGuid` and a non-self member +exists. Retail's Quit button, `gmFellowshipUI::ListenToElementMessage` case 8 +(`:156425-156444`): + +``` +0049034b case 8: +00490353 if (m_pFellowship_1 != 0) +0049035d if (m_pFellowship_1->_leader == GetPlayerID()) +00490361 eax_18 = Fellowship::GetNonLeaderFellowID(m_pFellowship_1); +00490368 if (eax_18 != 0) AssignLeadershipToFellow(this, eax_18); +004903f1 CM_Fellowship::Event_Quit(0); +``` + +and case `0xC` (Disband) is `CM_Fellowship::Event_Quit(1)` with **no** hand-off +(`:156475-156479`). FA2 collapses the two retail buttons into one +`Quit(disband)` and gates the hand-off on `!disband` — behaviourally identical. +`Fellowship::GetNonLeaderFellowID @0x005B9200` (`:456723-456770`) walks the +hash table returning the first key `!= _leader`; FA2 skips `selfGuid`, which +equals `_leaderGuid` on that branch. Equivalent modulo iteration order, which +retail does not define either. + +**Ordering is preserved on both hosts.** Direct sends `0x0290` then `0x00A3` +inline (`DirectGameRuntimeCommandAdapter.cs` `Quit`); App publishes two bus +messages under one lock (`LiveSessionCommandRouter.Publish`, `:293-300`), so +`FellowshipAssignNewLeaderRuntimeCmd` cannot be reordered after +`FellowshipQuitRuntimeCmd`. + +**`IsFull >= 9`** — verified by absence. No FA2 code encodes a fellowship +capacity anywhere (grep for `IsFull` returns only unrelated +`IsFullyConstrained`/`OutsideIsFullScreen`/`IsFullyResident`), and +`RuntimeFellowshipSnapshot` exposes `MemberCount` without an `IsFull`. Lane B +§7.1's byte-decoded `cmp [ecx+0x14],9 / sbb / inc` result has nothing to be +wrong about yet; FA4's `UpdateButtons` port is where it lands. + +### 3.2 The shared `0x027C` registration — the folding claim is correct and the live output is byte-identical + +`GameEventDispatcher.Dispatch` (`:95-117`) is a single +`_handlers.TryGetValue` → `registration.Handler(envelope)`; `Register` +(`:57-76`) writes `_handlers[type] = node`, i.e. it REPLACES. The seam doc +§2.3's "both fire" claim is wrong and the implementer's correction is right — +a literal second `registrar.Register(AllegianceInfoResponse, …)` would have +silently killed the live `@allegiance info` output. + +The folded handler (`GameEventWiring.cs`, `AllegianceInfoResponse` +registration) keeps the pre-existing three lines verbatim — parse, `if (info is +null) return;`, `foreach (FormatAllegianceInfoLines) chat.OnSystemMessage(line, +chatType: 0u)` — and only appends the self-gated callback afterwards. **One +parse feeds both; there is no double-parse divergence and no reordering of the +chat emission.** Pinned by +`WireAll_AllegianceInfoResponse_SelfGated_FiresOnlyForOwnGuid`, which asserts +`chatLines > 0` on both the self and the other-player response and `observed == +null` only on the latter. (Note this verdict is about the *fold*; whether the +callback should exist at all is MF-2.) + +Minor: the gate is `playerGuid is not null && info.Value.TargetGuid == +playerGuid()`, with no `playerGuid() != 0` clause. ACE always writes a real +guid (`GameEventAllegianceInfoResponse.cs:9-11`), so it is unreachable, but a +`!= 0` clause would be free. + +### 3.3 D4's `0x00A6` — present, generation-gated, not fired + +`IRuntimeFellowshipCommands.SetPanelOpen` (`GameRuntimeCommands.cs:341-343`) is +implemented on both adapters (`DirectGameRuntimeCommandAdapter.cs:941`, +`CurrentGameRuntimeCommandAdapter.cs:934`), both behind the generation +`Validate` gate, and reaches `WorldSession.SendFellowshipUpdateRequest` → +`SocialActions.BuildFellowshipUpdateRequest`. A whole-`src` grep for +`SetPanelOpen` finds no caller other than the unrelated +`ToolbarController.SetPanelOpen(uint panelId, bool open)`. **No premature +send.** The same holds for `IRuntimeAllegianceCommands.SetUpdateSubscription` +(`0x001F`) — declared, implemented, uncalled. + +### 3.4 Wiring symmetry + +Both `LiveSocialSessionBindings` construction sites are updated in the same +commit with identical named arguments — +`src/AcDream.App/Net/LiveSessionRuntimeFactory.cs:261-263` +(`Fellowship: _domain.Runtime.FellowshipOwner, Allegiance: +_domain.Runtime.AllegianceOwner`) and +`src/AcDream.Headless/Hosting/HeadlessSessionHost.cs:785-787` +(`Fellowship: Runtime.FellowshipOwner, Allegiance: Runtime.AllegianceOwner`). +There is still exactly one `GameEventWiring.WireAll` call site +(`LiveSessionEventRouter.cs:234`), so both hosts get the same registration. + +The self-guid used for the quit/dismiss self-vs-other split resolves to the +same owner on every path: the router uses `inventory.PlayerGuid()`, bound to +`() => _player.Identity.ServerGuid` in App +(`LiveSessionRuntimeFactory.cs:277`) and +`() => Runtime.PlayerIdentity.ServerGuid` in Headless +(`HeadlessSessionHost.cs:745`); Direct uses +`_runtime.PlayerIdentity.ServerGuid`; App's adapter uses +`_view.Lifecycle.PlayerGuid`, which `GameRuntime` fills from +`PlayerIdentity.ServerGuid` (`GameRuntime.cs:484-488`). One source. + +The two adapters' zero-guid rejection paths look different +(`EmitUnsupported(...,Rejected,guid)` vs `EmitResult(...,Rejected,guid)`) but +are semantically identical — both call +`EventSink.EmitCommand(domain, op, status, primaryObjectId)` and return +`Result(status, primaryObjectId)`; `EmitResult` merely also passes +`text: null` (`DirectGameRuntimeCommandAdapter.cs:1355-1383`). + +`IRuntimeEventObserver` is untouched — the `369729f0` diff of +`GameRuntimeEvents.cs` adds only two `RuntimeCommandDomain` ordinals and six +trace-string lines; the interface (`:100-118`) is unchanged, so the five bot +policies and the trace recorder still compile. This honours D2. + +The Runtime-only assembly guard still holds: `GameRuntimeTests.cs:174-181` +asserts no `AcDream.App`/`Silk.NET` assembly loads, and both new owners live in +`AcDream.Runtime` depending only on `AcDream.Core.Net.Messages` types. +`GameRuntimeContractTests.cs` gained both new types to its owned-type list. + +### 3.5 TS-81 + +The register row is honest and the mechanism claim is verifiable. At +`gmAllegianceUI::RecvNotice_AllegianceLogin @0x00492220` the two candidate +strings really do resolve through mislabelled vtable-slot symbols: + +``` +00492278 if (arg3 == 0) +00492281 var_48_1 = &gmAllegianceUI::`vftable'.RecvNotice_PrevSpellTab; +00492278 else +0049227a var_48_1 = &gmAllegianceUI::`vftable'.RecvNotice_UpdateSpellComponents; +00492286 AC1Legacy::PStringBase::PStringBase(&arg3, var_48_1); +``` +(`:158434-158439`) — a `PStringBase` constructed from what BN typed as +two unrelated vtable entries. Refusing to invent English here is correct. +`ApplyLoginNotification` does bump the revision +(`RuntimeAllegianceState.cs:131-135`), pinned by +`Revision_IsMonotonicAcrossEveryEventKind`. TS-80's narrowing is also accurate: +`Create(gen, name, shareXp)` takes the flag but no caller reads +`FellowshipShareXP` into it. + +### 3.6 Snapshot immutability, revision monotonicity, J-owner template + +Snapshots are `readonly record struct`s of scalars plus member value-copies; +the parsed `IReadOnlyList` stored by +`ApplyUpdate`/`ApplyInfoResponseSelf` is a freshly allocated list per parse +(`ClientCommandResponses.ReadAllegianceProfileBody`, `:518-519`) held by no one +else, so there is no aliasing path back into the owner. Every mutator bumps a +monotonic `long`; the no-change paths (`ClearLocked` when already empty, +`Remove` returning false) deliberately do not, which is correct for a poll +contract. + +The 8-edit template is complete and the teardown maths is right: +`GameRuntimeTeardownStage` renumbers `IdentityDisposed` `1<<9→1<<11` and +`EntityObjectsDisposed` `1<<10→1<<12` (in-memory flags only — no serialized +consumer), `TeardownStageCount` `11→13`, and the stage masks at +`GameRuntime.cs:716-735` compose correctly, with `10`/`11`/`12` expressed as +`Complete & ~…` of exactly the not-yet-run stages. `RuntimeGenerationResetStage` +stays contiguous (`Fellowship = 12` before `BeginEntityRetirement = 13`), which +`Advance`'s `state.Stage++` requires. Both fault-injection construction points +were added and are covered. + +### 3.7 Test run + +`dotnet test tests/AcDream.Runtime.Tests --no-build -c Release --filter +"FullyQualifiedName~Fellowship|FullyQualifiedName~Allegiance|FullyQualifiedName~GenerationReset"` +→ **43 passed, 0 failed, 0 skipped** on the committed Release binaries. The +ledger's 13,201/4/0 full-suite claim was not independently re-measured (no +build permitted for this review). + +--- + +## 4. Disposition summary + +| # | Severity | Finding | +|---|---|---| +| MF-1 | MUST-FIX | Allegiance survives generation reset; retail clears at `OnEndCharacterSession`, the cited precedent clears, the graphical host can enter a different character in the same process, and the behavior is test-pinned | +| MF-2 | MUST-FIX | `0x027C` seeds the owner; retail's handler is print-only over a stack temporary. Carries a stale-`Rank` second-order defect | +| SF-1 | SHOULD-FIX | Unconditional delegate holes remove 9 event types from the unhandled-count diagnostic | +| SF-2 | SHOULD-FIX | Disposed check outside the lock in both owners (ownership-ledger convergence risk) | +| SF-3 | SHOULD-FIX | `RecalculateEvenXPSplitting` not modelled on upsert/removal | +| SF-4 | SHOULD-FIX | Locked/departed admission gate + `0x02BE` field 8 not modelled | +| SF-5 | SHOULD-FIX | `AllegianceProfileLookups` duplicated in Runtime instead of reused | +| SF-6 | SHOULD-FIX | Defaulted checkpoint snapshots carry null `Name`/`AllegianceName` |