fix(net): FA1 review round -- zero-id tree rejection, monarch clear, 0x001F builder

Applies both MUST-FIX items and the code-facing SHOULD-FIX items from the
dual-lens FA1 review (docs/research/2026-08-12-fa1-review-mechanism.md,
docs/research/2026-08-12-fa1-review-blast.md):

Mechanism MF-1 / blast SF-2: AllegianceHierarchy::Add @0x005B6E90 wraps its
entire body in `if (_id != 0)` -- a record whose own id is zero discards
the WHOLE message, for both the monarch and a child record, and this is
also what makes treeParent == 0 unconditionally fatal for a non-monarch
record. ReadAllegianceProfileBody now rejects CharacterId == 0 on both
paths; four new boundary tests in AllegianceProfileVersionGateTests.cs
(zero-id monarch, zero-id child, zero treeParent, plus the existing
orphan/self-parent/duplicate trio).

Mechanism MF-2: added the missing 0x001F AllegianceUpdateRequest builder --
the structural twin of the fellowship 0x00A6 this slice already repaired --
with golden-vector tests for both on/off.

Mechanism SF-1 / blast SF-3: UnPack's last act before returning success
forces the monarch's MayPassupExperience to false regardless of the wire
bit or the HasPackedLevel-absent legacy-compat fallback. Ported at the end
of the record loop; the pre-existing HasPackedLevel-absent test moved off
the monarch record (which the new clear makes indistinguishable from "the
fallback never fired") onto a vassal record, and a new test proves the
monarch clear fires even when the wire bit explicitly asks for true.

Mechanism SF-2: removed ParseFellowshipDisband's invented body-length
validation -- retail's DispatchUI_Disband reads only the opcode and never
inspects a trailing body. The parser now always succeeds; the matching
test flips from asserting rejection to asserting acceptance.

Mechanism SF-3: added the D5 `<<1` shareLoot-shape test at the 0x02C0
FellowshipUpdateFellow site -- previously only pinned at 0x02BE, so a
future split of the shared ReadFellow helper could silently reintroduce a
bool read on this leg undetected.

Mechanism SF-5: renumbered the version-gate comments in
ReadAllegianceProfileBody to the true AllegianceVersion enum values
(1-11, matching acclient.h's SpokespersonAdded..ApprovedVassal) instead of
wire-appearance order, which only reached 10 and silently dropped gate 5
(BannedCharactersAdded, which is real but gates nothing in UnPack -- now
called out explicitly). Fixed the stale "lane B §12" citation in
SocialActions.cs to the actual master-table row.

Blast SF-1: pinned the two retail-faithful but user-visible behavior
changes FA1 made to the ALREADY-LIVE `@allegiance info` command --
reversed vassal print order (3-vassal test through
FormatAllegianceInfoLines) and malformed-tree silent-drop (test at the
GameEventWiring registration layer, which is `if (info is null) return;`).

Blast SF-4: fixed a doc comment citing a nonexistent `ConfirmationResponseTests`
class; the actual class is `ConfirmationTripleTests`.

Blast SF-5: cross-referenced the confirmation-triple discriminator's split
representation (ConfirmationType on the response leg only; bare uint on
the two inbound legs production actually reads) at both sites, so FA4
inherits a stated decision rather than an unexplained inconsistency.

Full Release suite: 13,158 passed / 4 skipped / 0 failed (13,162 total),
up from the pre-fix-round 13,149/4/0 (+9 tests this round).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

src/AcDream.Core.Net/Messages/AllegianceRequests.cs

@@ -37,6 +37,24 @@ public static class AllegianceRequests
     public const uint SwearOpcode = 0x001Du;
     public const uint BreakOpcode = 0x001Eu;
 
+    /// <summary>
+    /// <c>0x001F AllegianceUpdateRequest</c> — the allegiance panel's
+    /// subscribe/unsubscribe toggle, the structural twin of the fellowship
+    /// <c>0x00A6 FellowshipUpdateRequest</c> repaired elsewhere in this
+    /// slice. Retail's <c>CM_Allegiance::Event_UpdateRequest(u32)</c>
+    /// (<c>@0x006A72BA</c>, lane C §1.2) is NOT a one-shot query — send
+    /// <c>1</c> on <c>gmAllegianceUI::PostInit</c>'s tail
+    /// (<c>@0x004911C6</c>) and on <c>RecvNotice_PlayerDescReceived</c>
+    /// (<c>@0x00490D59</c>), <c>1</c> on <c>OnVisibilityChanged</c>'s
+    /// visible branch (<c>@0x004912DD</c>), <c>0</c> on its hidden branch
+    /// (<c>@0x00491311</c>). ACE's own handler
+    /// (<c>GameActionAllegianceUpdateRequest.cs:12</c>) reads the value
+    /// and ignores it — always replying once regardless — but retail
+    /// servers do not, so this must be sent correctly (lane C §7.1
+    /// message #3).
+    /// </summary>
+    public const uint AllegianceUpdateRequestOpcode = 0x001Fu;
+
     /// <summary>Pledge yourself to the given patron.</summary>
     public static byte[] BuildSwear(uint gameActionSequence, uint patronGuid)
     {
@@ -72,6 +90,17 @@ public static class AllegianceRequests
         return Build(gameActionSequence, BreakOpcode, vassalGuid);
     }
 
+    /// <summary>
+    /// Declare the allegiance panel's visibility/subscription state to the
+    /// server (<c>0x001F AllegianceUpdateRequest</c>). See
+    /// <see cref="AllegianceUpdateRequestOpcode"/> for the four retail send
+    /// sites and their arguments.
+    /// </summary>
+    public static byte[] BuildAllegianceUpdateRequest(uint gameActionSequence, bool on)
+    {
+        return Build(gameActionSequence, AllegianceUpdateRequestOpcode, on ? 1u : 0u);
+    }
+
     private static byte[] Build(uint seq, uint sub, uint targetGuid)
     {
         byte[] body = new byte[16];

src/AcDream.Core.Net/Messages/ClientCommandResponses.cs

@@ -408,11 +408,24 @@ public static class ClientCommandResponses
         ushort recordCount = ReadU16(payload, ref pos);
         ushort oldVersion = ReadU16(payload, ref pos);
 
-        // §4.2 gates 1/2: officers (oldVersion >= 6,
+        // §4.2's ELEVEN gates are the eleven non-zero AllegianceVersion
-        // MultipleAllegianceOfficersAdded) vs the legacy single
+        // enum values (acclient.h:2979-2994, SpokespersonAdded=1 through
-        // spokesperson-id 4-byte skip (1 <= oldVersion < 6). Entries are
+        // ApprovedVassal=11), numbered below by THAT version number — not
-        // consumed but not surfaced (ACE always sends officers empty —
+        // by wire-appearance order, which is a different sequence (the
-        // lane C §5.1) so every later field still lands correctly.
+        // officers table at version 6 is read before officer titles at
+        // version 9 but after the version-1/2/3/4 fields). Gate 5
+        // (BannedCharactersAdded) is real — it is one of the eleven
+        // AllegianceVersion values — but gates NOTHING in UnPack: the ban
+        // list never rides this blob, so there is no field/read for it
+        // here (pinned by the negative test
+        // VersionGate_4to5_BannedCharactersAddedGatesNothing).
+
+        // Gate 1 (SpokespersonAdded, 1 <= oldVersion < 6) vs gate 6
+        // (MultipleAllegianceOfficersAdded, oldVersion >= 6): the officers
+        // PHashTable REPLACES the legacy single spokesperson-id 4-byte
+        // skip at version 6. Entries are consumed but not surfaced (ACE
+        // always sends officers empty — lane C §5.1) so every later field
+        // still lands correctly.
         if (oldVersion >= 6)
         {
             ushort officerCount = ReadU16(payload, ref pos);
@@ -428,9 +441,8 @@ public static class ClientCommandResponses
             _ = ReadU32(payload, ref pos); // old single spokesperson id
         }
 
-        // §4.2 gate 3: officer titles (oldVersion >= 9,
+        // Gate 9 (OfficersTitlesAdded, oldVersion >= 9) — PSmartArray<PString>:
-        // OfficersTitlesAdded) — PSmartArray<PString>: a bare i32 count,
+        // a bare i32 count, NOT the PackableHashTable u16/u16 header.
-        // NOT the PackableHashTable u16/u16 header.
         if (oldVersion >= 9)
         {
             int titleCount = unchecked((int)ReadU32(payload, ref pos));
@@ -438,7 +450,7 @@ public static class ClientCommandResponses
                 _ = StringReader.ReadString16L(payload, ref pos);
         }
 
-        // §4.2 gate 4 (PoolsAdded, oldVersion >= 2): four broadcast counters.
+        // Gate 2 (PoolsAdded, oldVersion >= 2): four broadcast counters.
         if (oldVersion >= 2)
         {
             _ = ReadU32(payload, ref pos); // monarchBroadcastTime
@@ -447,7 +459,7 @@ public static class ClientCommandResponses
             _ = ReadU32(payload, ref pos); // spokesBroadcastsToday
         }
 
-        // §4.2 gate 5 (MotdAdded, oldVersion >= 3).
+        // Gate 3 (MotdAdded, oldVersion >= 3).
         string motd = "";
         string motdSetBy = "";
         if (oldVersion >= 3)
@@ -456,12 +468,12 @@ public static class ClientCommandResponses
             motdSetBy = StringReader.ReadString16L(payload, ref pos);
         }
 
-        // §4.2 gate 6 (ChatRoomIDAdded, oldVersion >= 4).
+        // Gate 4 (ChatRoomIDAdded, oldVersion >= 4).
         uint chatRoomId = 0;
         if (oldVersion >= 4)
             chatRoomId = ReadU32(payload, ref pos);
 
-        // §4.2 gate 7 (Bindstones, oldVersion >= 7): Position =
+        // Gate 7 (Bindstones, oldVersion >= 7): Position =
         // cell(u32) + pos(3xfloat) + rotation(4xfloat, W/X/Y/Z) = 32
         // bytes. Skipped, not surfaced — see the class doc on
         // AllegianceProfileBody for why.
@@ -471,7 +483,7 @@ public static class ClientCommandResponses
                 _ = ReadU32(payload, ref pos);
         }
 
-        // §4.2 gate 8 (AllegianceName, oldVersion >= 8).
+        // Gate 8 (AllegianceName, oldVersion >= 8).
         string allegianceName = "";
         uint nameLastSetTime = 0;
         if (oldVersion >= 8)
@@ -480,12 +492,12 @@ public static class ClientCommandResponses
             nameLastSetTime = ReadU32(payload, ref pos);
         }
 
-        // §4.2 gate 9 (LockedState, oldVersion >= 10).
+        // Gate 10 (LockedState, oldVersion >= 10).
         bool isLocked = false;
         if (oldVersion >= 10)
             isLocked = ReadU32(payload, ref pos) != 0u;
 
-        // §4.2 gate 10 (ApprovedVassal, oldVersion >= 11).
+        // Gate 11 (ApprovedVassal, oldVersion >= 11).
         uint approvedVassal = 0;
         if (oldVersion >= 11)
             approvedVassal = ReadU32(payload, ref pos);
@@ -494,15 +506,22 @@ public static class ClientCommandResponses
         // wire, never version-gated) followed by (recordCount-1) records
         // each carrying an explicit treeParent.
         // AllegianceHierarchy::Add @0x005B6E90 discards the WHOLE message
-        // if a treeParent is not already in the tree (orphan), equals the
+        // if: the record's own id is zero (MF-1 — retail's ENTIRE Add
-        // record's own id (self-parent), or duplicates an id already
+        // body is wrapped in `if (_id != 0)`, so a zero id falls straight
-        // seen — modeled here as a running knownIds set; any failure
+        // out to `return 0` for BOTH the monarch and a child record — this
-        // returns null rather than a partial/corrupted tree.
+        // is also what makes treeParent == 0 unconditionally fatal for
+        // every non-monarch record, since 0 can never be a knownId); a
+        // treeParent is not already in the tree (orphan); the treeParent
+        // equals the record's own id (self-parent); or the id duplicates
+        // one already seen — modeled here as a running knownIds set; any
+        // failure returns null rather than a partial/corrupted tree.
         AllegianceMemberRecord? monarch = null;
         var records = new List<AllegianceMemberRecord>();
         if (recordCount > 0)
         {
             AllegianceMemberRecord monarchRecord = ReadAllegianceData(payload, ref pos, parentGuid: 0u);
+            if (monarchRecord.CharacterId == 0u)
+                return null;
             monarch = monarchRecord;
             var knownIds = new HashSet<uint> { monarchRecord.CharacterId };
 
@@ -511,7 +530,8 @@ public static class ClientCommandResponses
                 uint parentGuid = ReadU32(payload, ref pos);
                 AllegianceMemberRecord record = ReadAllegianceData(payload, ref pos, parentGuid);
 
-                if (!knownIds.Contains(parentGuid)
+                if (record.CharacterId == 0u
+                    || !knownIds.Contains(parentGuid)
                     || parentGuid == record.CharacterId
                     || knownIds.Contains(record.CharacterId))
                 {
@@ -521,6 +541,16 @@ public static class ClientCommandResponses
                 knownIds.Add(record.CharacterId);
                 records.Add(record);
             }
+
+            // Retail's LAST act before UnPack returns success
+            // (@0x005B77A7-0x005B77B3): the monarch can never pass up,
+            // whatever the wire bit said — force it false regardless of
+            // ReadAllegianceData's HasPackedLevel-absent legacy-compat
+            // fallback (SF-1/SF-3). Inert against ACE (AllegianceData.cs
+            // never sets the bit for a monarch record) but load-bearing
+            // the moment a record is hand-built or read from a non-ACE
+            // server.
+            monarch = monarch.Value with { MayPassupExperience = false };
         }
 
         return new AllegianceProfileBody(

src/AcDream.Core.Net/Messages/GameEvents.cs

@@ -503,7 +503,14 @@ public static class GameEvents
             BinaryPrimitives.ReadUInt32LittleEndian(payload.Slice(8)) != 0);
     }
 
-    /// <summary>0x0274 CharacterConfirmationRequest — server-driven modal confirm.</summary>
+    /// <summary>
+    /// 0x0274 CharacterConfirmationRequest — server-driven modal confirm.
+    /// <see cref="Type"/> is a bare <c>uint</c>, not <see cref="ConfirmationType"/>
+    /// — this is the leg <c>GameplayConfirmationController.HandleRequest</c>
+    /// actually consumes in production today. See <see cref="ConfirmationType"/>'s
+    /// doc comment (blast review SF-5) for why the triple currently carries
+    /// its discriminator two different ways.
+    /// </summary>
     public readonly record struct CharacterConfirmationRequest(
         uint Type,
         uint ContextId,
@@ -526,7 +533,10 @@ public static class GameEvents
     /// <summary>
     /// 0x0276 CharacterConfirmationDone — server cancellation/completion of the
     /// outstanding confirmation tuple. Retail dispatches the same type/context
-    /// pair to <c>RecvNotice_AbortConfirmationRequest</c>.
+    /// pair to <c>RecvNotice_AbortConfirmationRequest</c>. <see cref="Type"/>
+    /// is a bare <c>uint</c>, not <see cref="ConfirmationType"/> — the leg
+    /// <c>GameplayConfirmationController.HandleDone</c> actually consumes;
+    /// see <see cref="ConfirmationType"/>'s doc comment (blast review SF-5).
     /// </summary>
     public readonly record struct CharacterConfirmationDone(uint Type, uint ContextId);
 
@@ -548,6 +558,19 @@ public static class GameEvents
     /// (lane B §3.15, lane C §1.3). Campaign FA needs exactly
     /// <see cref="SwearAllegiance"/> (1) and <see cref="Fellowship"/> (4)
     /// — D6.
+    ///
+    /// <para>
+    /// FA1 review round (blast SF-5): this enum currently types ONLY the
+    /// response-side leg (<see cref="ConfirmationResponse.Type"/>) —
+    /// <see cref="CharacterConfirmationRequest.Type"/> and
+    /// <see cref="CharacterConfirmationDone.Type"/>, the two legs
+    /// production actually reads today, remain bare <c>uint</c>. This is a
+    /// deliberate, not-yet-decided split, not a double parser: FA1 did not
+    /// touch the inbound legs. Before FA4 wires fellowship (4) and
+    /// allegiance (1) confirmations, either promote both inbound records
+    /// to <see cref="ConfirmationType"/> or treat this note as the standing
+    /// decision that the enum stays response-side only.
+    /// </para>
     /// </summary>
     public enum ConfirmationType : uint
     {
@@ -570,7 +593,7 @@ public static class GameEvents
     /// This record + parser exist to give the triple a complete, TYPED
     /// representation in Core.Net (the <see cref="ConfirmationType"/>
     /// enum, not a bare <c>uint</c>) and a round-trip conformance check —
-    /// see <c>ConfirmationResponseTests</c> for the golden-vector /
+    /// see <c>ConfirmationTripleTests</c> for the golden-vector /
     /// round-trip pair against <c>BuildConfirmationResponse</c>.
     /// </summary>
     public readonly record struct ConfirmationResponse(
@@ -742,11 +765,16 @@ public static class GameEvents
     }
 
     /// <summary>
-    /// <c>0x02BF FellowshipDisband</c> — empty body
+    /// <c>0x02BF FellowshipDisband</c> — <c>DispatchUI_Disband
-    /// (<c>DispatchUI_Disband @0x006A5E80</c> reads only the opcode; ACE
+    /// @0x006A5E80</c> reads ONLY the opcode and calls straight into the
-    /// writes no body — lane B §3.11).
+    /// handler; it never inspects, validates, or even looks at a body
+    /// length. ACE writes no body today (lane B §3.11), but encoding a
+    /// length check retail itself does not perform would make a future
+    /// non-empty body (an ACE change, a trailing pad) silently swallow the
+    /// disband and leave the roster stuck in a fellowship the server
+    /// already destroyed. Accept unconditionally — this always succeeds.
     /// </summary>
-    public static bool ParseFellowshipDisband(ReadOnlySpan<byte> payload) => payload.Length == 0;
+    public static bool ParseFellowshipDisband(ReadOnlySpan<byte> payload) => true;
 
     /// <summary>
     /// <c>0x01C9 FellowshipFellowUpdateDone</c> — dead in the Sept-2013

src/AcDream.Core.Net/Messages/SocialActions.cs

@@ -181,7 +181,8 @@ public static class SocialActions
     /// show/hide, and ACE gates the whole <c>0x02C0</c> member-vitals
     /// stream on it (<c>Fellowship.cs:723</c>): a client that never sends
     /// this sees names/levels/max-vitals from <c>0x02BE</c> but a roster
-    /// frozen at join time (lane B §4.5, §12). See
+    /// frozen at join time (lane B §4.5, master-table row 12 — "Panel
+    /// visibility → update request"). See
     /// <see cref="BuildFellowshipChangeOpenness"/> for the real openness
     /// toggle, which is a separate opcode this builder used to be
     /// (incorrectly) named for.

tests/AcDream.Core.Net.Tests/Messages/AllegianceProfileVersionGateTests.cs

@@ -352,6 +352,58 @@ public sealed class AllegianceProfileVersionGateTests
         Assert.Null(ClientCommandResponses.ParseAllegianceInfoResponse(wire));
     }
 
+    // MF-1 (mechanism review): AllegianceHierarchy::Add @0x005B6E90 wraps
+    // its ENTIRE body in `if (_id != 0)` — a record whose own id is zero
+    // falls straight out to `return 0`, which UnPack turns into "discard
+    // the whole message", for BOTH the monarch and a child record. This
+    // is a FOURTH rejection rule distinct from orphan/self-parent/
+    // duplicate above, and it is also what makes `treeParent == 0`
+    // unconditionally fatal for a non-monarch record — 0 can never be a
+    // knownId once the id==0 monarch case is rejected.
+
+    [Fact]
+    public void TreeAssembly_ZeroIdMonarch_DiscardsWholeMessage()
+    {
+        var records = new List<(uint, uint, bool, string)>
+        {
+            (0u /* zero id */, 0u, true, "ZeroIdMonarch"),
+        };
+        byte[] wire = BuildProfileWire(MonarchGuid, 11, records);
+
+        Assert.Null(ClientCommandResponses.ParseAllegianceInfoResponse(wire));
+    }
+
+    [Fact]
+    public void TreeAssembly_ZeroIdChildRecord_DiscardsWholeMessage()
+    {
+        var records = new List<(uint, uint, bool, string)>
+        {
+            (MonarchGuid, 0u, true, "Monarch"),
+            (0u /* zero id */, MonarchGuid, true, "ZeroIdChild"),
+        };
+        byte[] wire = BuildProfileWire(MonarchGuid, 11, records);
+
+        Assert.Null(ClientCommandResponses.ParseAllegianceInfoResponse(wire));
+    }
+
+    [Fact]
+    public void TreeAssembly_ZeroTreeParent_OnNonMonarchRecord_DiscardsWholeMessage()
+    {
+        // treeParent == 0 for anything but the (implicit) monarch slot can
+        // never resolve — 0 is never added to knownIds because a zero-id
+        // record is itself rejected (the two tests above), so this is the
+        // orphan rule specialized to the zero case retail's own Search
+        // (arg2 != 0) guard names explicitly.
+        var records = new List<(uint, uint, bool, string)>
+        {
+            (MonarchGuid, 0u, true, "Monarch"),
+            (0x50000008u, 0u /* treeParent == 0, not the monarch */, true, "ZeroParent"),
+        };
+        byte[] wire = BuildProfileWire(MonarchGuid, 11, records);
+
+        Assert.Null(ClientCommandResponses.ParseAllegianceInfoResponse(wire));
+    }
+
     [Fact]
     public void TreeAssembly_ValidChain_ParentBeforeChild_Succeeds()
     {
@@ -451,10 +503,21 @@ public sealed class AllegianceProfileVersionGateTests
     {
         // Lane C §4.1 point 1: when HasPackedLevel (0x8) is absent, retail
         // sets MayPassupExperience itself regardless of the wire bit.
+        //
+        // FA1 review round (SF-1/SF-3): the record under test here MUST be
+        // a non-monarch record. UnPack's LAST act unconditionally forces
+        // the monarch's MayPassupExperience back to false regardless of
+        // how it was computed (see
+        // ReadAllegianceData_MonarchMayPassupExperience_ForcedFalseRegardlessOfWireBit),
+        // so putting the legacy-compat fixture on the monarch (as this
+        // test originally did) could never distinguish "the fallback
+        // fired" from "the monarch clear fired" — both produce the same
+        // observed value on that record. A vassal record is untouched by
+        // the monarch-only clear and isolates the fallback.
         var w = new AceWireWriter()
             .Write(MonarchGuid)
-            .Write((uint)1).Write((uint)0)
+            .Write((uint)2).Write((uint)0)
-            .Write((ushort)1).Write((ushort)11)
+            .Write((ushort)2).Write((ushort)11)
             .Write((ushort)0).Write((ushort)256)
             .Write((uint)0)
             .Write((uint)0).Write((uint)0).Write((uint)0).Write((uint)0)
@@ -465,21 +528,76 @@ public sealed class AllegianceProfileVersionGateTests
             .Write((uint)0)
             .Write((uint)0)
             .Write((uint)0)
+            // monarch — ordinary HasPackedLevel-set record, not under test.
             .Write(MonarchGuid)
             .Write((uint)0).Write((uint)0)
+            .Write((uint)(0x4u | 0x8u))
+            .Write((byte)0).Write((byte)0)
+            .Write((ushort)0)
+            .Write((uint)0) // level (HasPackedLevel set)
+            .Write((ushort)0).Write((ushort)0)
+            .Write((uint)0).Write((uint)0)
+            .WriteString16L("Monarch")
+            // vassal — the record under test: HasPackedLevel absent.
+            .Write(MonarchGuid) // treeParent
+            .Write(0x50000005u)
+            .Write((uint)0).Write((uint)0)
             .Write((uint)0x4u) // HasAllegianceAge only — NO HasPackedLevel, NO MayPassupExperience bit
             .Write((byte)0).Write((byte)0)
             .Write((ushort)0)
             // no level field (HasPackedLevel unset)
             .Write((ushort)0).Write((ushort)0)
             .Write((uint)0).Write((uint)0)
+            .WriteString16L("Vassal");
+
+        var parsed = ClientCommandResponses.ParseAllegianceInfoResponse(w.ToArray());
+
+        Assert.NotNull(parsed);
+        Assert.False(parsed!.Value.Monarch!.Value.MayPassupExperience); // always forced false
+        Assert.Single(parsed.Value.Records);
+        Assert.True(parsed.Value.Records[0].MayPassupExperience); // legacy-compat fallback fired
+        Assert.Equal(0u, parsed.Value.Records[0].Level); // never read — HasPackedLevel unset
+    }
+
+    // SF-1/SF-3 (review): AllegianceHierarchy::UnPack's LAST act before
+    // returning success is SetMayPassupExperience(&monarch->_data, 0) —
+    // the monarch can never pass up, whatever the wire bit says. Build a
+    // monarch record with the wire bit explicitly SET (0x10) and prove
+    // the parsed record still comes back false.
+    [Fact]
+    public void ReadAllegianceData_MonarchMayPassupExperience_ForcedFalseRegardlessOfWireBit()
+    {
+        var w = new AceWireWriter()
+            .Write(MonarchGuid) // targetGuid
+            .Write((uint)1).Write((uint)0) // totalMembers, totalVassals
+            .Write((ushort)1).Write((ushort)11) // recordCount, oldVersion=11 (newest)
+            .Write((ushort)0).Write((ushort)256) // officers: empty
+            .Write((uint)0) // officerTitles: empty
+            .Write((uint)0).Write((uint)0).Write((uint)0).Write((uint)0) // broadcast counters
+            .WriteString16L("").WriteString16L("") // motd, motdSetBy
+            .Write((uint)0) // chatRoomID
+            .Write((uint)0).Write(0f).Write(0f).Write(0f).Write(1f).Write(0f).Write(0f).Write(0f) // bindPoint
+            .WriteString16L("Alle") // allegianceName
+            .Write((uint)0) // nameLastSetTime
+            .Write((uint)0) // isLocked
+            .Write((uint)0) // approvedVassal
+            // monarch AllegianceData — HasAllegianceAge | HasPackedLevel |
+            // MayPassupExperience (0x4|0x8|0x10 = 0x1C), i.e. the wire bit
+            // asks for MayPassupExperience = true.
+            .Write(MonarchGuid)
+            .Write((uint)0).Write((uint)0)
+            .Write((uint)0x1Cu)
+            .Write((byte)0).Write((byte)0)
+            .Write((ushort)1)
+            .Write((uint)5) // level (HasPackedLevel set)
+            .Write((ushort)0).Write((ushort)0)
+            .Write((uint)0).Write((uint)0)
             .WriteString16L("Monarch");
 
         var parsed = ClientCommandResponses.ParseAllegianceInfoResponse(w.ToArray());
 
         Assert.NotNull(parsed);
-        Assert.True(parsed!.Value.Monarch!.Value.MayPassupExperience);
+        Assert.False(parsed!.Value.Monarch!.Value.MayPassupExperience);
-        Assert.Equal(0u, parsed.Value.Monarch!.Value.Level); // never read — HasPackedLevel unset
     }
 
     // ── 0x0020 AllegianceUpdate shares the same profile reader ─────────────

tests/AcDream.Core.Net.Tests/Messages/AllegianceRequestsTests.cs

@@ -88,4 +88,41 @@ public sealed class AllegianceRequestsTests
         Assert.Equal(AllegianceRequests.BreakOpcode,
             BinaryPrimitives.ReadUInt32LittleEndian(body.AsSpan(8)));
     }
+
+    // FA1 review round (mechanism MF-2): the missing 0x001F
+    // AllegianceUpdateRequest builder — the structural twin of the
+    // fellowship 0x00A6 this slice already repaired. Golden vector
+    // hand-derived from lane C §3.2: [u32 0xF7B1][u32 seq][u32 0x1F]
+    // [u32 on], total 0x10.
+    [Fact]
+    public void BuildAllegianceUpdateRequest_GoldenByteVector_On()
+    {
+        byte[] body = AllegianceRequests.BuildAllegianceUpdateRequest(gameActionSequence: 4, on: true);
+
+        byte[] expected =
+        [
+            0xB1, 0xF7, 0x00, 0x00, // envelope 0xF7B1
+            0x04, 0x00, 0x00, 0x00, // seq 4
+            0x1F, 0x00, 0x00, 0x00, // opcode 0x001F
+            0x01, 0x00, 0x00, 0x00, // on = 1
+        ];
+
+        Assert.Equal(expected, body);
+    }
+
+    [Fact]
+    public void BuildAllegianceUpdateRequest_GoldenByteVector_Off()
+    {
+        byte[] body = AllegianceRequests.BuildAllegianceUpdateRequest(gameActionSequence: 7, on: false);
+
+        byte[] expected =
+        [
+            0xB1, 0xF7, 0x00, 0x00, // envelope 0xF7B1
+            0x07, 0x00, 0x00, 0x00, // seq 7
+            0x1F, 0x00, 0x00, 0x00, // opcode 0x001F
+            0x00, 0x00, 0x00, 0x00, // on = 0
+        ];
+
+        Assert.Equal(expected, body);
+    }
 }

tests/AcDream.Core.Net.Tests/Messages/ClientCommandResponsesTests.cs

@@ -314,6 +314,82 @@ public sealed class ClientCommandResponsesTests
             lines);
     }
 
+    // FA1 review round (blast SF-1): FA1's tree-assembly rules flipped
+    // vassal print order (FindVassals now walks in REVERSE wire order —
+    // see AllegianceProfileVersionGateTests.TreeAssembly_SiblingOrder_
+    // ReversesOnAssembly) at the LIVE @allegiance info surface. The
+    // pre-existing full-tree test above has exactly ONE vassal, which
+    // cannot distinguish forward from reverse order — this pins the
+    // reversal through FormatAllegianceInfoLines itself, not just through
+    // FindVassals, with three vassals so the ordering is unambiguous.
+    [Fact]
+    public void ParseAndFormatAllegianceInfoResponse_ThreeVassals_PrintsInReverseWireOrder()
+    {
+        const uint monarchGuid = 0x50000001u;
+        const uint vassalAGuid = 0x50000010u;
+        const uint vassalBGuid = 0x50000011u;
+        const uint vassalCGuid = 0x50000012u;
+
+        byte[] wire = BuildAllegianceWire(
+            monarchGuid,
+            new()
+            {
+                (monarchGuid, 0u, true, "Monarch"),
+                (vassalAGuid, monarchGuid, true, "VassalA"),
+                (vassalBGuid, monarchGuid, true, "VassalB"),
+                (vassalCGuid, monarchGuid, true, "VassalC"),
+            });
+
+        var response = ClientCommandResponses.ParseAllegianceInfoResponse(wire);
+        Assert.NotNull(response);
+
+        var lines = ClientCommandResponses.FormatAllegianceInfoLines(response.Value).ToArray();
+
+        Assert.Equal(
+            new[]
+            {
+                "Note: An asterisk (*) indicates that the character is currently online.",
+                "Allegiance information for Monarch *:",
+                "   Vassals: ",
+                "      VassalC *",
+                "      VassalB *",
+                "      VassalA *",
+            },
+            lines);
+    }
+
+    // FA1 review round (blast SF-1): the §4.4 tree-assembly rules turned a
+    // malformed tree (orphan / self-parent / duplicate treeParent) into a
+    // whole-message discard (ParseAllegianceInfoResponse returns null),
+    // and GameEventWiring's registration is `if (info is null) return;` —
+    // so the live @allegiance info command now goes from "prints a
+    // possibly-garbled roster" to "prints nothing at all" on a malformed
+    // tree. Pin that silent-drop behavior at the wiring layer that shows
+    // it to the user.
+    [Fact]
+    public void WireAll_AllegianceInfoResponse_MalformedTree_PrintsNothing()
+    {
+        var dispatcher = new GameEventDispatcher();
+        var chat = new ChatLog();
+        GameEventWiring.WireAll(dispatcher, new ClientObjectTable(), new CombatState(), new Spellbook(), chat);
+
+        const uint monarchGuid = 0x50000001u;
+        byte[] payload = BuildAllegianceWire(
+            monarchGuid,
+            new()
+            {
+                (monarchGuid, 0u, true, "Monarch"),
+                (0x50000005u, 0x5000FFFFu /* never-seen parent — orphan */, true, "Orphan"),
+            });
+
+        GameEventEnvelope? envelope = GameEventEnvelope.TryParse(
+            WrapEnvelope(GameEventType.AllegianceInfoResponse, payload));
+        Assert.NotNull(envelope);
+        dispatcher.Dispatch(envelope.Value);
+
+        Assert.Empty(chat.Snapshot());
+    }
+
     [Fact]
     public void FormatAllegianceInfoLines_NoAllegiance_PrintsNothing()
     {

tests/AcDream.Core.Net.Tests/Messages/FellowshipEventsTests.cs

@@ -166,6 +166,33 @@ public sealed class FellowshipEventsTests
         Assert.Equal(3u, update.Value.UpdateType);
     }
 
+    // D5/mechanism SF-3: the same raw-uint, never-a-bool shareLoot
+    // requirement pinned for ParseFellowshipFullUpdate above
+    // (ParseFellowshipFullUpdate_ShareLootIsRawNotBool_D5) must ALSO hold
+    // at the 0x02C0 site — ReadFellow is shared by both parsers, but
+    // nothing previously asserted the <<1 incremental-update shape here;
+    // a future split of ReadFellow could silently reintroduce a bool read
+    // on this leg without either test catching it.
+    [Fact]
+    public void ParseFellowshipUpdateFellow_ShareLootIsRawNotBool_D5()
+    {
+        byte[] wire = new AceWireWriter()
+            .Write(0x50000005u) // guid FIRST
+            .Write((uint)10).Write((uint)5).Write((uint)3)
+            .Write((uint)100).Write((uint)80).Write((uint)60)
+            .Write((uint)90).Write((uint)70).Write((uint)50)
+            .Write((uint)2) // shareLoot — ACE's incremental-update "<<1" shape
+            .WriteString16L("Vitals")
+            .Write((uint)3) // updateType = 3 UpdateVitals
+            .ToArray();
+
+        var update = GameEvents.ParseFellowshipUpdateFellow(wire);
+
+        Assert.NotNull(update);
+        Assert.Equal(2u, update.Value.Member.ShareLoot);
+        Assert.NotEqual(1u, update.Value.Member.ShareLoot);
+    }
+
     // ── 0x00A3/0x00A4 S→C ─────────────────────────────────────────────────
 
     [Fact]
@@ -194,10 +221,15 @@ public sealed class FellowshipEventsTests
         Assert.True(GameEvents.ParseFellowshipDisband(ReadOnlySpan<byte>.Empty));
     }
 
+    // FA1 review round (mechanism SF-2): retail's DispatchUI_Disband
+    // @0x006A5E80 reads only the opcode and never inspects a trailing
+    // body — the pre-fix parser's `payload.Length == 0` check was an
+    // invented validation retail does not perform. A non-empty body
+    // (an ACE change, a trailing pad) must still succeed.
     [Fact]
-    public void ParseFellowshipDisband_NonEmptyBody_ReturnsFalse()
+    public void ParseFellowshipDisband_NonEmptyBody_StillReturnsTrue()
     {
-        Assert.False(GameEvents.ParseFellowshipDisband(new byte[] { 1 }));
+        Assert.True(GameEvents.ParseFellowshipDisband(new byte[] { 1 }));
     }
 
     // ── 0x01C9/0x01CA dead events — parse-and-ignore, must never fail ──────