acdream/docs/research/2026-08-12-fa2-review-mechanism.md

# 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<char>::PStringBase<char>(&arg3, var_48_1);
```
(`:158434-158439`) — a `PStringBase<char>` 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<AllegianceMemberRecord>` 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` |

---

# 5. Narrow re-review — 2026-08-12

**Scope:** verify each finding's disposition in the actual fix-round diffs
(`4272ad0e`, `ded23067`, `ed8b3ec9`), spot-verify the blast dispositions, and
check the suite claim. Read-only.

## VERDICT: CLOSED — no reopen. One carry-forward (CF-1) for FA5.

All 2 MUST-FIX and all 6 SHOULD-FIX from this doc are correctly applied, and
every blast disposition I spot-checked holds. Nothing was skipped, and no fix
introduced a new mechanism defect. The two MUST-FIX corrections are real ports,
not suppressions.

## 5.1 MUST-FIX dispositions

**MF-1 — FIXED.** `RuntimeAllegianceState.ResetSession()` (`:192-209`)
delegates to a new `ClearLocked()` that clears the profile **and** drops
`_hasServerSeed`, exactly the `RuntimeCharacterOptionsState.ResetSession`
clear-and-relatch shape. `RuntimeGenerationResetStage.Allegiance = 13` is new
(`RuntimeGenerationReset.cs:39-58`), drained at
`case …Allegiance: Advance(state, _allegiance.ResetSession)` (`:320-322`), and
the owner is constructor-injected with a null check (`:113,148-149`). The enum
stays contiguous (`Allegiance = 13` … `Complete = 22`), which `Advance`'s
`state.Stage++` requires. **Stage enumerations are consistent everywhere:** a
whole-tree grep for `RuntimeGenerationResetStage.` outside its own file returns
only by-**name** references (`LiveSessionResetPlanTests.cs:279`,
`RuntimeGenerationResetTests.cs:121,160`, two doc-comment `<see cref>`s) — no
consumer depends on an ordinal and nothing serializes it, so the +1 shift is
inert. The test is inverted, not deleted:
`FellowshipClearsAtResetButAllegianceSurvivesReconnect` →
`FellowshipAndAllegianceBothClearAtGenerationReset`, now asserting
`!HasProfile`, `!HasServerSeed`, `AllegianceName == string.Empty`,
`MonarchGuid == 0`, `RecordCount == 0`.

One shape worth naming as correct rather than sloppy: `ResetSession` carries
**no** disposed guard, and `ClearLocked` on already-cleared fields computes
`changed == false` so it does not even bump the revision — a true no-op after
`Dispose`. That is right for a *retryable* reset transaction (a throwing guard
could never converge on retry), it matches the
`RuntimeInventoryState.ResetExternalContainer` /
`RuntimeCommunicationState.ResetNegotiatedChannels` precedent, and
`RuntimeFellowshipState.ResetSession` was brought to the same shape in
`ded23067` (blast SF-5). Note this is deliberately the opposite of SF-2's
lock-first *mutator* guard — mutators must reject post-dispose writes, reset
must tolerate them. Both are right.

**MF-2 — FIXED.** `ApplyInfoResponseSelf` is deleted from
`RuntimeAllegianceState`; the `onAllegianceInfoResponseSelf` delegate hole is
deleted from `GameEventWiring.WireAll`; the `TargetGuid == playerGuid()`
self-gate is deleted from the `AllegianceInfoResponse` registration, which is
now the pre-FA2 three-line parse → null-check → `FormatAllegianceInfoLines`
chat emission and nothing else; the router's forwarding lambda is gone. A
whole-tree grep for `ApplyInfoResponseSelf|onAllegianceInfoResponseSelf`
returns **0 hits** in `src/` and `tests/`. The test was rewritten rather than
dropped: `WireAll_AllegianceInfoResponse_SelfGated_FiresOnlyForOwnGuid` →
`…_IsTextOnly_ForSelfAndOtherGuidsAlike`, pinning the chat output firing
unconditionally for both a self query and a by-name query. The stale-`Rank`
second-order defect dies with the seeding path.

## 5.2 SHOULD-FIX dispositions

**SF-1 — FIXED.** All nine delegate holes are now passed conditionally
(`social.Fellowship is { } x ? x.ApplyFullUpdate : null`, etc.,
`LiveSessionEventRouter.cs`), so `GameEventWiring` registers a handler only
when an owner exists and `GameEventDispatcher.GetUnhandledCount` reports
correctly for owner-less callers. Nine, not ten — `onAllegianceInfoResponseSelf`
was removed by MF-2, which is consistent.

**SF-2 — FIXED.** Every mutator in both owners now takes `_gate` first and
checks the plain `_disposed` field inside it
(`RuntimeFellowshipState.ApplyFullUpdate`/`ApplyUpdateFellow`/`ApplyQuit`/
`ApplyDismiss`/`ApplyDisband`; `RuntimeAllegianceState.ApplyUpdate`/
`ApplyLoginNotification`/`ApplyUpdateDone`/`ApplyUpdateAborted`), matching
`RuntimeCommunicationState.cs:377-379`. The decode-thread-vs-host-thread window
that could repopulate a disposed owner and permanently falsify
`CaptureOwnership().IsConverged` is closed.

**SF-3 — FIXED, and the port is faithful.** `RecalculateEvenXpSplit()` is a
line-for-line port of lane B §2.10's transcription of
`Fellowship::RecalculateEvenXPSplitting @0x005B92E0`: early-return on
`!_shareXp` leaving the flag untouched; min/max scan; `_evenXpSplit = true`;
then `if (minLevel < 50) { if (maxLevel > leaderLevel + 5) false;
if (minLevel + 5 < leaderLevel) false; }`. Called from `ApplyUpdateFellow`
after the upsert (retail: `Fellow::operator=` then
`RecalculateEvenXPSplitting`, `@0x005B977E`/`@0x005B9785`) and from
`ApplyQuit`/`ApplyDismiss` on the removal branch only (retail `RemoveFellow
@0x005B96EE`) — **not** from `ApplyFullUpdate`, which is correct and is the
part that could easily have been got wrong: retail's `Fellowship::operator=`
copies the server's authoritative flag and does not recompute. `ClearLocked`
does not recompute either, which is right — a full clear is retail's
`DeleteFellowship`, not `RemoveFellow`.

Two deliberate departures, both flagged in the code and both defensible:
`minLevel` seeds from `uint.MaxValue` rather than retail's `100000` (identical
for any reachable level, and the empty-table case converges to the same
result); and the leaderless-table case returns `_evenXpSplit = true` instead of
replaying retail's unsigned wraparound against the `0xFFFFFFFF` sentinel. I
verified that second one against the cited source rather than trusting the
comment: lane B §7.4 (`2026-08-11-fa-fellowship-wire.md:962-966`) says verbatim
*"treat a leaderless table as 'leave `_even_xp_split` at 1'"*. The citation is
accurate and the lane is the pinned spec.

**SF-4 — FIXED.** `ApplyFullUpdate` now stores `update.Departed` into a
`_fellowsDeparted` dictionary (cleared alongside `_members` in both
`ApplyFullUpdate` and `ClearLocked`), and `ApplyUpdateFellow` gates the
new-guid branch through `IsAdmissibleWhileLocked` — `_fellowsDeparted`
membership plus a 900 s (`0x384`) window, matching `Fellowship::AddFellow
@0x005B94A5`. The gate correctly applies **only** to the "is this guid NEW"
branch; an existing member's refresh is never gated, matching retail's
`UpdateFellow`-hits-the-table path, and a refused guid skips the recompute
exactly as retail's early-returning `AddFellow` does. I confirmed the gate has
real data to work with: FA1's parser does read `0x02BE` field 8
(`GameEvents.cs:693-701` — `[u16 count][u16 buckets]` then
`count × [u32 guid][i32 timestamp]`), so this is not an always-refuse gate.

**SF-5 — FIXED.** `AllegianceProfileLookups` promoted `private` → `internal`
with `<InternalsVisibleTo Include="AcDream.Runtime" />` added to
`AcDream.Core.Net.csproj`; `TryGetMember`/`TryGetPatron`/`GetVassals` now call
`FindData`/`FindPatron`/`FindVassals` and the duplicated `TryGetMemberLocked`
helper is deleted. One implementation of the retail walk again.

**SF-6 — FIXED.** `RuntimeStateCheckpoint`'s `Fellowship`/`Allegiance`
parameters are now required rather than `= default`, and both snapshot types
gained an explicit parameterless constructor seeding `string.Empty`. The stated
reasoning is correct C#: `default(T)` bypasses a struct's parameterless
constructor, and CS1736 forbids a non-constant `new(...)` as an optional
parameter default — so required parameters really were the only way to
guarantee a non-null `Name`/`AllegianceName`.

## 5.3 Blast dispositions (spot-verified)

- **Teardown ledger (blast MF-1) — FIXED, better than the report asked for.**
  Case 9 was genuinely one flag too many (it included `FellowshipDisposed`, the
  stage it had not yet run); it is now
  `Complete & ~Fellowship & ~Allegiance & ~Identity & ~EntityObjects` = 9
  flags. I re-derived the whole table: at `_disposeStage == N` exactly N flags
  are set, for every N in 0..13. The new
  `CompletedTeardownStagesAccumulatesExactlyOneFlagPerStage` reflection walk
  pins **every** intermediate stage (the previous tests only sampled stage 0
  and `Complete`, which is precisely why the off-by-one survived), and its
  closing `Assert.Equal(Complete, expected)` ties the hand-written ordered flag
  list back to the enum so a future owner insertion cannot silently drop or
  duplicate a flag.
- **Router self-guid test (blast SF-4) — FIXED.**
  `FellowshipQuit_RoutesSelfGuidToClearAndOtherGuidToRemove` builds real
  `RuntimeFellowshipState`/`RuntimeAllegianceState` owners, wires them through
  the one production `LiveSessionEventRouter` registration site with
  `PlayerGuid: () => self`, and dispatches real `0x00A3` envelopes for both an
  other-quit (one member removed, still in fellowship) and a self-quit (whole
  snapshot cleared). This was the slice's only non-trivial lambda and it now
  has end-to-end coverage.
- **`ResetSession` guard alignment (blast SF-5) — FIXED**, see MF-1 above.
- **`GetVassals` allocation (blast SF-7) — documented** as an intentional
  exception with the reason (`yield return` is illegal inside a `lock`) and a
  caching hint for FA3. Acceptable disposition for a convention note.
- **Seam-doc addenda (blast SF-1/SF-2) — LANDED and dated.** §1.3's inverted
  `RuntimeCharacterOptionsState` citation, §2.3/§9's "dispatcher permits
  multiple owned handlers", the §8 table's Allegiance row, and the
  executive-summary ownership bullet all carry
  `[FA2 fix-round addendum, 2026-08-12 …]` strikes citing the review docs;
  §2.3 additionally gains the `0x01C9`/`0x01CA` "correctly left unregistered"
  disposition it was missing. The plan's D2, its architecture blurb, and FA2's
  slice-map contract row all carry matching dated corrections, so the
  "allegiance survives reconnect" claim does not survive uncorrected anywhere.
- **Ledger count corrections (blast SF-3) — LANDED:** 11 `Send*` wrappers (not
  12 — `SendAllegianceInfoRequest` pre-dates FA2) and 10 delegate holes (not
  15). I re-counted both against the diffs; both corrected figures are right.
- **Register-row re-evaluation (blast SF-6) — I audited the conclusion and
  AGREE.** With MF-1's clear-at-reset landed, acdream's allegiance lifetime now
  *matches* `ClientAllegianceSystem::OnEndCharacterSession @0x00569FA0` →
  `AllegianceProfile::Clear`, and with MF-2 landed `0x0020 AllegianceUpdate` is
  the sole seeder, matching retail's sole `AllegianceProfile::operator=` writer
  at `@0x0056A129`. The register tracks acdream-vs-retail *deviation*; there is
  no deviation left for a row to name — the same reasoning FA1's D9 addendum
  used for the absent WeenieError strings. **No register row is owed, and no
  other FA2 deviation surfaced during this re-review.** TS-81 (login-notice
  text) and TS-80 (narrowed) remain correct as filed.

## 5.4 Suite claim

**Consistent with the diffs, exactly.** Counting discovered xunit cases
(`[Fact]` + `[InlineData]` rows) at `12053e61` vs the fix-round tip:
`RuntimeAllegianceStateTests` 7 → 9 (**+2**: one `ApplyInfoResponseSelf_…`
deleted, three `ResetSession_*` added); `RuntimeFellowshipStateTests`
10 Facts + 4 InlineData = 14 → 17 Facts + 7 InlineData = 24 (**+10**);
`GameRuntimeTests` **+1**; `LiveSessionEventRouterTests` **+1**;
`RuntimeGenerationResetTests` and `GameEventWiringTests` **+0** (one in-place
rename each). Total **+14**, matching the ledger's 13,201/4/0 → 13,215/4/0
claim. The absolute totals were not independently re-measured (no build
permitted); the delta is exact.

Targeted runs on the post-fix Release binaries (built 02:19, after the last
source edit at 02:06 and the final code commit at 02:17):
`AcDream.Runtime.Tests` filtered to Fellowship/Allegiance/GenerationReset/
GameRuntimeTests/LiveSessionEventRouter — **95 passed, 0 failed, 0 skipped**;
`AcDream.Core.Net.Tests` filtered to GameEventWiring — **67 passed, 0 failed,
0 skipped**.

## 5.5 Carry-forward (FA5, not a reopen)

**CF-1 — nothing re-subscribes allegiance now that the reset clears it, and the
plan's FA5 row names the wrong opcode for the panel-show path.** Retail sends
`Event_UpdateRequest(1)` (`0x001F`) at `gmAllegianceUI::PostInit @0x004911C6`,
at `RecvNotice_PlayerDescReceived @0x00490D59` (i.e. every character-session
start), and on the panel's visible branch `@0x004912DD` (lane C §1.2). acdream
wires `0x001F` to no lifecycle event, so post-reset the owner stays empty until
ACE happens to push `0x0020` on a tree change. Before MF-1 this was masked by
persistence; the fix correctly exposes it. No register row is owed while there
is no allegiance panel at all (the whole surface is FA3/FA5 scope) and no
consumer reads the owner, but FA5 must wire it — and the plan's FA5 row
currently says *"profile parse on show (`0x027B`)"*, which is the by-name
**info request**, not the subscribe toggle. That distinction is now
load-bearing: after MF-2, `0x027B`'s response (`0x027C`) is text-only and no
longer feeds the owner at all, so an FA5 that follows the plan literally would
open a panel with chat text and no data. Correct the plan row to `0x001F`
before FA5 starts.