diff --git a/docs/research/2026-08-12-fa4-review-mechanism.md b/docs/research/2026-08-12-fa4-review-mechanism.md index 863476d1..0aed7a36 100644 --- a/docs/research/2026-08-12-fa4-review-mechanism.md +++ b/docs/research/2026-08-12-fa4-review-mechanism.md @@ -720,3 +720,103 @@ so `0x00A6` never reaches the reconnected server). Everything else CLOSED, with the dimming arithmetic audited correct at 34/16. Recommend a focused MUST-FIX-3-only re-fix (move the re-declaration to an in-world seam); no re-review of the other findings is owed. + +--- + +## MF-3 REOPEN re-fix re-review — 2026-08-12 (commit `04161def`) + +**Verdict: CLOSED.** The seam ordering the whole fix rests on holds — the +`EnteredWorld` re-declaration is genuinely post-world, the inverse of the +pre-world `ResetSessionState`/`SessionDialogs` stage the original fix used. +The widget-level root I named is addressed, the tests model the world gate +(not an unconditional fake), and the full suite reconciles. + +### The two halves, verified in the diff +1. **Widget-level (the root I named).** + `SocialFellowshipPageController.SetPageVisible` now advances the latch + ONLY on `Accepted`: + `if (_bindings.SetPanelOpen(visible).Status == RuntimeCommandStatus.Accepted) _pageVisible = visible;` + A dropped/Inactive publish (the pre-world reconnect state) leaves + `_pageVisible` untouched, so the in-world attempt is not deduplicated + away by the `if (_pageVisible == visible) return` guard. The normal + in-world case is unchanged (Accepted → latch advances as before), and a + persistently non-Accepted result retries at most once per generation + (EnteredWorld fires once per generation) — no retry loop. +2. **Lifecycle split.** `ResetSessionDeclaration` (pre-world) now ONLY + clears the latch (`=> _fellowship?.ResetPageVisibleLatch();`) — it no + longer calls `UpdateFellowshipPageVisibility`, so it makes no dropped + pre-world declaration. The new `RedeclareAfterWorldEntry()` + (`=> UpdateFellowshipPageVisibility();`) does the re-evaluation, exposed + as `RetailUiRuntime.RedeclareSocialPanelAfterWorldEntry` and composed + into `LiveSessionRuntimeFactory`'s `EnteredWorld.RestoreLayout` delegate + (RestoreLayout first, then redeclare). + +### Seam ordering — traced, and it holds (the load-bearing claim) +The fix is correct only if `EnteredWorld` runs after `_inWorld = true` and +after the command seam is active. It does. In `LiveSessionController.StartCore`: +- `ResetHostBeforeStart` (`:555`) → `host.ResetSessionState` → + `LiveSessionLifecycleHost.ResetSessionState` (`:47`) → `_bindings.Reset` + → the reset manifest's `SessionDialogs` stage → `ResetSessionTransientUi` + → `ResetSessionDeclaration` (clears the latch). **PRE-world** — `_inWorld` + is still false here. +- `binding.ActivateCommands()` (`:639`) — the deferred command seam binds + for the new generation. +- `_inWorld = true` (`:642`). +- `host.ApplyEnteredWorld(selection)` (`:644`) → + `LiveSessionLifecycleHost.ApplyEnteredWorld` (`:57`) → `_bindings.Entered` + → `LiveSessionHost.ApplyEnteredWorld` (`:212`) → + `_enteredWorld.RestoreLayout()` (`:216`) → the re-fixed delegate → + `RedeclareSocialPanelAfterWorldEntry` → `RedeclareAfterWorldEntry` → + `UpdateFellowshipPageVisibility` → `SetPageVisible(true)`. **POST-world** + — `_inWorld` is true and commands are active, so + `SetPanelOpen`'s `Validate(requireWorld: true)` passes → **Accepted → + `0x00A6` published** → latch advances. Fellow vitals resume on the fresh + server. + +This is the exact inverse of the pre-world `SessionDialogs` stage traced in +the REOPEN, and it is idempotent: the social panel is not a +state-managed-visibility window, so `RestoreLayout` does not re-show it and +no `OnShown` edge fires — `RedeclareAfterWorldEntry` is therefore +load-bearing (not redundant) in the reconnect-with-panel-open case, while +still a safe no-op if some other path had already re-declared (latch +already set). + +### Tests genuinely exercise the world gate (and would fail against pre-fix) +The fake `SetPanelOpen` now returns `AcceptedResult` when its +`inWorld`/`PanelOpenInWorld` flag is true and `InactiveResult` when false — +a real world-gate model, not an unconditional recorder. +- `SetPageVisible_DoesNotLatch_WhenDeclarationDropped_SoItRetriesInWorld`: + sets the flag false, `SetPageVisible(true)` records the dropped attempt; + flips the flag true, `SetPageVisible(true)` AGAIN records a send. Against + the pre-fix unconditional `_pageVisible = visible`, the second call would + early-return (deduplicated) and record nothing — the `Assert.Contains` + would fail. So this pins the widget-level root. +- `Reconnect_ReDeclares0x00A6_AfterWorldEntry_NotDuringPreWorldReset`: + after the in-world declare, sets in-world false, calls + `ResetSessionDeclaration()` and asserts `DoesNotContain(...set-panel-open...)` + — this is exactly the assertion that fails if the pre-world declaration is + reintroduced (matching the coordinator's RED-verification); then sets + in-world true, calls `RedeclareAfterWorldEntry()` and asserts the send + lands. +- `Reconnect_StaysSilent_WhenFellowshipPageIsNotActuallyOpen`: default tab + Allegiance; after reset + post-world redeclare, the D4 conjunction is + false, so no send — the counterpart holds. + +### Build currency and totals +The current App test binary is post-`04161def`: the three new tests +reference `SocialPanelController.RedeclareAfterWorldEntry`, a method that +exists only in the fixed source — the project would not compile against +pre-fix source, so a resolving+passing run proves the binary reflects the +fix. Ran on it: the three new tests 3/3, and the three touched classes +(`SocialFellowshipPageControllerTests` / `SocialPanelControllerTests` / +`GameplayConfirmationControllerTests`) 58/58. The **13,286/4/0** full-suite +claim reconciles by direct count: net **+1** test from the prior 13,285 +(the one new widget `[Fact]`; the two panel tests were renamed in place, +net 0), 0 deletions. Not re-run end-to-end here. + +### Disposition +MF-3 is CLOSED. All five MUST-FIX, all 9 SHOULD-FIX, all 4 NIT, and blast +SF-1 are now correctly applied; the D6/D7/SF-8 dimming stands at the +audited-correct 34/16. The FA4 fix round is fully resolved on the +mechanism lens; only the user's connected gate (several steps +`[TWO-CLIENT]`, deferrable to FA6) remains owed.