# Campaign OP slice OP1 — fix-round closure re-review

**Commit under review:** `09029f9f` ("fix(runtime,net): OP1 review fixes —
server-seed gate, tick-wired auto-save/logout flush, fellowship mutual
exclusion").
**Closes (claimed):** every finding in
`docs/research/2026-08-10-op1-review-mechanism.md` (MF-1/MF-2/MF-3) and
`docs/research/2026-08-10-op1-review-blast.md` (M1, S1–S6).
**Mode:** read-only. No build, no test run, no launch. Git reads only.
**Scope:** closure check against the diff and the post-commit code — not a
fresh review of OP1.
**Date:** 2026-08-11.

**Verdict: CLOSED.** All ten dispositions genuinely close their finding's
failure scenario. No finding is merely gestured at. 2 SHOULD-FIX and 5 NOTE
residuals below, all introduced *by* the fixes or adjacent to them; none
reopens a review finding.

---

## 1. Per-finding closure

### MF-1 (mechanism MUST-FIX 1) — TS-71's false deferral rationale · **CLOSED**

The fixer took the stronger of the two offered remedies: wired the triggers
and retired the row.

**Seam, verified end to end:**

- `src/AcDream.Runtime/GameRuntime.cs:339-342` — the constructor calls
  `context.Session.ConfigureAutoSaveTick(...)` /
  `ConfigurePreLogoffFlush(...)` on the same object it assigns to `Session`
  at `:315`.
- `src/AcDream.Runtime/Session/LiveSessionController.cs:319, 331` — both
  hooks are `internal`, single-assignment, null-checked.
- Tick half: `LiveSessionController.cs:482` calls
  `InvokeAutoSaveTick(scope.Session)` inside `RunTopLevel`, **after** the
  `IsCurrent(scope, generation)` guard at `:467`.
- Stop half: `LiveSessionController.cs:686-687` calls
  `InvokePreLogoffFlush(activeScope.Session)` as the first statement of
  `StopCore`, before the generation bump and before `DrainRetiredScope` →
  `SessionScope.DrainTeardown` → `operations.DisposeSession` →
  `WorldSession.Dispose` → `CharacterLogOff.BuildRequestBody`
  (`src/AcDream.Core.Net/WorldSession.cs:2966`). So the blob really does
  precede the logoff request, as retail's `CPlayerSystem::LogOffCharacter`
  does.

**Both hosts, zero host edits — confirmed.** `new LiveSessionController` has
exactly two production sites, both inside `GameRuntime`
(`GameRuntime.cs:167-168`), so no production controller can miss the wiring.
The two tick sites are unchanged by this commit:
`src/AcDream.Headless/Hosting/HeadlessSessionHost.cs:341` and
`src/AcDream.App/World/RetailLiveFrameCoordinator.cs:61`. The diff touches
no file under `src/AcDream.Headless/`, and its two App files
(`LiveSessionRuntimeFactory.cs`, `CurrentGameRuntimeCommandAdapter.cs`) are
a binding factory and a command adapter, not a host loop.

**Not just hook mechanics.** Beyond the six isolated hook tests added to
`tests/AcDream.Runtime.Tests/Session/LiveSessionControllerTests.cs`, two
end-to-end tests drive the real `GameRuntime` wiring:
`Session_Tick_AutoFlushesTheDirtyBlob_OnceThe480sTimerIsDue` and
`Stop_AutoFlushesTheDirtyBlob_BeforeTheCharacterLogoffRequest`
(`tests/AcDream.Runtime.Tests/Session/DirectGameRuntimeCommandAdapterTests.cs`,
new in this commit) — both assert the `0x01A1` opcode at body offset 8, and
the second asserts flush-before-logoff by capture order.

**The per-tick allocation pre-check is only PARTLY real — see R1.**

### MF-2 (mechanism SHOULD-FIX 2) — `OnChanged`'s side-effect switch · **CLOSED**

I read `CPlayerModule::OnChanged @0x0059A8E0` myself at
`docs/research/named-retail/acclient_2013_pseudo_c.txt:423491-423554`
rather than trusting either review. The body is:

```
0059a8e9  CM_UI::SendNotice_PlayerOptionChanged(arg2);
0059a904  switch (arg2) {
0059a96a    case 2:    if (PlayerModule::IgnoreFellowshipRequests(this))     SetFellowshipAutoAcceptRequests(this, 0);
0059a921    case 4:    SmartBox::EnableWeather(...)
0059a916    case 5:    LScape::SetDay(...)
0059a954    case 7:    ClientCombatSystem::TrackTarget(...)
0059a980    case 0x12: if (PlayerModule::FellowshipAutoAcceptRequests(this)) SetIgnoreFellowshipRequests(this, 0);
0059a940    case 0x30: LScape::m_fFogEnabled = ...
           }
0059a99d  if (CPlayerModule::IsAutoSaveOption(...)) { Event_PlayerOptionChangedEvent(...); return; }
0059a9c1  if (!m_bDirty) { m_bDirty = 1; m_timeFirstDirtied = Timer::cur_time; }
```

Checked point by point against
`src/AcDream.Runtime/Gameplay/RuntimeCharacterState.cs:785-821`:

| Question | Retail | acdream | Verdict |
|---|---|---|---|
| Which id clears which | `case 2` (Ignore) clears **AutoAccept**; `case 0x12` (AutoAccept) clears **Ignore** | `:800-806` id `0x02` → clears `FellowshipAutoAcceptRequests`; `:807-813` id `0x12` → clears `IgnoreFellowshipRequests` | ✅ not transposed |
| Guard | reads the option **after** the accessor's own write, i.e. "if now ON" | writes at `:795`, then tests `value` at `:800`/`:807` — equivalent, and the comment says why | ✅ |
| Turning one OFF | guard false, no clear | `value &&` short-circuits | ✅ (`TrySetOption_TurningOffAFellowshipOption_NeverTriggersTheClear`) |
| Other already clear | the nested `Set…` accessor's own unchanged-value early return fires — **no** `OnChanged`, **no** second `0x0005` | the recursive `TrySetOption` hits the same early return at `:792-793` | ✅ (`..._WhenTheOtherIsAlreadyOff_SendsOnlyThePrimary`) |
| Wire order | the nested call finishes its whole `OnChanged` (including its `Event_PlayerOptionChangedEvent`) before the outer call reaches step 3 → **clear first, primary second** | recursion at `:800-813` precedes `sendAutoSave(characterOptionId, value)` at `:815-816` | ✅ (both order tests assert the exact two-element `sent` sequence) |
| Termination | the clear passes `0`, which cannot re-arm either "if now true" guard | same | ✅ one level, always |

Id/mask spot-checks against the verbatim header:
`IgnoreFellowshipRequests_PlayerOption = 0x2` /
`FellowshipAutoAcceptRequests_PlayerOption = 0x12`
(`docs/research/named-retail/acclient.h:4167, 4183`) match
`src/AcDream.Core.Net/Messages/SocialActions.cs:366, 382`;
`IgnoreFellowshipRequests_CharacterOption = 0x8` /
`AutoAcceptFellowRequest_CharacterOption = 0x20000000`
(`acclient.h:3409, 3430`) match
`src/AcDream.Runtime/Gameplay/CharacterOptionTable.cs:113, 129`. Both rows
carry `IsAutoSave = true`, so the recursion can never reach `MarkDirty` —
as the remark claims.

The four presentation cases are correctly left unported and correctly filed
(TS-73, §2).

### MF-3 (mechanism SHOULD-FIX 3) — no id→(word, mask) pin · **CLOSED**

`tests/AcDream.Runtime.Tests/Gameplay/CharacterOptionTableTests.cs` adds
`WordAndMask_MatchesIndependentTranscriptionOfVerbatimAcclientEnums`, a
`[Theory]` with **53 `[InlineData]` rows** — one per id in the `0x00..0x34`
space — asserting `(IsOptions1, Mask)`. I spot-checked the five
transposition-prone rows the mechanism review singled out
(`IgnoreTradeRequests` O1 `0x20000`, `DisplayAge` O2 `0x20`,
`DisplayNumberDeaths` O2 `0x10`, `UseCraftSuccessDialog` O1 `0x80000000`,
`ConfirmVolatileRareUse` O2 `0x40000`); all match both the header and the
table. `WordAndMask_AreAllPairwiseDistinct` additionally closes blast NOTE
N7 with the `Distinct().Count() == 53` check the reconstruction test
structurally cannot do. See R6 for the one column still unpinned.

### M1 (blast MUST-FIX) — no server-seed gate on the blob flush · **CLOSED**

The latch: `_hasServerSeed` (`RuntimeCharacterState.cs:665`) starts `false`,
is set **only** by `Replace` (`:726-736`) and cleared **only** by
`ResetSession` (`:944-955`). Both `TryFlush` (`:901`) and
`TryFlushIfAutoSaveDue` (`:925`) test `!_isDirty || !_hasServerSeed` and
return `false` without invoking the callback.

**Traced: no path can build or send a `0x01A1` pre-seed.** There is exactly
one wire method (`WorldSession.SendSetCharacterOptions`,
`src/AcDream.Core.Net/WorldSession.cs:2216`) and exactly one builder
(`SocialActions.BuildSetCharacterOptions`), whose only non-test caller is
that method. `SendSetCharacterOptions` has **three** call sites in `src/`,
and every one is the body of a `TryFlush`/`TryFlushIfAutoSaveDue` callback,
so the gate sits upstream of both the `Capture` and the send on all three:

| Trigger | Site | Gated by |
|---|---|---|
| Explicit `SaveOptions`, graphical | `src/AcDream.App/Net/LiveSessionRuntimeFactory.cs:359-372` | `TryFlush` |
| Explicit `SaveOptions`, headless/bot | `src/AcDream.Runtime/Session/DirectGameRuntimeCommandAdapter.cs:691-703` | `TryFlush` |
| New 480 s timer tick **and** new pre-logoff flush | `src/AcDream.Runtime/GameRuntime.cs:386-403` (one shared `SendBlob`) | `TryFlushIfAutoSaveDue` / `TryFlush` |

`Options.Replace` likewise has exactly one production caller —
`src/AcDream.Runtime/Session/LiveSessionEventRouter.cs:212`, the
`PlayerDescription` route — so the latch cannot be armed by anything but a
parsed server description. (One caveat on *which* descriptions arm it: R3.)

Nothing is lost when the gate refuses: `_isDirty` is untouched, so the
pending change survives until a real seed arrives — asserted by
`TryFlush_RefusesBeforeServerSeed_EvenWhenDirty_ThenSucceedsAfterSeed`,
`TryFlushIfAutoSaveDue_RefusesBeforeServerSeed_EvenAtThreshold`,
`ReconnectSequence_ResetSessionClearsSeed_NewReplaceUnblocksFlushAgain`
(`RuntimeCharacterStateTests.cs`) and
`SaveOptions_RefusesBeforeServerSeed_EvenWhenDirty_ThenSucceedsAfterSeed`
(`DirectGameRuntimeCommandAdapterTests.cs`).

### S1 (blast) — CH3/CH4 regression test mirrored the pre-OP1 shape · **CLOSED**

`tests/AcDream.App.Tests/Net/LiveSessionCommandRouterTests.cs:498-505` now
binds `characterState.Options.TrySetOption(id, value, sendAutoSave: …)` —
the actual production seam — instead of the hand-rolled
`SetOptionBit` + `sent.Add` substitute. The test therefore now exercises
the unchanged-value early return and the MF-2 side effect, which was the
point of the finding.

### S2 (blast) — flush callback invoked under `_dirtyGate` · **CLOSED (with R2)**

Both methods now use decide-under-lock / callback-outside-lock /
clear-under-lock (`RuntimeCharacterState.cs:896-909` and the identical
shape at `:920-934`):

```csharp
lock (_dirtyGate) { if (!_isDirty || !_hasServerSeed) return false; }  // :899-902
flush();                                                               // :903
lock (_dirtyGate) { _isDirty = false; }                                // :904-907
return true;
```

The callback runs outside the lock on **every** path — there is no branch
that still invokes it inside — and the throw path leaves the module dirty
(`TryFlush_PreservesDirtyState_WhenTheCallbackThrows`). `_dirtyGate` is now
a strict leaf lock: no site holds it while acquiring another. The tick
path's order is `LiveSessionController._gate` → `_dirtyGate` (`Tick` holds
`_gate` across `InvokeAutoSaveTick`, `LiveSessionController.cs:453-484`),
the same direction as App's router, so the named inversion is gone.

**On the re-entrancy NOTE the fixer left open:** the same-thread claim is
correct. A re-entrant `MarkDirty` from inside a flush callback sets
`_isDirty = true` and is then erased by the trailing clear — identical
outcome before and after the restructure, because C# locks are reentrant.
It is unreachable today: all three callbacks do exactly
`CharacterOptionsBlobSource.Capture` + `WorldSession.SendSetCharacterOptions`,
and that send is a pure outbound enqueue (`SendGameAction`) that pumps no
inbound handlers, so nothing on the stack can call `MarkDirty`,
`TrySetOption` or `Replace`. The *cross-thread* half of that claim is wrong
— see R2.

### S3 (blast) — ownership ledger blind to the dirty flag · **CLOSED**

`RuntimeCharacterOwnershipSnapshot` gains `OptionsAreClean`
(`RuntimeCharacterState.cs:33`, default `true`, appended last so no
positional call site breaks) and `IsConverged` requires it (`:50`);
`CaptureOwnership` populates it from `!Options.IsDirty` (`:235`). Safe
against false non-convergence: the only consumers are the disposal-stage
ledger (`GameRuntime.cs:708, 736`) and `RuntimeGameplayOwnership.Capture`,
and stage 6 disposes `CharacterOwner` (→ `Options.ResetSession()` →
`_isDirty = false`) before reading it.

### S4 (blast) — `SaveOptions` encoded a bool in `PrimaryObjectId` · **CLOSED**

`DirectGameRuntimeCommandAdapter.SaveOptions` now emits
`(Accepted, objectId 0)` unconditionally
(`DirectGameRuntimeCommandAdapter.cs:712-716`), matching
`CurrentGameRuntimeCommandAdapter.SaveOptions`
(`src/AcDream.App/Runtime/CurrentGameRuntimeCommandAdapter.cs:706-710`).

**K2 event-stream consumers checked:** `PrimaryObjectId` is carried by
`GameRuntimeEvents.cs:36/144/242` into the ordered bot-facing stream and is
read by **no** production consumer — `grep` over `src/AcDream.Headless`
finds no `ResultObjectId` use at all. The only assertions on it are the two
new `Assert.Equal(0u, …)` lines in
`DirectGameRuntimeCommandAdapterTests.cs`. Nothing depended on the `1u`
encoding, so dropping it is inert outside the shape fix itself.

### S5 (blast) — re-seed discarded pending intent but left the module dirty · **CLOSED**

`Replace` now clears `_isDirty` alongside arming the seed
(`RuntimeCharacterState.cs:726-736`), with the decision recorded in the
member's XML doc: a wholesale re-seed has no partial-merge path in retail's
`PlayerModule` either, so the server's words are current and a later flush
would only echo them. Pinned by
`Replace_ClearsDirtyState_ServerTruthSupersedesPendingLocalIntent`. This is
the "record the decision plus a test" outcome the finding asked for.

### S6 (blast) — the same bits defined twice with no cross-check · **CLOSED**

`CharacterOptionTable_AgreesWithPlayerDescriptionParserEnums`
(`CharacterOptionTableTests.cs`) asserts each table entry against
`(uint)PlayerDescriptionParser.CharacterOptions1/2` members directly. I
verified the comment's completeness claim: `CharacterOptions1` has three
non-`None`/`Default` members (`AllowGive`, `HearAllegianceChat`,
`DragItemOnPlayerOpensSecureTrade`) and `CharacterOptions2` has five — all
eight appear in the theory
(`src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs:206-236`). Edit
either definition alone and the test fails, which is exactly the CH3 class.

---

## 2. Register bookkeeping

The register diff is **exactly two single-line hunks** (`git show -U0`:
`@@ -349 +349 @@` and `@@ -353 +353 @@`), so nothing else in the file moved.

- **TS-71 retirement — accurate.** The row is deleted, not amended, and the
  section-header note reads "both remaining `SetCharacterOptions (0x01A1)`
  flush triggers (the 480 s auto-save timer, the pre-logoff flush) are now
  wired through `LiveSessionController`'s own tick/stop transaction
  (`ConfigureAutoSaveTick`/`ConfigurePreLogoffFlush`, wired once by
  `GameRuntime`'s constructor)". Every clause is true of the code as landed
  (§1 MF-1). The active-row count stays 41 because TS-73 replaces it. Two
  narrow paths where the logout half does not fire are R4/R5 — neither is on
  a normal shutdown, so the retirement stands.
- **TS-73 — truthful about what remains unported.** It names the four
  omitted `OnChanged` cases by id **and** by the retail consumer each drives
  (`0x04`→`SmartBox::EnableWeather`, `0x05`→`LScape::SetDay`,
  `0x07`→`ClientCombatSystem::TrackTarget`, `0x30`→`LScape::m_fFogEnabled`);
  I confirmed all four against the decomp body quoted in §1 MF-2. It states
  correctly that the two ported cases are the only ones mutating
  `PlayerModule` state, gives the right file/member pointer, anchors to the
  named symbol + address, and describes the user-visible consequence
  accurately (bit writes and auto-saves fine, no local presentation change).
  Its "pre-anchored to OP4" rationale points at an existing plan section
  rather than inventing a deferral reason — the opposite of the defect MF-1
  was filed against.
- **AP-193 / AP-194 — untouched and still accurate.** Neither row is in the
  diff. Nothing in this commit changes the table data they describe: the
  `0x34` `HearPkDeathMessages` row is still the ACE-sourced one (the new
  MF-3 theory pins it at O2 `0x02000000` with an inline comment naming the
  register row), and the 16-entry `ClientDefaultOnIds` array bounded at
  `0x2A` — the source of AP-194's `0x2D`/`0x2F`/`0x32` disagreement — is
  unchanged.

---

## 3. New blast radius from the fixes

All three axes named in the brief were walked.

**The new `LiveSessionController` seams.** `ConfigureAutoSaveTick` /
`ConfigurePreLogoffFlush` are `internal`, and `grep` finds no consumer
outside `GameRuntime.cs:339-342` and the new tests. Nothing else observes
`_autoSaveTickHook`/`_preLogoffFlushHook`.

**Can the flush tick fire for a non-current generation?** No. `Tick()` holds
`_gate` for its whole body (`LiveSessionController.cs:453-484`), every
mutation of `_scope`/`_generation` requires `_gate`, and `InvokeAutoSaveTick`
runs after the `IsCurrent(scope, generation)` early-out at `:467` inside the
same lock hold — the scope cannot be retired between the check and the
flush. It also cannot fire pre-`Start` or post-`Stop`: `:455-458` returns
when `_disposed`, `!_inWorld`, `_scope is null`, or `_operationDepth != 0`.
A throwing hook is caught and logged (`:487-500`), pinned by
`Tick_AutoSaveHookThrowing_DoesNotFailTheTickOrTearDownTheSession`.

**`TrySetOption`'s widened signature.** Three production call sites, all
updated to pass `WorldSession.SendSetSingleCharacterOption` as a method
group: `LiveSessionRuntimeFactory.cs:348-354`,
`DirectGameRuntimeCommandAdapter.cs:666-673`, and the router binding they
share. `CurrentGameRuntimeCommandAdapter` reaches it only through the bus,
so it needs no change. No other caller exists in `src/`. The widening cannot
change what any existing caller sends, because every caller now forwards the
callback's own `(id, value)` rather than a closed-over pair.

**S4's result shape.** Covered in §1 S4 — no consumer.

**Two flush bodies, one owner graph.** The graphical closure builds from
`_domain.Character`/`_domain.Inventory` and the new hook from
`CharacterOwner`/`InventoryOwner`;
`src/AcDream.App/Composition/SessionPlayerComposition.cs:1070-1076` passes
the same canonical `d.Character`/`d.Inventory` owners into
`LiveSessionDomainRuntime`, so both bodies read identical state. A double
flush is impossible in either order because `TryFlush` clears `_isDirty`
under the lock.

---

## 4. Residuals

### R1 — SHOULD-FIX: the per-tick allocation pre-check guards the delegate, not the closure

`src/AcDream.Runtime/GameRuntime.cs:380-404`. `SendBlob` captures **two**
things: `this` (for `CharacterOwner`/`InventoryOwner`) *and* the `session`
parameter. Because it is converted to a delegate, its closure environment
must be a class, and Roslyn instantiates a method-scope environment holding
a captured **parameter** in the method prologue — i.e. ahead of the
`if (!options.IsDirty) return;` guard at `:383-384`. The guard therefore
elides the `Action` delegate allocation on a clean tick but not the display
class, so `FlushCharacterOptions` still allocates once per
`LiveSessionController.Tick()` per live session — the exact per-tick
allocation the doc comment at `:370-377` claims it prevents ("the closure
below is only ever allocated on the rare tick where a flush might really
happen"). At 30 sessions that is the dimension the K4 headless
resource-envelope gate measures.

A one-line fix that is genuinely zero-alloc: move the guard up into the two
constructor lambdas, which capture only `this` and are allocated once —

```csharp
context.Session.ConfigureAutoSaveTick(
    session => { if (CharacterOwner.Options.IsDirty) FlushCharacterOptions(session, true); });
```

so `FlushCharacterOptions` (and its prologue) is never entered on a clean
tick. Alternatively give `RuntimeCharacterOptionsState` a
`TryFlush<TState>(TState, Action<TState>)` overload fed by a cached static
lambda. **Caveat:** this review is read-only, so I did not compile to
confirm the lowering — verify with a compiled allocation check before
rewriting. Either way the doc comment's absolute claim needs softening.

### R2 — SHOULD-FIX: S2's restructure turns a blocking-but-correct concurrent `MarkDirty` into a silent lost update

The commit message's disposition on mechanism NOTE 6 says the re-entrancy
outcome is "the same whether the callback runs inside or outside the lock".
True for the *same-thread* re-entrant case; false for the *cross-thread*
case. Pre-fix, a concurrent `MarkDirty` blocked on `_dirtyGate` until the
flush finished and `_isDirty` had been cleared, then set it — nothing lost.
Post-fix it runs during `flush()` and is erased by the trailing
`_isDirty = false` at `RuntimeCharacterState.cs:904-907`. If the toggle
landed after `CharacterOptionsBlobSource.Capture` read the words, that
option change is silently dropped until something else dirties the module.

The commit's own new test constructs exactly that interleaving —
`TryFlush_ReleasesTheDirtyGate_DuringTheCallback_SoAConcurrentMarkDirtyDoesNotBlock`
(`tests/AcDream.Runtime.Tests/Gameplay/RuntimeCharacterStateTests.cs:596-628`)
proves the concurrent `MarkDirty` completes mid-flush, then asserts nothing
about `IsDirty` afterwards.

Whichever threading model actually holds, one of the two claims in the
disposition is wrong: if two threads can reach these seams, this lost update
is live; if only Runtime's single update thread can (memory `#368`), then
the `_gate`→`_dirtyGate` inversion S2 was filed against was never reachable
either. Worth deciding and recording before OP4 puts an Options panel on
this seam. The per-dirty-period generation token the fixer already named
closes this and NOTE 6 together (capture the revision under the first lock;
clear only if it is unchanged under the second).

### R3 — NOTE: the seed latch arms on a truncated `PlayerDescription` too

`onCharacterOptions?.Invoke(p.Value.Options1, p.Value.Options2)`
(`src/AcDream.Core.Net/GameEventWiring.cs:732`) fires for **every** parse
that returns non-null, and `PlayerDescriptionParser.TryParse` yields
`options1 = options2 = 0` when the trailer is shorter than 8 bytes at the
option block (`src/AcDream.Core.Net/Messages/PlayerDescriptionParser.cs:369-372`)
or when that read throws (`:450-466`, which still returns the record, with
`TrailerTruncated = true`). `LiveSessionEventRouter.cs:212` then calls
`Replace(0, 0)` — zeroing both words *and* arming `HasServerSeed`. The local
zeroing is pre-existing (it would already break the Turbine membership
gate); what is new is that MF-1's timer makes it wire-reaching, so 480 s
later the auto-save would ship `options1 = options2 = 0` to ACE — M1's
failure class through a different door. `TrailerTruncated` already exists
and is unused by the wiring; gating the `Replace` (or at least the latch) on
it is the natural close. Reachability against ACE is low — a real
`PlayerDescription` always carries the trailer.

### R4 — NOTE: a deferred `Stop`/`Dispose` skips the pre-logoff flush

`Schedule` sets `_inWorld = false`
(`src/AcDream.Runtime/Session/LiveSessionController.cs:775`) before the
pending operation later runs `StopCore`, whose flush is gated on `_inWorld`
(`:686`). So a `Stop()`/`Dispose()` requested while `_operationDepth != 0` —
i.e. re-entrantly, from inside another operation's callback — silently drops
the logout flush. Not on either host's normal shutdown:
`GameRuntime.StopSession` (`GameRuntime.cs:564-572`) calls
`Session.Dispose()` from outside any operation and *throws* if the shutdown
was deferred, and `adapter.Session.Stop(generation)` likewise reaches
`RunTopLevel(StopCore)` directly. Recorded so the gap is known rather than
rediscovered.

### R5 — NOTE: the "pre-logoff" hook also fires on reconnect and after a failed tick

`StopCore` is reached from `ReconnectCore`
(`LiveSessionController.cs:530`) and `StopAfterFailure` (`:665`) as well as
`Stop`/`DisposeCore`, so the hook fires whenever an in-world scope is torn
down — including a mid-session reconnect and a tick that just threw. Saving
pending options before a session replacement is arguably right, and a send
on a dead socket is caught and logged (`:706-719`), so this is benign; but
the register row and the XML doc both describe the hook purely as retail's
`LogOffCharacter` site, which is narrower than what it actually does.

### R6 — NOTE: the id↔name column still has no id-by-id pin

MF-3 closes the (word, mask) column, but all three transcribed
arrays/theories in `CharacterOptionTableTests.cs` key on `CharacterOptionId`
**members**, not on numeric wire ids. Swap two values in the
`CharacterOptionId` enum
(`src/AcDream.Core.Net/Messages/SocialActions.cs:362-417`) and both the
table registration and every test lookup move together — every test stays
green while the client sends the wrong id to ACE.
`CoversTheCompleteIdSpace` pins contiguity `0x00..0x34` but not the name→id
assignment. A `[InlineData((uint)0x1D, CharacterOptionId.DisplayAge)]`-shaped
theory transcribed from `docs/research/named-retail/acclient.h:4162-4218`
would close it; that column was verified by hand this round and last, so
nothing is wrong today.

### R7 — NOTE (minor): the hook fields are written without `_gate`

`ConfigureAutoSaveTick`/`ConfigurePreLogoffFlush`
(`LiveSessionController.cs:319, 331`) assign
`_autoSaveTickHook`/`_preLogoffFlushHook` outside `_gate`, while
`Tick`/`StopCore` read them under it. There is no formal happens-before from
the constructing thread to the first ticking thread. Practically safe —
`GameRuntime` construction is fully ordered before any host can tick, and
that handoff carries its own synchronization — but a `lock (_gate)` around
the two assignments would cost nothing.

---

## 5. Verdict

**CLOSED.** Ten findings, ten real closures: M1's latch sits upstream of the
single builder on all three flush triggers; MF-1 reaches both hosts through
`GameRuntime` → `LiveSessionController` with zero host edits and end-to-end
tests, not just hook mechanics; MF-2 matches `OnChanged @0x0059A8E0` on
which-clears-which, the already-clear short-circuit, and the clear-before-
primary wire order, all read out of the decomp directly; MF-3, S1, S3, S4,
S5 and S6 each close their stated failure scenario and are pinned by a test
that would actually fail on regression. The register is honest: TS-71's
retirement matches the code, TS-73 states precisely what is still unported
and anchors it to an existing plan section, and AP-193/AP-194 are untouched
and still accurate.

Two SHOULD-FIX residuals arise from the fixes themselves — R1 (the per-tick
allocation guard does not cover the closure environment, and the doc comment
overclaims) and R2 (S2's restructure trades a blocking-but-correct
concurrent `MarkDirty` for a silent lost update, and the disposition that
waves it off is only half true). Neither reopens a finding; both should be
settled before OP4 wires the Character tab onto this seam.