Canonicalize the account before any family image reads it

Addresses the second review.

The emote-collection migration ran inside the Family-4 builder, but the
subscription path builds the Family-3 roster before the Family-4 companion,
and Family 0 reads the account directly without passing through that builder
at all. A Family-3 character record could therefore name the individual emote
instance that the Family-4 manifest had already replaced, with no correction
published afterwards.

It now runs in push::ensure_account_canonical(), ahead of the family-specific
dispatch rather than inside one family's builder, and is called from every
public entry point that reaches a builder: the subscription path, the Family-4
resync, both banner paths, and both account resync paths. The last two are
reachable only behind their own familyNActive guards, so they were already
covered transitively, but calling it there keeps the invariant true by
construction instead of by an argument about reachability. Settling latches in
an atomic, so the repeated calls cost one relaxed load.

ensure_character_emote_collection() now reports why it stopped instead of
returning true for every non-failure. "not ready" (build data or account not
published yet) is distinguished from "unsupported" (the domains are published
and the content still does not carry the item, which cannot change under a
running process, so the preflight latches and stops retrying), and each is
logged with its reason. The extraction path treats only a genuine failure as
fatal: letting an unfinished extraction short-circuit that && chain would stop
build_data::persist() from ever writing the cache.

Repairing an existing collection item now copies it and mutates only what the
repair owns -- the definition hash, the sockets and the mutation serial -- so
accumulated item-state flags survive instead of being zeroed. The socket block
is still replaced whole, because the lanes past the used prefix have to be
empty for it to validate. A fresh grant still builds a canonical item.

Sunrise/src/client/content/investment/investment_refresh.cpp

@@ -35,6 +35,18 @@ SRWLOCK g_refreshLock{SRWLOCK_INIT};
            && state::build_data::investment_constants_ready();
 }
 
+/**
+ * Runs the emote-collection canonicalization on the extraction path, where it is an opportunistic
+ * head start rather than a precondition: the snapshot path runs the same step behind its own
+ * preflight, so nothing here is the last chance to apply it.
+ * @return False only when the account itself could not be updated, which is the one outcome that
+ * says something is wrong rather than merely unfinished. A build that cannot carry the item, and
+ * one whose data is still being extracted, both leave the cache worth writing.
+ */
+[[nodiscard]] bool emote_collection_settled() noexcept {
+    return state::ensure_character_emote_collection() != state::EmoteCollectionOutcome::failed;
+}
+
 } // namespace
 
 /** @return True when the next refresh slice needs a visible overlay for a package sweep. */
@@ -50,7 +62,7 @@ bool refresh() noexcept {
         AcquireSRWLockExclusive(&g_refreshLock);
         const bool persisted = state::ensure_profile_item_identities()
                                && state::ensure_character_subclasses()
-                               && state::ensure_character_emote_collection()
+                               && emote_collection_settled()
                                && state::build_data::persist();
         // Nothing reads a package again until the next boot, so the open files and the held
         // tables go back now rather than at process exit.
@@ -70,7 +82,7 @@ bool refresh() noexcept {
     const bool domainsReady = ready();
     const bool complete = domainsReady && state::ensure_profile_item_identities()
                           && state::ensure_character_subclasses()
-                          && state::ensure_character_emote_collection()
+                          && emote_collection_settled()
                           && state::build_data::persist();
     // The overlay ends with the work, not with the slice, so it spans every retry the pass needs.
     if (complete) {

Sunrise/src/server/bap/encrypted/internal.h

@@ -210,6 +210,18 @@ namespace body {
 /** Owns server-initiated encrypted frames appended after correlated replies. */
 namespace push {
 
+/**
+ * Canonicalizes the account ahead of the family-specific snapshot dispatch.
+ * Families 0, 3 and 4 each take their own account snapshot, and the roster is built before the
+ * account companion, so a migration performed inside one family's builder would leave the others
+ * describing a different account: a Family-3 character record naming an emote instance the
+ * Family-4 manifest has already replaced, with no correction published afterwards. Running it
+ * ahead of every builder is what keeps the three images talking about one account.
+ * Idempotent, and one relaxed load once the answer can no longer change, so calling it from every
+ * entry point that reaches a builder costs nothing.
+ */
+void ensure_account_canonical() noexcept;
+
 /**
  * Appends the queuez snapshots one subscription needs, including the Family-4 companion.
  * A snapshot that cannot be built is reported and skipped. The subscribe is answered either way,

Sunrise/src/server/bap/encrypted/push/queuez/queuez_account_preflight.cpp

@@ -0,0 +1,64 @@
+#include <array>
+#include <atomic>
+#include <cstdio>
+
+#include "../../../../../core/logging/log.h"
+#include "../../../../../state/runtime/runtime.h"
+#include "../../internal.h"
+
+namespace sunrise::server::bap::encrypted::push {
+namespace {
+
+/**
+ * Set once the answer can no longer change within this process, so the common path costs one
+ * relaxed load rather than a lock and a whole account copy on every pushed frame.
+ */
+std::atomic<bool> g_settled{false};
+
+/**
+ * Reports a preflight that left the account uncanonical, naming which of the two reasons it was.
+ * Silence here would be indistinguishable from a migration that ran, which is the confusion this
+ * whole preflight exists to remove.
+ */
+void report(const char* reason) noexcept {
+    std::array<char, 96> line{};
+    const int written = std::snprintf(line.data(),
+                                      line.size(),
+                                      "ev=queuez stage=account_preflight result=skip reason=%s",
+                                      reason);
+    if (written > 0) {
+        core::log::write(core::log::Channel::server,
+                         core::log::Level::warn,
+                         {line.data(), static_cast<std::size_t>(written)});
+    }
+}
+
+} // namespace
+
+/** Canonicalizes the account before any family image is allowed to read it. */
+void ensure_account_canonical() noexcept {
+    if (g_settled.load(std::memory_order_acquire)) {
+        return;
+    }
+    switch (state::ensure_character_emote_collection()) {
+    case state::EmoteCollectionOutcome::ready:
+        g_settled.store(true, std::memory_order_release);
+        break;
+    case state::EmoteCollectionOutcome::unsupported:
+        // The installed content decides this one and cannot change under a running process, so
+        // the verdict is final. Reported once rather than on every frame that follows.
+        g_settled.store(true, std::memory_order_release);
+        report("unsupported");
+        break;
+    case state::EmoteCollectionOutcome::notReady:
+        // Content extraction or account setup has not finished. Every family reads the same
+        // un-migrated account meanwhile, so they still agree with each other.
+        report("not_ready");
+        break;
+    case state::EmoteCollectionOutcome::failed:
+        report("failed");
+        break;
+    }
+}
+
+} // namespace sunrise::server::bap::encrypted::push

Sunrise/src/server/bap/encrypted/push/queuez/queuez_banner_push.cpp

@@ -164,6 +164,9 @@ bool append_banner_notification(Scratch& scratch,
                                 std::size_t& written,
                                 queuez::SessionState& after) noexcept {
     after = before;
+    // Before the account is read, so this pair cannot describe a different account than the
+    // family-three roster or the family-four manifest.
+    ensure_account_canonical();
     // The pair names the first character when none is picked yet. The client's family-zero record
     // accepts a snapshot for about ten seconds, then clears the family and refuses every later
     // one, so holding the pair for the pick spends that window and the subscription times out.
@@ -240,6 +243,7 @@ bool append_banner_move_notification(Scratch& scratch,
     bool publish = false;
     bool incremental = false;
     after = before;
+    ensure_account_canonical();
     // A family zero with no first delivery yet has no ladder to move, and no root to name it with.
     const char* reason = nullptr;
     if (!queuez::stage_family0_subscription(
@@ -497,6 +501,7 @@ bool append_account_resync_appearance_notification(
     std::size_t& written,
     queuez::SessionState& after) noexcept {
     after = before;
+    ensure_account_canonical();
     if (!before.family0Active) {
         return true;
     }
@@ -539,6 +544,7 @@ bool append_account_resync_roster_notification(Scratch& scratch,
                                                std::size_t& written,
                                                queuez::SessionState& after) noexcept {
     after = before;
+    ensure_account_canonical();
     if (!before.family3Active) {
         return true;
     }

Sunrise/src/server/bap/encrypted/push/queuez/queuez_subscription.cpp

@@ -90,6 +90,7 @@ bool append_account_resync_notification(Scratch& scratch,
                                         std::size_t& written,
                                         queuez::SessionState& after) noexcept {
     after = before;
+    ensure_account_canonical();
     if (!queuez::valid(before) || !before.family4Active || before.family4RootSoid == 0
         || before.family4Version == (std::numeric_limits<std::int32_t>::max)()) {
         return false;
@@ -146,6 +147,10 @@ void append_queuez_notification(Scratch& scratch,
     after = before;
     armsRepush = false;
     armsBannerRepush = false;
+    // Ahead of the dispatch below, not inside one family's builder: family zero reads the account
+    // directly and family three is built before the family-four companion, so a migration run any
+    // later would leave the three images describing different accounts.
+    ensure_account_canonical();
     if (subscription.familyType == queuez::kAccountFamilyType && before.family4Active
         && before.family4Version != queuez::kInitialFamilyVersion) {
         // Our mirror of the Client's records is an observation, not an authority on what may be

Sunrise/src/server/bap/encrypted/push/snapshot/family4_snapshot_preparer.cpp

@@ -68,13 +68,10 @@ bool prepare(Scratch& scratch,
     if (!state::ensure_profile_item_identities()) {
         return report_failure("profile_identities");
     }
-    // The investment-refresh migration runs on the content-extraction path, which is not
-    // guaranteed to finish before the first Family-4 subscription is served on a cache-hit boot.
-    // Repeating it here, idempotently, is the only boundary that is actually ordered ahead of
-    // every possible first image.
-    if (!state::ensure_character_emote_collection()) {
-        return report_failure("emote_collection");
-    }
+    // The emote-collection canonicalization deliberately does not live here. Family zero and
+    // family three build their own images from the same account and neither passes through this
+    // function, so it runs in the shared preflight ahead of the whole dispatch instead
+    // (push::ensure_account_canonical).
     const state::AccountState account = state::account_snapshot();
     if (!state::account::valid(account)) {
         return report_failure("account_state");

Sunrise/src/state/runtime/runtime.h

@@ -15,6 +15,18 @@ namespace sunrise::state {
  */
 [[nodiscard]] bool ensure_profile_item_identities() noexcept;
 
+/** Why one attempt to canonicalize the "Emotes" collection item ended. */
+enum class EmoteCollectionOutcome : std::uint8_t {
+    /** Every character carries a sound collection item, either already or as of this call. */
+    ready,
+    /** The build data or account this reads is not published yet, so a retry is still owed. */
+    notReady,
+    /** The installed content does not carry the item this expects, so it can never be applied. */
+    unsupported,
+    /** The item could not be placed, so no character was changed and a retry is still owed. */
+    failed,
+};
+
 /**
  * Grants each character the other 2 subclasses of its equipped subclass's class, placing missing
  * ones into unequipped inventory with native socket defaults. Idempotent: one already equipped or
@@ -63,11 +75,12 @@ struct PendingSubclassSelection {
  * item; its 4 ordinary sockets seed default lanes from the item's real plug pool so the wheel has
  * something in every slot the first time it opens.
  * Idempotent, and safe to call from more than one boundary: a character already carrying a sound
- * copy is left alone, one whose sockets no longer resolve is repaired in place and keeps its
- * existing instance identity, and a build whose content does not match what this expects is
- * skipped rather than reported as a failure.
+ * copy is left alone, and one whose sockets no longer resolve is repaired in place, keeping its
+ * instance identity and every field this does not own.
+ * The outcome distinguishes "nothing to do" from "could not be done", so a caller never records
+ * the account as canonical on the strength of a prerequisite that was never met.
  */
-[[nodiscard]] bool ensure_character_emote_collection() noexcept;
+[[nodiscard]] EmoteCollectionOutcome ensure_character_emote_collection() noexcept;
 
 /** Direction of one checked character equipment mutation. */
 enum class EquipmentMutationKind : std::uint8_t {

Sunrise/src/state/runtime/state_account_acquisition_runtime.cpp

@@ -678,25 +678,29 @@ resolve_emote_collection_definition(build_data::items::Definition& definition) n
  * request (opcode 1901) lets the player reassign them afterward, the same mechanism it already uses
  * for weapon mods and shaders.
  */
-bool ensure_character_emote_collection() noexcept {
+EmoteCollectionOutcome ensure_character_emote_collection() noexcept {
     constexpr std::size_t kEmoteCollectionSlot =
         static_cast<std::size_t>(authored_inventory::EquipmentSlot::emote);
 
-    // The installed content must actually match what this migration assumes before anything is
-    // touched. A build whose "Emotes" collection item doesn't resolve this way yet -- rather than
-    // being wrong -- just isn't ready for the migration; skip this boot without failing the whole
-    // refresh, the same way the account-not-ready check below does.
+    // The domains every check below reads have to be published first. Until they are, nothing can
+    // be concluded about the installed content, so this is a retry rather than a verdict.
+    if (!build_data::item_definitions_ready() || !build_data::configured_item_details_ready()
+        || !build_data::socket_plug_rules_ready()) {
+        return EmoteCollectionOutcome::notReady;
+    }
+    // With those published, an item that still does not resolve this way is a build that cannot
+    // carry the wheel at all. Retrying that within this process would never change the answer.
     build_data::items::Definition collectionDefinition{};
     if (!resolve_emote_collection_definition(collectionDefinition)
         || !default_plugs_valid(collectionDefinition.definitionIndex)) {
-        return true;
+        return EmoteCollectionOutcome::unsupported;
     }
 
     AcquireSRWLockExclusive(&runtime::storage::g_stateLock);
     AccountState candidate = runtime::storage::g_state.account;
     if (!account::valid(candidate)) {
         ReleaseSRWLockExclusive(&runtime::storage::g_stateLock);
-        return true;
+        return EmoteCollectionOutcome::notReady;
     }
     bool changed = false;
     bool failed = false;
@@ -715,18 +719,26 @@ bool ensure_character_emote_collection() noexcept {
             failed = true;
             break;
         }
-        // A repair keeps the existing instance identity; only a fresh grant needs a new one.
-        std::uint64_t instanceSoid = present ? collectionSlot->instanceSoid : 0;
-        if (!present && !next_item_instance_soid(candidate, instanceSoid)) {
-            failed = true;
-            break;
+        // A repair owns only the definition, the sockets and the serial. Everything else the item
+        // already carries, the accumulated item-state flags above all, belongs to the player and
+        // survives. The account was checked whole on entry, so a present item's remaining scalars
+        // are already known good and need no normalizing here.
+        authored_inventory::Item granted = present ? *collectionSlot : authored_inventory::Item{};
+        if (!present) {
+            std::uint64_t instanceSoid = 0;
+            if (!next_item_instance_soid(candidate, instanceSoid)) {
+                failed = true;
+                break;
+            }
+            granted.instanceSoid = instanceSoid;
+            granted.level = 0;
+            granted.quantity = 1;
         }
-        authored_inventory::Item granted{};
-        granted.instanceSoid = instanceSoid;
         granted.definitionHash = authored_inventory::kEmoteCollectionDefinitionHash;
-        granted.level = 0;
-        granted.quantity = 1;
         granted.mutationSerial = static_cast<std::int32_t>(character.nextInventorySerial++);
+        // Replaced whole rather than edited: the lanes past the used prefix have to be empty for
+        // the socket block to validate, whatever the malformed copy left behind.
+        granted.sockets = authored_inventory::Sockets{};
         granted.sockets.policy = authored_inventory::SocketPolicy::authored;
         granted.sockets.plugCount = kEmoteCollectionDefaultPlugHashes.size();
         for (std::size_t lane = 0; lane < kEmoteCollectionDefaultPlugHashes.size(); ++lane) {
@@ -737,19 +749,19 @@ bool ensure_character_emote_collection() noexcept {
     }
     if (failed) {
         ReleaseSRWLockExclusive(&runtime::storage::g_stateLock);
-        return false;
+        return EmoteCollectionOutcome::failed;
     }
     if (!changed) {
         ReleaseSRWLockExclusive(&runtime::storage::g_stateLock);
-        return true;
+        return EmoteCollectionOutcome::ready;
     }
     if (!account::valid(candidate)) {
         ReleaseSRWLockExclusive(&runtime::storage::g_stateLock);
-        return false;
+        return EmoteCollectionOutcome::failed;
     }
     runtime::storage::g_state.account = candidate;
     ReleaseSRWLockExclusive(&runtime::storage::g_stateLock);
-    return true;
+    return EmoteCollectionOutcome::ready;
 }
 
 } // namespace sunrise::state