diff --git a/docs/research/2026-08-12-fa1-review-mechanism.md b/docs/research/2026-08-12-fa1-review-mechanism.md new file mode 100644 index 00000000..4c1a9d46 --- /dev/null +++ b/docs/research/2026-08-12-fa1-review-mechanism.md @@ -0,0 +1,500 @@ +# Campaign FA slice FA1 — MECHANISM-FAITHFULNESS review + +**Date:** 2026-08-12. **Lens:** mechanism faithfulness (is every byte and +every rule right?). **Mode:** read-only; this doc is the only file written. + +**Target:** `7be86f47` (builders), `6bedbc47` (parsers), `5f9aa16f` +(WeenieError), `4281750b` (delete `AllegianceTree`), ledger `ee1124ca`. + +**Contract:** `docs/plans/2026-08-11-fellowship-allegiance-campaign.md` +slice FA1 + D3/D5/D6/D9, with +`docs/research/2026-08-11-fa-fellowship-wire.md` (lane B) and +`docs/research/2026-08-11-fa-allegiance-wire.md` (lane C) as ground truth. + +**Primary sources re-read independently for this review** (not taken on +the lane docs' word): `docs/research/named-retail/acclient_2013_pseudo_c.txt` +(`Fellow::UnPack @0x005B9AD0`, `Fellowship::UnPack @0x005B94F0`, +`PackableHashTable::UnPack @0x00569A90`, +`CM_Fellowship::DispatchUI_UpdateFellow @0x006A6700`, +`CM_Fellowship::Event_Create @0x006A67A0`, +`AllegianceHierarchy::UnPack @0x005B7520`, +`AllegianceHierarchy::Add @0x005B6E90`, +`AllegianceProfile::UnPack @0x005B6B90`, the `0x20` dispatch case at +`0055b5dd`, `ClientCommunicationSystem::HandleFailureEvent @0x00571990`), +`docs/research/named-retail/acclient.h:2979-2994` (`AllegianceVersion`), +`docs/research/named-retail/symbols.json`, and the ACE writers +`references/ACE/Source/ACE.Server/Network/Structure/AllegianceHierarchy.cs`, +`.../AllegianceData.cs`, `.../AllegianceProfile.cs`, +`.../GameEvent/Events/GameEventAllegianceUpdate.cs`. + +--- + +## VERDICT: **APPROVE-WITH-FIXES** + +Every wire layout in this slice is byte-correct against the retail +decompilation, and both premise-contradiction calls are **independently +verified correct from primary source** (§2). The fixes below are two +rule/coverage gaps and a set of smaller fidelity items; none of them +invalidates a landed byte vector, and all of them are unreachable against +ACE today — which is exactly why they need pinning now rather than +discovering them against a non-ACE server or a hand-built FA2 fixture. + +**MUST-FIX: 2. SHOULD-FIX: 5. Nits (non-blocking): 5.** + +--- + +## 1. MUST-FIX + +### MF-1 — `AllegianceHierarchy::Add`'s FOURTH rejection rule (`_id == 0`) is not modeled + +`src/AcDream.Core.Net/Messages/ClientCommandResponses.cs:501-524` models +three of the four rejection rules in +`AllegianceHierarchy::Add @0x005B6E90`. The whole function body in retail +is wrapped in `if (_id != 0)`: + +``` +005b6e96 uint32_t _id = arg3->_id; +005b6e9e if (_id != 0) <-- every success path is inside this + ... +005b6ecd return 0; <-- _id == 0 falls straight out +``` + +and the caller (`005b779d`) turns any `Add` failure into `return 0` from +`UnPack`, i.e. the whole message is discarded. Lane C §4.4's own +pseudo-code lists it first (`if (data._id == 0) return 0; // reject`). + +acdream reads the monarch at `:505` and each record at `:512` with no +`CharacterId != 0` check, so: + +* a profile whose **monarch** record has id 0 is accepted where retail + discards it, and +* every subsequent record naming `treeParent = 0` is then admitted by + `knownIds.Contains(0)` — where retail rejects `treeParent == 0` + unconditionally (`005b6ee5`: `esi = (arg2 != 0) ? Search(...) : nullptr`, + then `esi != 0` is required). + +One guard fixes both: reject the whole message when any record's +`CharacterId == 0`. Add the boundary test beside the existing three at +`tests/AcDream.Core.Net.Tests/Messages/AllegianceProfileVersionGateTests.cs:312-353`. + +Unreachable against ACE (`AllegianceData.cs:69` always writes a real +guid) — the same "correct only because ACE is generous" class lane C §0 +warns about for the version gates, which this slice otherwise closed. + +### MF-2 — `0x001F AllegianceUpdateRequest` builder is missing + +`src/AcDream.Core.Net/Messages/AllegianceRequests.cs:34-83` ships +Swear (`0x001D`), Break (`0x001E`) and the new Kick alias — and nothing +else. There is no `0x001F` builder anywhere in the tree (repo-wide grep +for `0x001F` / `AllegianceUpdateRequest` over `src/**/*.cs` returns only +`PropertyBool.AllegianceUpdateRequest`, an unrelated property id). + +`0x001F` is the allegiance panel's subscribe/unsubscribe toggle — the +exact structural twin of the fellowship `0x00A6` this slice *did* repair +and rename. Lane C §1.2 pins all four retail send sites +(`gmAllegianceUI::PostInit` tail `@0x004911C6` → 1, +`RecvNotice_PlayerDescReceived @0x00490D59` → 1, +`OnVisibilityChanged` visible `@0x004912DD` → 1, hidden `@0x00491311` → 0) +and lane C §7.1 numbers it #3 of the six messages the minimum-viable +panel needs, with the explicit instruction "ACE ignores the value but +retail servers do not; send it correctly." + +FA1 is the Core.Net truth slice and FA2 (Runtime owners + typed commands) +lands before FA5; leaving this out pushes a `AcDream.Core.Net` builder +addition into a Runtime or panel slice. Six lines plus a golden vector +(`[u32 0xF7B1][u32 seq][u32 0x1F][u32 on]`, total `0x10` per lane C §3.2). + +--- + +## 2. The two premise-contradiction calls — BOTH VERIFIED CORRECT + +I re-derived both from primary source rather than accepting the commit +messages. Both stand. + +### (a) The 8 fellowship `WeenieError` ids genuinely have no retail text — `5f9aa16f` is right, D9's premise was wrong + +D9 asserted "the 8 missing fellowship WeenieError strings are added in +FA1 … all resolve from the DAT string tables, never invented." The +implementer refused and documented absence instead. Verified: + +1. **Case-label walk.** In `HandleFailureEvent @0x00571990` + (`acclient_2013_pseudo_c.txt:382616`+) the label sequence is + `case 0x413: → case 0x414: → case 0x416: → case 0x41d: → case 0x41e: + → case 0x41f:` — **no label for `0x417`–`0x41C`** — and + `case 0x4d9: → case 0x4da: → case 0x4dd:` — **no label for `0x4DB`/ + `0x4DC`**. +2. **The `else if` chain too.** `WeenieErrorMessages`' own class doc warns + that five ids dispatch via `else if (arg2 == N)` above the switch, so + a case-label-only sweep would be unsound. The function contains exactly + seven such comparisons — `0x43`, `0x3f7`, `0x3e`, `0x23`, `0x36`, + `0x4e8`, `0x48a`, `0x408`, `0x3ee`, `0x4f` — none of them ours. +3. **Whole-file sweep.** `\b0x417\b` … `\b0x41c\b`, `\b0x4db\b`, + `\b0x4dc\b` across all 1.4 M lines return only unrelated hits: struct + field offsets in three `*(uint16_t*)(ecx+0x418)` copy loops, and + `[0x417]`/`[0x418]`/… array-index headers inside a `MeshBuffer` + render-batch data dump at `.data 0x008872xx` / `0x008c4axx`. Nothing + textual, nothing comparative. +4. **Decimal forms.** No `1047`–`1052`/`1243`/`1244` anywhere in the + function's address range. +5. **No `default:`** in the switch, so an unmatched id produces no text at + all — which is precisely what `Resolve()` already returns. + +Conclusion: acdream's existing silence for `0x0417 FellowshipIgnoringRequests` +and `0x04DB FellowshipDeclined` (the two ACE actually sends) is +retail-faithful. Adding invented English would have been the exact +failure mode the table's no-default-case rule exists to prevent. The +conformance `[Theory]` at +`tests/AcDream.Core.Tests/Chat/WeenieErrorMessagesTests.cs:390-393` is the +right artifact. **No fix owed; no register row owed** (silence *is* the +retail behavior, so there is no deviation to record). + +### (b) `0x0275` is client-authored — a receive parser was never the ask, and D6's need is satisfied + +Verified: `0x0275` is a C→S GameAction sub-opcode +(`CM_Character::Event_ConfirmationResponse @0x006A1210`, buffer `0x18`; +consumed server-side by ACE's `GameActionConfirmationResponse`). Retail +never receives it. So "the missing third message of the triple" (D6) +could not have meant a parser. + +`ClientCommandRequests.BuildConfirmationResponse` already existed pre-FA1 +byte-for-byte (`git show 6bedbc47~1` — 24-byte body, +`[env][seq][0x0275][type][context][accepted]`, matching retail's `0x18` +allocation exactly). What was genuinely absent was the **typed +discriminator**, and `GameEvents.ConfirmationType` +(`src/AcDream.Core.Net/Messages/GameEvents.cs:552-561`) supplies it with +all seven values matching retail's +`ClientUISystem::Handle_Character__ConfirmationRequest @0x005640A0` switch +(symbol verified at that exact address) and ACE's enum. + +**D6's FA4/FA5 flows are buildable on this today**: receive `0x0274` via +`ParseCharacterConfirmationRequest` (`:512`, `Type` as `uint` — cast to +the new enum), send via `BuildConfirmationResponse((uint)ConfirmationType.Fellowship, …)`, +receive `0x0276` via `ParseCharacterConfirmationDone` (`:533`). The +reinterpretation is sound. + +--- + +## 3. SHOULD-FIX + +### SF-1 — the monarch's `MayPassupExperience` must be forced false + +`AllegianceHierarchy::UnPack`'s last act before returning success is +`AllegianceData::SetMayPassupExperience(&m_pMonarch->_data, 0)` +(`acclient_2013_pseudo_c.txt:454670`, `005b77b3`) — the monarch can never +pass up, whatever the wire bit says. Lane C §4.1 names this in the same +paragraph as the `HasPackedLevel`-absent rule that +`ClientCommandResponses.cs:564-565` *did* port; only one of the pair +landed. The field is public +(`AllegianceMemberRecord.MayPassupExperience`, `:201`), so FA5 could +render it wrong. Clear it on the monarch record at `:505-506`. + +Inert against ACE (`AllegianceData.cs:76` never sets the bit for a +monarch), but it becomes live the moment the record is hand-built or a +non-ACE server is used. + +### SF-2 — `ParseFellowshipDisband` rejects a non-empty body; retail never does + +`src/AcDream.Core.Net/Messages/GameEvents.cs:749`: +`ParseFellowshipDisband(payload) => payload.Length == 0`. Retail's +`CM_Fellowship::DispatchUI_Disband @0x006A5E80` reads **only** the opcode +and then calls the handler — it never inspects or validates a trailing +byte. Encoding a length check retail does not perform means any future +non-empty body (an ACE change, a trailing pad) silently swallows the +disband and leaves FA2's roster stuck in a fellowship the server already +destroyed. The faithful contract is unconditional acceptance. The +matching test at +`tests/AcDream.Core.Net.Tests/Messages/FellowshipEventsTests.cs:197-201` +pins the unfaithful half and should flip with it. + +### SF-3 — D5's `<<1` shape is not pinned at the `0x02C0` site + +D5 requires the raw-uint `shareLoot` read at **both** message sites. The +mechanism is right — `ReadFellow` (`GameEvents.cs:777`, `shareLoot` at +`:786`) is shared by `ParseFellowshipFullUpdate` (`:661`) and +`ParseFellowshipUpdateFellow` (`:706`), and `FellowMember.ShareLoot` +(`:622`) is a `uint` with no `== 1` comparison anywhere in the tree. But +the tests pin `0x10` and `2` only at `0x02BE` +(`FellowshipEventsTests.cs:70,95`); the `0x02C0` fixture writes +`shareLoot = 0` (`:154`). Add the `<<1`-shape (`2`) assertion at the +`0x02C0` site so a future refactor that splits `ReadFellow` cannot +silently reintroduce a bool there. + +Carry-forward for the FA2 reviewer: `!= 0` semantics are currently only +*documented* (`:606-609`) — nothing consumes `ShareLoot` yet. The +consuming comparison lands in FA2 and must be checked there. + +### SF-4 — AP-90's register row now cites a deleted class + +`docs/architecture/retail-divergence-register.md:298` (AP-90) reads +"… and its `AllegianceTree` is not wired into GameWindow". `4281750b` +deleted that class. The row itself is still valid (radar relationship +state is still not delivered), but the prose must be re-pointed at +`ClientCommandResponses.AllegianceProfileLookups` / the FA2 owner. The +register rule says the bookkeeping rides the same commit; this one +slipped. + +### SF-5 — the "eleven gates" numbering in the code comments tops out at ten, and one lane citation does not exist + +Two documentation-accuracy items that will cost a future auditor time: + +* `ClientCommandResponses.cs:411-491` labels the gates "1/2, 3, 4, 5, 6, + 7, 8, 9, 10". The contract, the commit message and lane C all say + **eleven**. All eleven *fields* are gated correctly (verified §4 below) + — only the comment numbering is off, and it is the exact artifact + someone would use to answer "are all eleven present?". +* `SocialActions.cs:184` cites "lane B §4.5, §12". Lane B has no §12 — + the intended reference is master-table **row 12** + (`Panel visibility → update request`). + +--- + +## 4. Verified clean (re-derived, not taken on trust) + +### 4.1 Golden byte vectors — all six independently re-derived, all correct + +| Builder | Expected bytes (hand-derived from the cited retail sender) | Test | Verdict | +|---|---|---|---| +| `BuildFellowshipCreate(seq 7,"Team",true)` | `B1F70000 07000000 A2000000` + str16L`0400 54 65 61 6D 0000` + `01000000` = 24 B | `SocialActionsTests.cs:65-83` | ✔ | +| `BuildFellowshipCreate(seq 1,"X",false)` | 12 + 4 (str16L `0100 58 00`) + 4 = 20 B, tail `00000000` | `:85-95` | ✔ | +| `BuildFellowshipUpdateRequest(9,true)` | `…A6000000 01000000` | `:130-146` | ✔ | +| `BuildFellowshipAssignNewLeader(2,0x50001234)` | `…90020000 34120050` | `:151-168` | ✔ | +| `BuildFellowshipChangeOpenness(4,false)` | `…91020000 00000000` | `:173-190` | ✔ | +| `BuildSwear/BuildBreak/BuildKick` | `…1D000000 AAAA0000` / `…1E000000 BBBB0000` / `…1E000000 42000050` | `AllegianceRequestsTests.cs:39-90` | ✔ | + +The `0x00A2` repair is confirmed at the strongest possible provenance: +`CM_Fellowship::Event_Create @0x006A67A0` allocates `strPackSize + 0x10`, +stores `0xa2` at `006a6809`, packs the string at `006a6828`, then +`*(uint32_t*)var_c = arg2` at `006a6835` — **one trailing u32 shareXP and +no openness field**. The prior two-byte `openness`/`shareXp` shape (and +the test that pinned it) was exactly the inversion lane B §5.2(a) +described. + +`PackString16L` (`SocialActions.cs:389-403`) is Windows-1252 with the pad +computed over `2 + length` — matching retail's `PStringBase::Pack`, ACE's +`WriteString16L`, and the reader `ReadString16L` +(`GameEvents.cs:913-926`). No endianness, padding, or encoding slip +anywhere in the set. + +Byte-write-vs-u32-write equivalence (`body[12] = 0|1` into a zeroed +16-byte buffer) is byte-identical to retail's full-word store for these +0/1 payloads — see nit N-2. + +### 4.2 `Fellow` record layout (the load-bearing one) + +`Fellow::UnPack @0x005B9AD0` reads, in order: `_cp_cache`, `_lum_cache`, +`_level`, `_max_health`, `_max_stamina`, `_max_mana`, `_current_health`, +`_current_stamina`, `_current_mana`, `_share_loot`, then `str16L _name` +(min size `0x28`). `GameEvents.ReadFellow` (`:777-793`) is field-for-field +identical. ✔ + +### 4.3 `0x02BE` field order and the hash-table header + +`Fellowship::UnPack @0x005B94F0`: `_fellowship_table` → `_name` → +`_leader` → `_share_xp` → `_even_xp_split` → `_open_fellow` → `_locked` → +`_fellows_departed`, then returns — it never reads field 9 (locks). +`ParseFellowshipFullUpdate` (`GameEvents.cs:637-681`) matches exactly, +**including** the deliberate non-parse of field 9, which is the +retail-faithful choice (lane B §3.9 suggested reading it; retail does +not, and lane B's own U3 flags the struct width as unverified — the +implementer's stop-and-document call is correct and needs no register +row). + +Header: `PackableHashTable::UnPack @0x00569A90` +reads one u32 and splits `_table_size = v >> 16`, `count = v & 0xFFFF` — +so the wire is `[u16 count][u16 numBuckets]` LE, which is what the parser +reads for both the member table (`:653-654`) and the departed table +(`:670-671`). ✔ + +### 4.4 `0x02C0` is guid-first — the Chorizite disagreement independently re-resolved + +`CM_Fellowship::DispatchUI_UpdateFellow @0x006A6700`: + +``` +006a6726 cond = *(uint32_t*)eax_3 == 0x2c0; // opcode at +0 +006a673b uint32_t edi = *(uint32_t*)(eax_3 + 4); // MEMBER GUID at +4 +006a6763 Fellow::UnPack(&var_30, &arg2, …); // Fellow from +8 +006a676c uint32_t ecx_4 = *(uint32_t*)eax_7; // updateType last +``` + +Retail puts the guid first. `ParseFellowshipUpdateFellow` +(`GameEvents.cs:700-712`) reads guid → `Fellow` → `updateType`. ✔ The +lane-B three-source resolution (retail + ACE + holtburger over +Chorizite's generated shape) is confirmed at its retail end. + +### 4.5 The eleven version gates — every threshold and the ORDER + +Re-derived from `AllegianceHierarchy::UnPack @0x005B7520` and the +`AllegianceVersion` enum (`acclient.h:2979-2994`). Order on the wire is +**not** monotonic in version — officer titles (v9) precede the pools +block (v2) — and the implementation follows the wire, not the numbering: + +| # | Field | Retail predicate (address) | `ClientCommandResponses.cs` | ✔ | +|---|---|---|---|---| +| 1 | officers `PHashTable` | `m_oldVersion >= 6` (`005b7570`/`005b7585`) | `:416-425` | ✔ | +| 2 | legacy spokesperson 4-byte skip | `1 <= v < 6` (`005b759b`) | `:426-429` | ✔ | +| 3 | officer titles `PSmartArray` (bare i32 count) | `>= OfficersTitlesAdded(9)` (`005b75b4`) | `:434-439` | ✔ | +| 4–7 | monarch/spokes broadcast time+today ×4 | `>= PoolsAdded(2)` (`005b75e0`) | `:442-448` | ✔ | +| 8 | `m_motd`, `m_motdSetBy` | `>= MotdAdded(3)` (`005b7645`) | `:453-457` | ✔ | +| 9 | `m_chatRoomID` | `>= ChatRoomIDAdded(4)` (`005b7696`) | `:461-462` | ✔ | +| 10 | `m_BindPoint` (32-byte Position) | `>= Bindstones(7)` (`005b76b2`) | `:468-472` | ✔ | +| 11 | `m_AllegianceName` + `m_NameLastSetTime` | `>= AllegianceName(8)` (`005b76d1`) | `:477-481` | ✔ | +| 12 | `m_isLocked` | `>= LockedState(10)` (`005b770d`) | `:485-486` | ✔ | +| 13 | `m_ApprovedVassal` | `>= ApprovedVassal(11)` (`005b7728`) | `:490-491` | ✔ | + +`BannedCharactersAdded(5)` gates nothing — confirmed, and pinned by the +negative test `VersionGate_4to5_BannedCharactersAddedGatesNothing`. + +**Boundary tests sit on the boundaries.** Every gate has an `N-1`/`N` +fixture pair: 0→1, 1→2, 2→3, 3→4, 4→5 (negative), 5→6, 6→7, 7→8, 8→9, +9→10, 10→11 +(`AllegianceProfileVersionGateTests.cs:152-308`). For the gates whose +fields are not surfaced (pools, bind point, officers, titles) the +assertion is the monarch record surviving intact — which IS a real +structural proof here, because the fixture writer +(`BuildProfileWire`, `:40-134`) applies the gate conditions +independently of the parser: a wrong threshold would consume or skip +bytes the writer did not emit and garble every field after it. + +Cross-checked against ACE's writer +(`AllegianceHierarchy.cs:140-155`): identical field order, and +`oldVersion = 0x000B` always (`:50`) — so these gates are inert against +ACE and the live `@allegiance info` path is unchanged, exactly as +intended. + +### 4.6 `AllegianceData` field order and `AllegianceProfile`/`0x0020` framing + +`AllegianceData` per lane C §4.1, cross-checked against ACE +`AllegianceData.cs:95-117`: id, cpCached, cpTithed, bitfield, u8 gender, +u8 heritage, u16 rank, [u32 level if `HasPackedLevel`], u16 loyalty, +u16 leadership, [8 bytes either branch], str16L name. +`ReadAllegianceData` (`:532-571`) matches. ✔ + +`AllegianceProfile::UnPack @0x005B6B90` = `totalMembers`, `totalVassals`, +hierarchy — matches `:406-409`. ✔ +The `0x20` dispatch case (`0055b5dd`-`0055b607`) reads one u32 and then +`AllegianceProfile::UnPack`, and ACE's `GameEventAllegianceUpdate.cs:22-25` +writes `rank` then the profile — so `ParseAllegianceUpdate`'s +rank-leading + shared-body split (`:375-391`) is right, as is the +guid-leading `0x027C` sibling. Sharing one `ReadAllegianceProfileBody` is +exactly lane C §7.2's reuse verdict honored. ✔ + +### 4.7 Tree rules — three of four correct, and the reversal test pins ORDER + +Orphan / self-parent / duplicate all match `Add`'s semantics: + +* orphan — retail `esi = (arg2 != 0) ? Search(arg2, monarch) : nullptr; + if (esi == 0) return 0` ≡ `!knownIds.Contains(parentGuid)` (`:514`); +* self-parent — retail `esi->_data._id != _id` required, and `esi` was + found *by* `treeParentId`, so it is `treeParentId == _id` ≡ + `parentGuid == record.CharacterId` (`:515`); +* duplicate — retail `Search(_id, monarch) == 0` required ≡ + `knownIds.Contains(record.CharacterId)` (`:516`); +* all three `return null`, i.e. whole-message discard, matching + `UnPack`'s `return 0` at `005b779d`. ✔ + +The record loop's first-record-has-no-`treeParent` shape is confirmed at +`005b776e` (`if (ebp_1 != 0) { read treeParent }`) and matches `:505-512`. +✔ (The fourth rule is MF-1.) + +**Sibling reversal — the test pins the ORDER, not just membership.** +`FindVassals` (`:288-294`) walks the record list backwards, which is the +correct model of retail's push-front insert (`node->_peer = parent->_vassal; +parent->_vassal = node;` at `005b6f15`-`005b6f38`) followed by a +`GetFirstVassal`/`GetNextVassal` walk. +`TreeAssembly_SiblingOrder_ReversesOnAssembly` +(`AllegianceProfileVersionGateTests.cs:371-396`) asserts the exact +sequence `{"VassalC","VassalB","VassalA"}` against wire order A,B,C. ✔ +Note this changes the live `@allegiance info` vassal render order (it was +forward before FA1) — the new order is the retail one; lane C §9 U3 flags +a side-by-side eyeball as the closing check. + +### 4.8 Dead opcodes `0x01C9`/`0x01CA` + +`ParseFellowshipFellowUpdateDone` / `…StatsDone` +(`GameEvents.cs:759-775`) return a non-nullable record and cannot fail: +empty payload → `RawValue = null`, trailing u32 → captured. Both cases +are tested (`FellowshipEventsTests.cs:205-233`). Nothing user-visible is +possible — neither is registered in `GameEventWiring.WireAll` (grep for +`Fellowship` there returns nothing), and `RawValue` has no consumer. +Matches lane B §2.7's parse-and-ignore instruction and its BN-zero-fold +caution (payload tolerated, not assumed empty). ✔ + +### 4.9 Retail anchors in code comments — spot-check (10, not 5) + +Resolved against `symbols.json` (exact-address match unless noted): + +| Anchor cited in code | Result | +|---|---| +| `gmFellowshipUI::CreateFellowship @0x0048F730` | exact ✔ | +| `gmFellowshipUI::OnVisibilityChanged @0x0048E460` | exact ✔ | +| `Event_AssignNewLeader @0x006A5F70` | exact (`CM_Fellowship::`) ✔ | +| `Event_ChangeFellowOpeness @0x006A6040` | exact (`CM_Fellowship::`) ✔ | +| `AssignLeadershipToFellow @0x0048F1E0` | exact ✔ | +| `Fellow::Pack @0x005B9A10` / `UnPack @0x005B9AD0` | exact ✔ | +| `AllegianceHierarchy::Add @0x005B6E90` | exact ✔ | +| `Handle_Character__ConfirmationRequest @0x005640A0` | exact (`ClientUISystem::`) ✔ | +| `gmAllegianceUI::CloseKickConfirmationDialog @0x00490B00` | exact ✔ | +| `gmFellowshipUI` Quit button case 8 `@0x0049034B`; `Event_BreakAllegiance @0x006A69DA` | instruction-level, correctly inside `gmFellowshipUI::ListenToElementMessage @0x004901C0` and `CM_Allegiance::Event_BreakAllegiance @0x006A6980` ✔ | + +No misattributed anchor found. + +### 4.10 Other contract items + +* All **11 unwired S→C parsers** are present (fellowship `0x00A3`, + `0x00A4`, `0x01C9`, `0x01CA`, `0x02BE`, `0x02BF`, `0x02C0`; allegiance + `0x0003`, `0x0020`, `0x01C8`, `0x027A`), and all 11 ids exist in + `GameEventType`. ✔ +* `AllegianceTree` deletion is clean — no production or test reference + survives (the one `AllegianceTree` hit in `ChatChannelInfoTests.cs:28` + is an unrelated test-method name). Lane C §7.3's DELETE verdict + honored. ✔ (See SF-4 for the register prose.) +* Test fixtures are generated by `AceWireWriter`, a line-for-line mirror + of ACE's `Extensions.cs` writer — provenance is the server's own + algorithm, not hand-typed hex. ✔ +* No stale callers of the renamed/re-signatured builders remain. ✔ + +--- + +## 5. Nits (non-blocking; listed for completeness, no action required) + +* **N-1** — Retail degrades *gracefully* on a short legacy blob (each + gated read is additionally guarded by "≥4 bytes remain", e.g. + `005b759b`, `005b75fc`); acdream throws `FormatException` and discards + the message instead. Only reachable on a truncated pre-v11 blob, which + ACE cannot produce. +* **N-2** — `BuildFellowshipQuit`/`BuildFellowshipUpdateRequest`/ + `BuildFellowshipChangeOpenness` write a single byte where retail writes + a full u32. Byte-identical for the 0/1 payloads these carry (zeroed + buffer), and blessed by lane B §5.2 — noted only so nobody "fixes" it + into a behavior change. +* **N-3** — Retail's record count is a **signed** `int16_t` + (`005b7546`: `int16_t i = ecx;` … `if (i > 0)`), so a header with the + high bit set reads as zero records in retail and as up to 32 767 + records in acdream. Bounded and unreachable against ACE. +* **N-4** — `GameEvents.ParseConfirmationResponse` (`:581`) is by + construction unreachable in production (nothing receives `0x0275`); its + only consumer is `ConfirmationTripleTests`. Justified in its own doc + comment as a round-trip conformance mechanism, and it is the cheapest + way to prove the builder against a second implementation — but it is + new never-called code in a slice whose D3 rule is "connected, repaired, + or deleted — never trusted", so FA2 should either route it or leave the + doc comment intact as the standing justification. +* **N-5** — `4281750b`'s commit message repeats lane C §0's "~1000×" + figure for the deleted `AllegianceMath.ComputePassup`. The actual error + is roughly 40× at maxed loyalty and *under*-states at low loyalty + (`(50 + 22.5·291)/291 = 22.67` vs a true passup factor of ≈`0.53`). + The DELETE verdict is unaffected and correct; the magnitude claim + originates in the lane doc, not the implementer. + +--- + +## 6. Note on evidence I could not produce + +The slice's automated gate (Release build + full suite 13,149/4/0) is the +regression reviewer's lane. I did not build. The Debug test binaries in +this worktree predate FA1 (a `--no-build --filter` run over the six FA1 +test classes matched only the 16 pre-existing `SocialActionsTests`/ +`AllegianceRequestsTests` cases), so per the stale-build-artifact rule I +treat that run as evidence of nothing and reviewed every new test by +reading it instead. All test *logic* above is verified by inspection +against the primary sources cited.