fix(saveslot): persist the Continue pointer apart from the active slot

[Quenty]

The bug

The "Continue" button disappears for a player who still has save slots.

HasSaveSlots kept one system-store key, activeSlotId, doing two jobs:

• which slot is selected right now
• which slot Continue resumes (LastActiveSlotId, PromiseLastActiveSlotId, PromiseSelectLastSaveSlot)

A deselect has to clear the first. So PromiseDeselectSlot — backing out to the title menu — wrote nil over the second as well. The resume id survived only in memory:

self._systemStore:Store("activeSlotId", active)
if active ~= nil then                 -- skipped on a deselect, so the memory below stands
    self._lastActiveSlotId = active
    self.LastActiveSlotId.Value = active
end

That in-memory value is why the behaviour looked correct in-session (and why the existing PromiseDeselectSlot spec passes). Nothing wrote it down, so the next session loaded activeSlotId → nil and came up with slots but nothing to continue on.

Repro: play a slot → return to the main menu → leave → rejoin. Continue is gone; the slot list is intact. A session that ends while still in the slot keeps the pointer, which is what made this look intermittent.

Downstream, egg-hunt-2026 drives the button straight off this property, so the menu offered Load Game and no Continue:

self._mainMenuPane:SetContinueVisible(lastActiveSlotId ~= nil)

The fix

• The resume pointer gets its own persisted key, SaveSlotConstants.LAST_ACTIVE_SLOT_ID_KEY, written on the same edge that already remembered it in memory.
• Load seeds activeSlotId or lastActiveSlotId, so a session that ended mid-play still resumes off the active slot; the new key only carries the case where the player ended at the menu.
• Every site that moves the pointer goes through one _setContinueSlotId — the in-memory value, the replicated property, and the persisted key can no longer drift apart.
• Both store keys are named in SaveSlotConstants rather than repeated as literals.

Also fixed, because the same invariant reaches it: deleting the pointed-at slot now clears the active-slot key too. A session that ended inside a slot leaves that key naming it, and load seeds Continue from it first — so a slot deleted from the menu could be offered again a session later, pointing at nothing.

No behaviour change for ephemeral slots: both guards in the selection hook are untouched, and the new key is only ever written from the real-slot branch.

Tests

Three tests in a new HasSaveSlots continue pointer across sessions block. Each drives a real rejoin — flush to the mock datastore, destroy the whole service bag, bind a fresh binder over the same stored bytes — so only what was persisted can survive:

• still has a slot to continue after a session that ended back at the menu
• continues on the slot selected last where a session used several
• has nothing to continue on once the slot it pointed at is deleted

All three fail on main (3 failed, 188 passed) and pass with the fix.

nevermore test --cloud --timeout 300   # 191/191
stylua / selene / luau-lsp analyze     # clean

No migration needed: any player who selects a slot writes the new key, and until then the existing activeSlotId still seeds the pointer. Players already in the broken state get Continue back the first time they enter a slot through Load Game.

https://claude.ai/code/session_01CTVd4jVT9i4U48vAfRoEVn

[github-actions]

Deploy Results

ℹ️ No changed packages with deploy targets were discovered for this PR. · View logs

Test Results

Package	Status	Try it
@quenty/saveslot	✅ Passed (198/198) (15.9s)	Open | Play

1 package tested, 1 passed, 0 failed in 21.0s · View logs

[Quenty]

Pushed a revision after an adversarial review pass. Three real defects came out of it; two were mine, one was pre-existing and directly in this change's blast radius.

1. PromiseDeleteAllSlots left the active-slot key naming a deleted slot (pre-existing, now fixed). It nils _lastActiveSlotId before the delete loop, so every PromiseDeleteSlot in that loop sees its slotId == self._lastActiveSlotId guard as false and the new clear never runs. ActiveSlotId.Value = nil on the line above doesn't cover it either — ValueObject._applyValue only fires Changed when previous ~= value, so deleting from the menu (nothing selected) writes nothing at all, and an ephemeral active slot hits the leavingEphemeralToMenu skip. Both pointers are now retired outright in that method. Repro: play a slot → leave → rejoin → PromiseDeleteAllSlots → rejoin, and the old code publishes a pointer to a slot that no longer exists.

2. Nothing validated the seeded pointer against the slot roster. Before this PR a stale pointer was self-healing, because the next deselect cleared the key. The whole point of the new key is that it survives a deselect — which also made staleness permanent. A slot deleted by another server left a Continue button that always rejects (EggHuntMainMenuService hard-rejects with "There is no save slot to continue"), and nothing would ever clear it. Load now publishes a pointer only if _slotMap actually has the slot, and retires both keys on disk when it doesn't.

3. The precedence comment was wrong. I documented activeSlotId or lastActiveSlotId as "the active slot is the more specific answer". Tracing every writer shows the two keys can never disagree — they move on the same edge — so that state is unreachable and the fallback is load-bearing for exactly one thing: data written before this commit, where activeSlotId is the only pointer an existing player has. Comment now says that, and there's a test pinning it, because a future "simplification" to read only the new key would silently strip Continue from every existing player.

Also fixed: the new resolve() test helper asserted and then fell through into promise:Yield(), which blocks forever on a pending promise — one failed assertion would have become a whole-run timeout with no Jest summary. It now errors, is hoisted to module scope (it was a copy of the ephemeral block's), and teardown moved to afterEach. That last one mattered: a test that failed mid-way skipped its destroy() and leaked a PlayerMock, which is refused as a second live mock and failed every subsequent test with a misleading id mismatch. It sent me chasing a phantom ephemeral-slot bug — the diagnostic showed both keys and the published pointer all correctly on the real slot.

Tests: 4 new (7 total in the block). Against origin/main, 5 of the 7 fail; the other two — the pre-key migration fallback and the ephemeral invariant — pass on main by construction and are regression guards, not repros. 195/195 with the fix, and stylua / selene / luau-lsp clean.

Not changed, for the record: previousActiveSlotId still seeds from activeId alone rather than the fallback. It is only ever read through _isEphemeral, and a persisted id is never ephemeral, so the asymmetry is inert — and it is pre-existing.

[Quenty]

Second review round. One real defect, two coverage gaps that mattered, and one finding I'm rejecting with evidence.

Fixed — PromiseDeleteSlot could clear a pointer naming a live slot. My guard inferred the persisted active slot's value from _lastActiveSlotId, justified by a comment claiming the two can never name different slots. That's wrong: PromiseResetSlot's non-active branch moves the Continue pointer alone, and it captures wasLastActive before awaiting _dataStore:Save(), so a selection landing in that window leaves the two genuinely divergent. Deleting the reset slot afterwards then wiped the pointer to the slot the player was in — reintroducing this PR's own bug as persisted data loss. Now tracked explicitly in _persistedActiveSlotId and retired only when it actually names the slot going away, so the safety property holds by construction instead of by argument. Both key writes go through _setPersistedActiveSlotId, mirroring _setContinueSlotId.

Fixed — the delete-path clears were untested. Both existing tests passed with those lines removed, because the load-time validation masked them. The discriminating case is a reset across a session boundary, and it's now covered: end a session inside a slot → next session resets it from the menu → rejoin must Continue on the fresh id. I verified it discriminates by deleting the clear and re-running — 1 failed, 195 passed, that test alone.

Fixed — the validation test didn't test the risky half. It ran with zero slots, so it proved "no slots ⇒ pointer dropped" rather than "dangling pointer dropped while the player's other slots survive". It now creates a surviving slot and asserts it's still there and still selectable afterwards, which is the assertion that would catch a false positive in the one block here that destroys persisted data.

Fixed — the resolve() error widened the leak surface. Making it throw meant a never-settled promise could abort an it body before its context.destroy() in the older describe blocks, which have no afterEach. Rather than add one per block, setup() now registers every session and a single module-level afterEach tears down whatever is still open; destroy() is idempotent so the existing per-test calls still work. That also removed the need for the openSession/closeSession wrappers I'd added.

Rejecting one finding. The review said there is no duplicate-mock refusal, citing PlayerMock.new. The refusal is real, it's just in a different class — PlayerMockServiceBase.lua:153, "Two %ss are alive at once (mock %s is already consumed)". I hit it verbatim in test output last round, which is what sent a passing ephemeral test red and had me briefly chasing a phantom ephemeral bug. The comment now names that class instead of "the mock".

Accepted, not fixed: rejoin awaits Save() rather than SaveAndCloseSession(), so it leans on best-effort lock release at teardown. SaveAndCloseSession asserts when session locking is disabled, which would make the helper brittle in the other direction; the suite has been stable across ~10 runs. And the reset-vs-selection interleaving above isn't directly tested — it needs a selection driven from inside a datastore await — so it's covered by construction rather than by a test. Flagging both rather than implying coverage I don't have.

Lint, repo-wide rather than just this package: stylua --check src games plugins, selene across every package via lerna, and luau-lsp analyze over all of src are clean. The typecheck did catch one thing in my new test — hoisting resolve let its inferred SlotId? return leak into call sites expecting SlotId, now pinned to any at the test boundary. lint:moonwave I could not run: moonwave-extractor 404s from the registry in this environment, and it fails identically on an untouched checkout, so it's an environment gap and not something this PR changed.

Tests: 9 in the cross-session block, 196/196 green.