diff --git a/docs/research/2026-08-11-op7-review.md b/docs/research/2026-08-11-op7-review.md new file mode 100644 index 00000000..b07f110b --- /dev/null +++ b/docs/research/2026-08-11-op7-review.md @@ -0,0 +1,412 @@ +# Campaign OP slice OP7 — combined dual-lens review + +**Commit under review:** `09cb548a` ("feat(headless): Campaign OP slice OP7 — +declared characterOptions with seed-diff sends") +**Contract:** `docs/plans/2026-08-10-options-panel-campaign.md` §4 OP7 + §2 D8 +**Research base:** `docs/research/2026-08-10-character-options-map.md` §5.2/§7.3; +`docs/research/2026-08-10-set-character-options-wire.md` §3.5/§5 +**Reviewer:** one agent carrying BOTH lenses (mechanism/contract + blast radius), +per the coordinator's smaller-slice protocol. +**Method:** read-only. No build, no test run, no client launch. All verdicts are +source-derived with `file:line` evidence. + +## Verdict + +**APPROVE-WITH-FIXES** — two MUST-FIX, five SHOULD-FIX, seven NOTEs. + +The core mechanism is right: the 26-name allow-list is an exact, correct +transcription of research §5.2's tier-1+tier-2 rows (every one of the risky +prose→enum renames resolves to the correct `PlayerOption` id — see the table in +§1); the diff engine's send policy matches wire research §3.5 and §5.5 +exactly; the two-precondition latch is genuinely per-connect correct (it is +paired with `RuntimeCharacterOptionsState.ResetSession`'s own +`HasServerSeed`/word reset at generation reset); and #368 is honoured — +the whole diff is synchronous delegate plumbing with zero `async`/`Task`/ +continuation added anywhere under `src/`. + +The two MUST-FIX findings are both *silent* failures rather than crashes: one +production launch mode drops the declared block entirely, and one degraded-parse +path lets the seeder act on words OP1's own R3 fix already declared untrusted. + +--- + +## 1. The 26-name → id cross-check (mechanism lens, item 1) + +Allow-list under review: `src/AcDream.Headless/Configuration/HeadlessConfigurationLoader.cs:26-57`. +Enum: `src/AcDream.Core.Net/Messages/SocialActions.cs:362-417`. +Table: `src/AcDream.Runtime/Gameplay/CharacterOptionTable.cs:111-165`. +Retail ground truth: `docs/research/named-retail/acclient.h:4165-4217` +(`enum PlayerOption`). +Research rows: `character-options-map.md` §5.2 (`:467-483`) with per-row +storage/wire columns from §2.1-§2.7. + +Legend: **Word/Mask** from `CharacterOptionTable`; **Wire** S = auto-save +(`0x0005` immediately), B = batched (dirties the `0x01A1` module); +research §2's own Wire column is the cross-check. + +### Tier 1 (22 rows) + +| # | Research §5.2 prose name | id | Allow-list member | Enum value | retail `acclient.h` name | Word · Mask | Wire (table / §2) | Verdict | +|---|---|---|---|---|---|---|---|---| +| 1 | `IgnoreAllegianceRequests` | 0x01 | `IgnoreAllegianceRequests` | 0x01 | `IgnoreAllegianceRequests_PlayerOption` | O1 · `0x00000004` | S / S (§2.3) | ✅ | +| 2 | `IgnoreFellowshipRequests` | 0x02 | `IgnoreFellowshipRequests` | 0x02 | `IgnoreFellowshipRequests_PlayerOption` | O1 · `0x00000008` | S / S (§2.3) | ✅ | +| 3 | `IgnoreAllTradeRequests` | 0x03 | **`IgnoreTradeRequests`** (rename) | 0x03 | `IgnoreTradeRequests_PlayerOption` | O1 · `0x00020000` | B / B (§2.4) | ✅ | +| 4 | `LetOtherPlayersGiveYouItems` | 0x06 | **`AllowGive`** (rename) | 0x06 | `AllowGive_PlayerOption` | O1 · `0x00000040` | B / B (§2.4) | ✅ | +| 5 | `ShareFellowshipExpAndLuminance` | 0x0F | **`FellowshipShareXP`** (rename) | 0x0F | `FellowshipShareXP_PlayerOption` | O1 · `0x00040000` | S / S (§2.3) | ✅ | +| 6 | `AcceptCorpseLootingPermissions` | 0x10 | **`AcceptLootPermits`** (rename) | 0x10 | `AcceptLootPermits_PlayerOption` | O1 · `0x00080000` | S / S (§2.4) | ✅ | +| 7 | `ShareFellowshipLoot` | 0x11 | `FellowshipShareLoot` | 0x11 | `FellowshipShareLoot_PlayerOption` | O1 · `0x00100000` | S / S (§2.3) | ✅ | +| 8 | `AutomaticallyAcceptFellowshipRequests` | 0x12 | **`FellowshipAutoAcceptRequests`** (rename) | 0x12 | `FellowshipAutoAcceptRequests_PlayerOption` | O1 · `0x20000000` | S / S (§2.3) | ✅ | +| 9 | `ShowAllegianceLogons` | 0x18 | **`DisplayAllegianceLogonNotifications`** (rename) | 0x18 | `DisplayAllegianceLogonNotifications_PlayerOption` | O1 · `0x08000000` | B / B (§2.3) | ✅ | +| 10 | `UseChargeAttack` | 0x19 | `UseChargeAttack` | 0x19 | `UseChargeAttack_PlayerOption` | O1 · `0x10000000` | S / S (§2.5) | ✅ | +| 11 | `UseCraftingChanceOfSuccessDialog` | 0x1A | **`UseCraftSuccessDialog`** (rename) | 0x1A | `UseCraftSuccessDialog_PlayerOption` | O1 · `0x80000000` | B / B (§2.2) | ✅ | +| 12 | `AutoRepeatAttacks` | 0x00 | **`AutoRepeatAttack`** (rename) | 0x00 | `AutoRepeatAttack_PlayerOption` | O1 · `0x00000002` | S / S (§2.5) | ✅ | +| 13 | `LeadMissileTargets` | 0x2A | `LeadMissileTargets` | 0x2A | `LeadMissileTargets_PlayerOption` | O2 · `0x00008000` | S / S (§2.5) | ✅ | +| 14 | `UseFastMissiles` | 0x2B | `UseFastMissiles` | 0x2B | `UseFastMissiles_PlayerOption` | O2 · `0x00010000` | S / S (§2.5) | ✅ | +| 15 | `ConfirmUseOfRareGems` | 0x2D | **`ConfirmVolatileRareUse`** (rename) | 0x2D | `ConfirmVolatileRareUse_PlayerOption` | O2 · `0x00040000` | B / B (§2.2) | ✅ | +| 16 | `AppearOffline` | 0x27 | `AppearOffline` | 0x27 | `AppearOffline_PlayerOption` | O2 · `0x00001000` | S / S (§2.7) | ✅ | +| 17 | `ListenToAllegianceChat` | 0x1B | `ListenToAllegianceChat` | 0x1B | `HearAllegianceChat_PlayerOption` † | O1 · `0x40000000` | S / S (§2.6) | ✅ | +| 18 | `ListenToGeneralChat` | 0x23 | `ListenToGeneralChat` | 0x23 | `HearGeneralChat_PlayerOption` † | O2 · `0x00000100` | S / S (§2.6) | ✅ | +| 19 | `ListenToTradeChat` | 0x24 | `ListenToTradeChat` | 0x24 | `HearTradeChat_PlayerOption` † | O2 · `0x00000200` | S / S (§2.6) | ✅ | +| 20 | `ListenToLFGChat` | 0x25 | `ListenToLFGChat` | 0x25 | `HearLFGChat_PlayerOption` † | O2 · `0x00000400` | S / S (§2.6) | ✅ | +| 21 | `ListenToRoleplayChat` | 0x26 | `ListenToRoleplayChat` | 0x26 | `HearRoleplayChat_PlayerOption` † | O2 · `0x00000800` | S / S (§2.6) | ✅ | +| 22 | `ListenToSocietyChat` | 0x2E | `ListenToSocietyChat` | 0x2E | `HearSocietyChat_PlayerOption` † | O2 · `0x00080000` | S / S (§2.6) | ✅ | + +† The six `ListenTo*Chat` members are the deliberate pre-existing CH3 spelling +divergence from retail's `Hear*Chat_PlayerOption`, documented at +`SocialActions.cs:354-357`. Ids are identical; the rename is ours and +established, not introduced here. + +### Tier 2 (4 rows) + +| # | Research §5.2 prose name | id | Allow-list member | Enum value | retail `acclient.h` name | Word · Mask | Wire (table / §2) | Verdict | +|---|---|---|---|---|---|---|---|---| +| 23 | `UseMainPackAsDefaultForPickingUpItems` | 0x29 | **`MainPackPreferred`** (rename) | 0x29 | `MainPackPreferred_PlayerOption` | O2 · `0x00004000` | B / B (§2.1) | ✅ | +| 24 | `RunAsDefaultMovement` | 0x0A | **`ToggleRun`** (rename) | 0x0A | `ToggleRun_PlayerOption` | O1 · `0x00000400` | B / B (§2.5) | ✅ | +| 25 | `AutoTarget` | 0x0D | `AutoTarget` | 0x0D | `AutoTarget_PlayerOption` | O1 · `0x00002000` | B / B (§2.5) | ✅ | +| 26 | `SalvageMultipleMaterialsAtOnce` | 0x22 | **`SalvageMultiple`** (rename) | 0x22 | `SalvageMultiple_PlayerOption` | O2 · `0x00000080` | B / B (§2.1) | ✅ | + +**The three renames the review brief flagged as risky are all correct:** + +* **`RunAsDefaultMovement` → `ToggleRun` (0x0A) — CORRECT, and it is NOT 0x04.** + `acclient.h:4175` is `ToggleRun_PlayerOption = 0xA`; `acclient.h:4169` is + `DisableMostWeatherEffects_PlayerOption = 0x4`. Research §2.5 (`:307`) names + the retail UI row "Run as Default Movement | 0x0A · O1 `0x00000400`" and + explicitly warns that acdream's `RuntimeMovementCommand.ToggleRunLock` is a + **keybind latch, not this option** — the implementer picked the option, not + the latch. +* **`AcceptCorpseLootingPermissions` → `AcceptLootPermits` (0x10) — CORRECT** + (`acclient.h:4181`; research §2.4 `:291` gives O1 `0x00080000`, matching + `CharacterOptionTable.cs:127`). +* **`ShowAllegianceLogons` → `DisplayAllegianceLogonNotifications` (0x18) — + CORRECT** (`acclient.h:4189`; research §2.3 `:282` gives O1 `0x08000000`, + matching `CharacterOptionTable.cs:135`). + +**Count and exclusion check.** 22 + 4 = 26 entries, matching §7.3's "22 options +in tier 1 plus 4 in tier 2". No tier-3 row leaked in — spot-checked against +§5.2's exclusion list (`AttemptToDeceiveOtherPlayers` 0x09, the six privacy +rows 0x1C-0x20/0x28, `SideBySideVitals` 0x13, `AdvancedCombatUI` 0x0C, +`StayInChatMode` 0x0B, `LockUI` 0x33, `UseMouseTurning` 0x31, `ShowHelm` 0x2F, +`ShowCloak` 0x32, `HearPkDeathMessages` 0x34): none appear. The landmine ids +`0x35`/`0x36` (§5.3 constraint 2) do not exist in `CharacterOptionId` at all, +so they are unreachable by construction — correct. + +**Wire-policy split:** 17 auto-save + 9 batched (`0x03, 0x06, 0x0A, 0x0D, +0x18, 0x1A, 0x22, 0x29, 0x2D`). Every one agrees with research §2's own S/B +column. No mismatch found. + +--- + +## MUST-FIX + +### MF-1 — the direct-CLI launch mode silently drops the entire declared block + +`src/AcDream.Headless/Hosting/HeadlessProcessHost.cs:146-157` + +```csharp +private static HeadlessSessionDescriptor WithAccount( + HeadlessSessionDescriptor source, + string account) => + new() + { + Id = source.Id, + Endpoint = source.Endpoint, + Account = account, + Character = source.Character, + Policy = source.Policy, + Credential = source.Credential, + }; +``` + +`HeadlessSessionDescriptor` now has **seven** properties +(`HeadlessConfiguration.cs:32-69`); this hand-rolled copy sets six. When +`--user`/`--password` are supplied (`HeadlessProcessHost.cs:78-83`, +reached from `HeadlessEntryPoint.cs:82-83` via +`HeadlessCommandLine.cs:56-95` — the K3 "direct single-session CLI launch" +mode used in the connected closeout), the descriptor handed to +`HeadlessSessionHost` has `CharacterOptions == null`. +`ParseDeclaredCharacterOptions` (`HeadlessSessionHost.cs:815-827`) then +returns an empty dictionary, `HasDeclaredOptions` is false, and +`TryDiffAndSend` early-returns forever. The config parsed, validated, and +did nothing — no exception, no diagnostic, no wire traffic. + +This is directly load-bearing on the slice's own gate: OP7's connected gate is +"one live bot-vs-local-ACE run"; the test script's recipe +(`docs/research/2026-08-11-campaign-op-test-script.md`, OP7 §Recipe step 3) +uses `--config` with an environment credential, which happens to avoid the +broken path — but a coordinator who passes `--user/--password` instead would +observe a total no-op and have no signal explaining why. + +**Fix:** add `CharacterOptions = source.CharacterOptions,` to `WithAccount`, +and add a regression test asserting the direct-credential path preserves it. +Consider converting `HeadlessSessionDescriptor` to a `record` so `with` makes +this class of omission impossible for the next field. + +### MF-2 — a truncated-trailer PlayerDescription re-seed lets the seeder diff (and flush) against untrusted words + +`src/AcDream.Runtime/Session/LiveSessionEventRouter.cs:210-215` + +`src/AcDream.Headless/Hosting/HeadlessCharacterOptionsSeeder.cs:113-136` + +```csharp +onCharacterOptions: (options1, options2, trailerTruncated) => +{ + character.Character.Options.Replace( + options1, options2, armServerSeed: !trailerTruncated); + character.OnCharacterOptionsChanged?.Invoke(options1, options2); // ← fires unconditionally +}, +``` + +OP1's R3 fix established that a `TrailerTruncated` PlayerDescription carries +**untrusted** option words ("the parser's zero placeholders, not server truth", +`RuntimeCharacterState.cs:739-745`) and therefore must not authorize a flush. +But `Replace` only ever *sets* `_hasServerSeed`, never clears it +(`RuntimeCharacterState.cs:751-756`), so after one good seed the latch stays +armed for the rest of the session. OP7 now hangs an automatic diff-and-send on +the notification that fires for truncated parses too: + +1. Good PlayerDescription lands → `HasServerSeed = true`, real words. +2. A later PlayerDescription truncates. The parser reads `options1` early + (`PlayerDescriptionParser.cs:371-372`) but `options2` only after the + shortcut / 8-spell-list / desired-comps sections + (`:374-430`), so a `FormatException` anywhere in those sections + (`:450-460`) yields `options2 = 0` with `trailerTruncated = true`. +3. `Replace` installs `options2 = 0`; `HasServerSeed` stays **true**. +4. `OnCharacterOptionsChanged` fires → `NoteOptionsSeeded` → + `TryDiffAndSend` passes its `HasServerSeed` guard + (`HeadlessCharacterOptionsSeeder.cs:120-121`) and diffs against the zeroed + word: every declared Options2 id now reads "off". +5. Any declared **batched** Options2 id (`SalvageMultiple`, + `MainPackPreferred`, `ConfirmVolatileRareUse`) sets `needsFlush` → + `SaveOptions` → `TryFlush` succeeds (dirty AND seeded) → + `CharacterOptionsBlobSource.Capture` packs `character.Options.Options2` + — i.e. **zero** (`CharacterOptionsBlobSource.cs:41-42`) — into `0x01A1`. + ACE stores it raw with no validation (wire research §5.1), wiping + `AppearOffline`, all six `ListenTo*Chat`, helm/cloak and every other + Options2 bit for that character. + +That is exactly the wipe class OP1's M1/R3 exist to close, re-opened from a +different direction. Without OP7 nothing dirties the module at that instant, so +this slice is what makes it reachable automatically rather than incidentally. + +**Fix (cheap, at the notification seam):** carry the trust signal to the +observer — either skip the `OnCharacterOptionsChanged` invoke when +`trailerTruncated`, or widen the callback with the flag and have +`NoteOptionsSeeded` ignore untrusted seeds. A seeder-local guard alone is not +possible today: `HasServerSeed` cannot distinguish "seeded once, then +corrupted" from "seeded". + +--- + +## SHOULD-FIX + +### SF-1 — `Enum.TryParse` accepts numeric and comma-combined keys, bypassing the name allow-list + +`src/AcDream.Headless/Configuration/HeadlessConfigurationLoader.cs:218-227` + +```csharp +if (!Enum.TryParse(name, ignoreCase: false, out CharacterOptionId id) + || !AllowedCharacterOptions.Contains(id)) +``` + +`Enum.TryParse` accepts (a) a decimal numeric string and (b) a +comma-separated list of member names, OR-combining them — both behaviours apply +to non-`[Flags]` enums. The allow-list check then runs against the *resulting +value*, so a key that is not an enum-member spelling at all can pass: + +* `"15"` → `0x0F` → `FellowshipShareXP` — allow-listed, accepted. +* `"ToggleRun,AutoTarget"` → `0x0A | 0x0D` = `0x0F` → **`FellowshipShareXP`** — + allow-listed, accepted. A config that reads as two movement options silently + sets fellowship XP sharing instead. + +`HeadlessSessionHost.ParseDeclaredCharacterOptions` (`:815-827`) re-parses with +`Enum.Parse` and reaches the same wrong id, so nothing downstream catches it. +This is the "wrong name silently sets a different option" class D8's strictness +exists to prevent — the wire is safe (the id is real and allow-listed) but the +declaration is not honoured as written. + +**Fix:** validate the *name*, not the parsed value — e.g. a +`HashSet(StringComparer.Ordinal)` built from +`AllowedCharacterOptions.Select(id => id.ToString())`, or keep `TryParse` and +add `&& string.Equals(name, id.ToString(), StringComparison.Ordinal)`. + +### SF-2 — a both-true fellowship declaration never converges, contradicting D8's idempotence claim + +`HeadlessCharacterOptionsSeeder.cs:123-136` + +`RuntimeCharacterState.cs:818-834` (OP1's MF-2 mutual exclusion). + +`IgnoreFellowshipRequests` (0x02) and `FellowshipAutoAcceptRequests` (0x12) are +both in the allow-list and each clears the other when set ON. Declaring both +`true` produces, on every single connect, in id-ascending order: + +* 0x02 → set ON (recursive clear of 0x12 is a no-op if already off) → `0x0005` +* 0x12 → set ON → recursion clears 0x02 → `0x0005` (0x02=0) → `0x0005` (0x12=1) + +Final state 0x02 = **off**, 0x12 = on. The next PlayerDescription echoes that, +so the declared 0x02 = true differs again and the whole dance repeats — three +wire messages per connect, forever, with the config's 0x02 declaration never +honoured. The slice's stated property ("idempotent on reconnect by +construction — the diff simply finds nothing once the server agrees", commit +message and `HeadlessCharacterOptionsSeeder.cs:47-58`) does not hold for this +config, and nothing warns the operator. + +**Fix:** reject the contradiction at load (`ValidateCharacterOptions` already +has the whole dictionary in hand — both-true is a two-line check naming both +keys), which is cheaper and more honest than teaching the diff about retail's +exclusion. + +### SF-3 — the "purely passive" observation hook is invoked mid-sequence in the portal path + +`src/AcDream.Runtime/Session/RuntimeLiveEntitySessionController.cs:877-882` + +```csharp +_session.SendGameAction(GameActionLoginComplete.Build()); +_onLoginCompleteSent?.Invoke(); // ← runs the whole diff-and-send here +transit.EndTeleport(); +_log(...); +``` + +The other site (`:199-202`) puts the invoke last in its block, which is +genuinely additive. This one sits **between** the send and +`transit.EndTeleport()`, after `_pendingPortalCompletion` was already cleared +at `:831-832`. The callback body is not trivial — it runs the full diff, real +`SendGameAction`s, `TryFlush`, and `EmitResult` → event-hub publication to +subscribed bot policies (a faulting policy observer is a modelled scenario; +see `HeadlessSessionHost.Quarantine`'s own comment at `:419-424`). A throw from +any of those aborts `EndTeleport()`, leaving the transit state machine +mid-teleport with its retry token already discarded — a new failure mode the +hook's doc comment (`:77-91`, "Never changes when or whether LoginComplete is +sent — purely additive") claims cannot exist. + +**Fix:** move the invoke after `transit.EndTeleport()` (or after the `_log`), +matching the other site's tail placement. + +### SF-4 — no test covers any of the three production LoginComplete → seeder hooks + +The wiring tests drive the login-complete half through the +`HeadlessSessionHost.OptionsSeeder` test seam +(`HeadlessCharacterOptionsSeederWiringTests.cs:66, 109, 124, 348`) and justify +it by saying the three production sites are "each simple one-line delegate +wiring already covered by their OWN existing tests +(`RuntimeLiveEntitySessionControllerTests`)" (`:29-37`). That justification does +not hold: those tests +(`tests/AcDream.Runtime.Tests/Session/RuntimeLiveEntitySessionControllerTests.cs:149-191`) +assert the LoginComplete *bytes* are sent and construct the controller without +the new `onLoginCompleteSent` argument — the commit added no test to any of +them, and `grep onLoginCompleteSent tests/` returns nothing. So all three +call-throughs (`RuntimeLiveEntitySessionController.cs:201`, `:878`, +`HeadlessSessionHost.cs:794-800`) are unverified. + +**Can the simulation hide an ordering bug?** I traced the orderings and found +none hiding today: the seeder is assigned at the *top* of `CreateEventRoute` +(`HeadlessSessionHost.cs:633-637`) before both the controller (`:722-734`) and +the bindings (`:770-783`), all three hooks read the field at invoke time (never +a captured instance), both arrival orders are unit-covered +(`HeadlessCharacterOptionsSeederTests.cs:123-167`), and generation reset clears +`HasServerSeed` per connect (`RuntimeCharacterState.cs:980-990` via +`RuntimeGenerationReset.cs:272`). The gap is regression protection, not a live +defect: a future edit that drops one of the three invokes, or wires it to the +wrong object, is invisible to the entire suite. + +**Fix:** one test per site — cheapest is a `RuntimeLiveEntitySessionController` +test passing a counting `Action` and asserting it fires exactly once after each +of the two send paths, plus a host-level assertion for the first-entry +callback. + +### SF-5 — the connected-gate script tells the coordinator to expect a `0x0005` that a batched id never sends + +`docs/research/2026-08-11-campaign-op-test-script.md`, OP7 §"Expected wire +sends on first connect": + +> One `SetSingleCharacterOption (0x0005)` for `SalvageMultiple` (id `0x22`) +> followed by exactly one `SetCharacterOptions (0x01A1)` blob flush + +`SalvageMultiple` is batched (`CharacterOptionTable.cs:145`, `autoSave: false`), +and `TrySetOption` only invokes `sendAutoSave` when `entry.IsAutoSave` +(`RuntimeCharacterState.cs:836-839`) — a batched id puts **nothing** on the +wire except the later `0x01A1`. The step-5 recipe repeats the error ("exactly +ONE `0x0005` + `0x01A1` pair for that id"). The parenthetical immediately after +the first bullet gets it right ("dirties the module"), so the doc contradicts +itself. Since the gate is a WireMCP loopback capture run by the user, this +would produce a false defect report or a false pass. + +**Fix:** reword both bullets — declared batched ids produce no `0x0005`; the +only observable send is the single trailing `0x01A1`. + +--- + +## NOTE + +* **N-1 — the seeder is silent and ignores every command result.** + `HeadlessCharacterOptionsSeeder.cs:131-135` discards the + `RuntimeCommandResult` from both `SetSingleOption` and `SaveOptions`, and the + class emits no diagnostic at all. A generation-mismatch or session-null + rejection (`DirectGameRuntimeCommandAdapter.cs:657-660`, `:683-686`) is + indistinguishable from "nothing to do". For a no-window bot whose gate is + "did the declared options land", one `_diagnostics.Message` line per seed + (count sent / flushed) would pay for itself. Note the partial-failure case + self-heals: a rejected `SetSingleOption` leaves local state unchanged so the + next hook re-diffs; a rejected `SaveOptions` after a successful local write + leaves the module dirty for the 480 s auto-save timer. +* **N-2 — generation captured once, before the loop** + (`HeadlessCharacterOptionsSeeder.cs:126`). If a generation change lands + mid-loop the remaining commands fail closed rather than crossing sessions. + Correct direction; worth knowing. +* **N-3 — the `OptionsSeeder` test seam is `internal`** on an `internal` type + (`HeadlessSessionHost.cs:337`), reachable only via `InternalsVisibleTo`. It + exposes `NoteLoginCompleteSent`/`NoteOptionsSeeded`, i.e. "force a diff" — + but both are idempotent and generation-gated, and the property mirrors the + pre-existing `Commands` seam. No production surface widened. +* **N-4 — doc-comment typo:** `HeadlessConfiguration.cs:60-64` says "Dictionary + **VALUES** bypass the loader's camelCase property-naming policy". It is the + dictionary **KEYS** that bypass it (the values are `bool`). The underlying + claim is correct — `JsonSerializerOptions.DictionaryKeyPolicy` is never set + (`HeadlessConfigurationLoader.cs:59-72`), so keys are read verbatim and the + PascalCase enum spelling is what the file must contain. +* **N-5 — two distinct JSON keys resolving to one id collapse silently.** + `ParseDeclaredCharacterOptions` (`HeadlessSessionHost.cs:822-825`) uses the + dictionary indexer, so `{"AutoTarget": true, "13": false}` keeps only the + last. Same root cause as SF-1; fixing SF-1 makes it unreachable. +* **N-6 — the reconnect wiring test asserts only absence** + (`HeadlessCharacterOptionsSeederWiringTests.cs:134`, `Assert.Empty`). It would + also pass if the second-session seeder were entirely dead. A flipped-value + reconnect case (declare the opposite of what session 1 persisted, assert one + send) would make it load-bearing. +* **N-7 — bookkeeping is correct as far as it goes.** "No register row" is the + right call: OP7 adds acdream-native bot tooling over wire mechanisms OP1 + already ported, and introduces no retail-behaviour deviation. The §9 ledger + row for OP7 is still `—`, matching OP3's own unfilled row — per §7/§8 that is + the coordinator's post-review amend, not the implementer's omission. + +--- + +## Lens coverage summary + +| Brief item | Result | +|---|---| +| 1 — 26-name allow-list vs §5.2 tier-1+2, esp. the three risky renames | ✅ all 26 correct; table in §1. `ToggleRun` is 0x0A run-as-default, not a keybind latch and not 0x04 | +| 2 — diff engine: only differences, `SetSingleOption` then exactly one `SaveOptions`, never interleaved, reconnect-idempotent | ✅ mechanism correct (`HeadlessCharacterOptionsSeeder.cs:118-136`); the seeder **duplicates** `TrySetOption`'s unchanged-value check rather than relying on it — deliberate and beneficial (it keeps `needsFlush` from being set by a no-op). Idempotence holds except SF-2's contradictory config | +| 3 — two-precondition latch, per-connect freshness, double-fire / second-PlayerDescription races | ✅ order-independent and re-entrancy-safe (`OnCharacterOptionsChanged` fires only from the PlayerDescription route, `LiveSessionEventRouter.cs:210-215`, never from our own writes); a fresh seeder per `CreateEventRoute` plus `Options.ResetSession` on generation reset makes the latch truly per-connect; a repeat `NoteLoginCompleteSent` or a second good PlayerDescription re-diffs and finds nothing. ❌ the *truncated* second PlayerDescription is MF-2 | +| 4 — #368 compliance | ✅ zero `async`/`await`/`Task`/`ThreadPool`/`ContinueWith` added under `src/`; `Start` and every scheduler turn run on the one dedicated thread (`HeadlessProcessHost.cs:163-199`), and the affinity test measures the send from inside `ILiveSessionOperations.Tick` | +| 5 — `RuntimeLiveEntitySessionController` out-of-contract touch | Additive and null-safe (optional ctor arg, `?.Invoke()`), and **not** reachable from the graphical host — the only production construction is `HeadlessSessionHost.cs:722`; App's own LoginComplete sites (`LiveSessionRuntimeFactory.cs:268`, `LocalPlayerTeleportController.cs:301`) are untouched and pass no callback. ⚠️ but see SF-3 on the portal-path invoke position | +| 6 — HeadlessConfiguration/Loader strict-schema behaviour for existing configs | ✅ byte-identical: the new property is optional, `UnmappedMemberHandling.Disallow` / `PropertyNameCaseInsensitive = false` / camelCase policy all unchanged (`:59-72`), `CurrentVersion` still 1, and the semantic-vs-type-shape error convention is followed (`HeadlessConfigurationException` for the name, raw `JsonException` for a non-bool). ⚠️ SF-1 is a strictness hole, not a compatibility break | +| 7 — HeadlessSessionHost wiring: reconnect leaks, test-seam surface | ✅ leak-free — the only strong reference to a seeder is `_optionsSeeder`; all three hooks close over `this` and read the field at invoke time, so reassignment in `CreateEventRoute` makes the old instance collectable and no stale hook can resurrect it. Test seam: see N-3 | +| 8 — does the simulated LoginComplete half hide an ordering bug? | No live ordering bug found (trace in SF-4), but the production wiring is entirely untested — SF-4 |