Narrow re-review of the fix round (ed308087 code+tests, 511ba6e5 docs)
against the mechanism findings doc. Verdict CLOSED, no REOPEN.
All 2 MUST-FIX and 5 SHOULD-FIX verified in the actual diffs, each
re-derived rather than taken on the commit message's word:
- MF-1: zero-id rejection on BOTH the monarch and child paths, which is
also what makes treeParent == 0 provably fatal (knownIds can never
contain 0); three boundary tests.
- MF-2: the 0x001F builder re-verified against primary source --
CM_Allegiance::Event_UpdateRequest @0x006A7260 allocates 0x10, stores
0x1f at 006a72ba, writes the arg as a full u32 at 006a72cb. Both golden
vectors correct; all five new anchors resolve; the ACE claim
(GameActionAllegianceUpdateRequest.cs:12 reads and ignores the value)
is accurate.
- SF-1: monarch clear placed at retail's own position/guard; the fixture
relocation onto a vassal is not just correct but necessary, since the
clear would otherwise mask the legacy-compat fallback.
- SF-2/SF-3/SF-5 all closed; SF-5 resolved better than asked, renumbering
to the real AllegianceVersion enum values (verified against
acclient.h:2979-2994) and naming gate 5 as real-but-gating-nothing.
Spot-verified all six blast dispositions: AP-90 re-pointed without being
wrongly retired; four seam-map corrections applied as dated strikes (its
open-question-8 answer independently re-verified against PackString16L
and ACE's ReadString16L pad skip); D9 + slice row struck and annotated;
ledger arithmetic now closes (13,153 total sums correctly, -4 skips =
13,149).
Suite claim corroborated: the fix diff adds exactly +9 [Fact]/[Theory]
and removes 0, and the post-fix Release binaries (stamped after
ed308087, so --no-build is legitimate here) measure AcDream.Core.Net.Tests
at 886/0/0 -- exactly the blast doc's 877 pre-fix anchor plus 9, with all
9 new tests in that project.
One carry-forward, NOT a reopen: blast MF-2's enumeration stopped at four
rows; lane D still names the deleted AllegianceTree at :128 and :214,
both forward-looking FA2 design guidance of the same danger class as the
:791 row that was corrected. Two more dated addenda close it; FA2 should
not start before that.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mechanism-faithfulness lens on 7be86f47/6bedbc47/5f9aa16f/4281750b.
Every wire layout re-derived independently from the retail decomp rather
than taken on the lane docs' word: Fellow::UnPack, Fellowship::UnPack,
PackableHashTable::UnPack's count/buckets split, DispatchUI_UpdateFellow's
guid-first read, Event_Create's single trailing u32 shareXP,
AllegianceHierarchy::UnPack's eleven gates + their non-monotonic wire
order, AllegianceHierarchy::Add, AllegianceProfile::UnPack, the 0x20
dispatch case. All six golden byte vectors re-computed field by field --
no encoding, padding, or endianness slip found.
Both premise-contradiction calls VERIFIED CORRECT from primary source:
the 8 fellowship WeenieError ids genuinely have no case label, no else-if
comparison, no decimal form and no default fallthrough in
HandleFailureEvent (D9's premise was wrong, the refusal to invent English
was right); and 0x0275 is client-authored, so the typed ConfirmationType
enum -- not a receive parser -- was the real gap, and D6's FA4/FA5 flows
are buildable on what landed.
MUST-FIX: (1) AllegianceHierarchy::Add's fourth rejection rule (_id == 0,
which also makes treeParent == 0 unconditionally fatal) is unmodeled;
(2) the 0x001F AllegianceUpdateRequest builder -- the allegiance twin of
the 0x00A6 this slice repaired, and lane C's #3 minimum-viable message --
is missing entirely.
SHOULD-FIX: monarch MayPassupExperience is not force-cleared;
ParseFellowshipDisband validates a body length retail never inspects;
D5's <<1 shape is unpinned at the 0x02C0 site; AP-90's register row still
cites the deleted AllegianceTree; the gate comment numbering stops at ten.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
