From 87ee137e65f087ec2c4bf6b416e83d85e433b116 Mon Sep 17 00:00:00 2001 From: arandomogg Date: Sat, 29 Aug 2026 22:27:22 +0100 Subject: [PATCH] contrib: add migration checklist for breaking session schema changes Self-contained reference for issue #282 under contrib/examples. There is no standard process for safely rolling out a breaking change to the stored WalletSession schema across consumer versions. Sessions are persisted by the consumer's app, and restore() is deliberately fail-soft, so an unmigrated breaking change silently signs every existing user out on their next app load with no error surfaced anywhere. CHECKLIST.md covers backward compatibility, migration helper usage, the tests a migration PR must carry, and rollout, and asks migration PRs to reference it in their description. It is backed by a runnable implementation of a worked example, replacing a flat lastActiveAt field with a structured activity object, so the guidance is demonstrably correct rather than only asserted. Includes 18 tests exercising every item in checklist sections 1 to 3: both shapes accepted, invalid data still rejected, migration on read, idempotency, purity, and the fallback for the newly required field. --- .../CHECKLIST.md | 145 ++++++++++++++++ .../README.md | 52 ++++++ ...session-schema-migration-checklist.test.ts | 144 ++++++++++++++++ .../session-schema-migration-checklist.ts | 160 ++++++++++++++++++ 4 files changed, 501 insertions(+) create mode 100644 contrib/examples/issue-282-session-schema-migration-checklist/CHECKLIST.md create mode 100644 contrib/examples/issue-282-session-schema-migration-checklist/README.md create mode 100644 contrib/examples/issue-282-session-schema-migration-checklist/session-schema-migration-checklist.test.ts create mode 100644 contrib/examples/issue-282-session-schema-migration-checklist/session-schema-migration-checklist.ts diff --git a/contrib/examples/issue-282-session-schema-migration-checklist/CHECKLIST.md b/contrib/examples/issue-282-session-schema-migration-checklist/CHECKLIST.md new file mode 100644 index 0000000..cecf712 --- /dev/null +++ b/contrib/examples/issue-282-session-schema-migration-checklist/CHECKLIST.md @@ -0,0 +1,145 @@ +# Session schema migration checklist + +Use this checklist for any PR that makes a **breaking change to the stored +session schema** — `WalletSession` (`src/types.ts`), `isWalletSession` or the +storage adapters in `src/session.ts`, or anything else a consumer persists +across app versions. + +A change is **breaking** when a session object written by an older SDK version +would be misread, rejected, or silently corrupted by a newer one: renaming a +field, changing its type or allowed values, making an optional field required, +or splitting one field into several. Adding a new **optional** field is not +breaking on its own and does not need this checklist. + +> **Reference this checklist in your PR description.** A migration PR should +> link here and either check every box or mark one N/A with a reason, e.g. +> `Migration checklist: contrib/examples/issue-282-session-schema-migration-checklist/CHECKLIST.md`. +> Reviewers should ask for it when it's missing. + +## Why this exists + +`WalletSession` is persisted by the **consumer's app**, not by the SDK's own +process. A user can have a session written by SDK 0.6.x sitting in +`localStorage` for months before their app upgrades. `restore()` in +`src/session.ts` then reads that old object with the new code, and it is +deliberately fail-soft — unreadable storage means "disconnected", never a +crash. So a breaking schema change shipped without a migration path silently +signs every existing user out on their next app load, all at once, with no +error surfaced anywhere. + +## 1. Backward compatibility + +- [ ] **Old sessions still parse.** `isWalletSession` (or its replacement) + accepts BOTH the old and new shapes, for at least one full minor + version cycle. A session from the previous release must round-trip + through `restore()` without becoming `null`. +- [ ] **New required fields have a defined fallback for old data.** Decide and + document what a migrated-from-old-data session gets — a computed + default, not `undefined` for downstream code to trip over. +- [ ] **Removed/renamed fields are still read from old data** (not merely + dropped), so the migration step has something to read from. +- [ ] **The guard was widened, not disabled.** Genuinely invalid data must + still be rejected. Don't loosen it so far it stops rejecting garbage. +- [ ] **Storage adapters are unaffected**, or an adapter change is itself + covered by this checklist. Adapters are dumb load/save/clear — schema + logic belongs in the guard and the store. + +## 2. Migration helper usage + +- [ ] **A migration step upgrades old data rather than discarding it.** The + guard recognises the old shape and a helper upgrades it in memory before + the store accepts it. +- [ ] **The migration runs on READ** (`restore()` / `load()`), not at write + time. A consumer should never run an explicit "migrate my users" step. +- [ ] **It is pure and synchronous** — no network, no passkey prompt. If the + new schema genuinely can't be derived from old data, the change needs a + softer rollout, not a bigger migration function. +- [ ] **It is idempotent.** Running it twice — repeated `restore()` calls, or a + future migration chaining on top — gives the same result as running it + once. +- [ ] **It does not mutate its input.** + +## 3. Testing + +- [ ] A test constructs a session in the **old** shape and asserts the guard + and read path accept it, with the migrated result carrying the expected + new-shape fields. +- [ ] A test asserts a **new**-shape session round-trips unchanged (the + migration doesn't double-apply or corrupt current data). +- [ ] A test asserts genuinely invalid data still yields `null` / + disconnected, not a crash. +- [ ] A test asserts the fallback value for any newly required field. + +## 4. Rollout + +- [ ] Called out in `CHANGELOG.md` under a **Breaking** heading, naming the + exact fields affected and how long the backward-compatible read path + will be kept. +- [ ] If the migration can't fully preserve session state, the changelog says + what the user-visible effect is (e.g. "existing users will be prompted + to reconnect once"). +- [ ] A tracked follow-up exists to remove the compatibility read path once + enough time has passed, so it doesn't accumulate forever. + +## Worked example + +The hypothetical change: replace the flat `lastActiveAt: string` on +`WalletSession` with a structured +`activity: { lastActiveAt: string; lastActiveNetwork: Network }`, so a stored +session records which network the user was last active on. + +`session-schema-migration-checklist.ts` in this folder implements it, and the +test file exercises every item in sections 1–3 above. + +**1. Backward compatibility** — the guard branches on which shape it sees, and +still rejects anything that is neither: + +```ts +export function isStoredSession(value: unknown): boolean { + if (typeof value !== "object" || value === null) return false; + const v = value as Record; + if (!hasCoreFields(v)) return false; + + // New shape. + if (typeof v.activity === "object" && v.activity !== null) { + const a = v.activity as Record; + return typeof a.lastActiveAt === "string"; + } + // Old shape — still valid, migrated below. + return typeof v.lastActiveAt === "string"; +} +``` + +**2. Migration helper** — pure, synchronous, idempotent, with a defined +fallback (`network`) for the field that didn't exist in old data: + +```ts +export function migrateSession(stored: OldWalletSession | NewWalletSession): NewWalletSession { + if (isCurrentShape(stored)) return stored; // already current — idempotent + + const { lastActiveAt, ...rest } = stored; + return { + ...rest, + activity: { lastActiveAt, lastActiveNetwork: rest.network }, + }; +} +``` + +The read path is validate-then-migrate, returning `null` for anything +unrecognised to match `restore()`'s fail-soft posture: + +```ts +export function loadSession(raw: unknown): NewWalletSession | null { + if (!isStoredSession(raw)) return null; + return migrateSession(raw as OldWalletSession | NewWalletSession); +} +``` + +**3. Testing** — old-shape input migrates correctly with the right fallback; +new-shape input round-trips unchanged; migrating twice is stable; invalid data +returns `null`; the helper doesn't mutate its input. + +**4. Rollout** — `CHANGELOG.md` notes that `lastActiveAt` is replaced by +`activity.lastActiveAt` / `activity.lastActiveNetwork`, that both shapes are +read for the next two minor versions, and links a tracking issue to drop the +old-shape read path afterwards. diff --git a/contrib/examples/issue-282-session-schema-migration-checklist/README.md b/contrib/examples/issue-282-session-schema-migration-checklist/README.md new file mode 100644 index 0000000..33f0f1d --- /dev/null +++ b/contrib/examples/issue-282-session-schema-migration-checklist/README.md @@ -0,0 +1,52 @@ +# Session schema migration checklist + +Self-contained reference for issue [#282](https://github.com/Vellar-Wallet/vellar-sdk/issues/282): a standard checklist for safely rolling out a breaking change to the stored session schema across consumer versions. + +**The checklist itself is in [CHECKLIST.md](CHECKLIST.md).** + +## What's here + +| File | What it is | +| ---- | ---------- | +| [`CHECKLIST.md`](CHECKLIST.md) | The checklist — backward compatibility, migration helper usage, testing, rollout — plus the worked example. | +| `session-schema-migration-checklist.ts` | A runnable implementation of that worked example, so the guidance is demonstrably correct rather than only asserted. | +| `session-schema-migration-checklist.test.ts` | Tests exercising every item in checklist sections 1–3. | + +## Why a checklist is needed + +`WalletSession` is persisted by the **consumer's app** (`localStorage`, or +their own `SessionStorageAdapter`), not by the SDK's own process. A user can +carry a session written by an older SDK for months before their app upgrades. +`restore()` in `src/session.ts` is deliberately fail-soft — unreadable storage +means "disconnected", never a crash — so an unmigrated breaking change +silently signs every existing user out on their next app load, with no error +surfaced anywhere. + +## Referencing it in a migration PR + +A PR that breaks the session schema should link the checklist and check the +boxes (or mark one N/A with a reason): + +``` +Migration checklist: contrib/examples/issue-282-session-schema-migration-checklist/CHECKLIST.md +``` + +## The worked example + +Replacing the flat `lastActiveAt: string` with a structured +`activity: { lastActiveAt, lastActiveNetwork }`. The guard accepts both +shapes, a pure and idempotent `migrateSession` upgrades old data on read with +a defined fallback for the field that didn't exist before, and invalid data is +still rejected. + +## Run it + +```sh +npx tsx session-schema-migration-checklist.ts +``` + +## Tests + +```sh +npx vitest run contrib/examples/issue-282-session-schema-migration-checklist +``` diff --git a/contrib/examples/issue-282-session-schema-migration-checklist/session-schema-migration-checklist.test.ts b/contrib/examples/issue-282-session-schema-migration-checklist/session-schema-migration-checklist.test.ts new file mode 100644 index 0000000..07ce3c3 --- /dev/null +++ b/contrib/examples/issue-282-session-schema-migration-checklist/session-schema-migration-checklist.test.ts @@ -0,0 +1,144 @@ +import { describe, expect, it } from "vitest"; +import { + isCurrentShape, + isStoredSession, + loadSession, + migrateSession, + type NewWalletSession, + type OldWalletSession, +} from "./session-schema-migration-checklist"; + +const ACCOUNT = "CAFIATCEAZJTGQQKFL3N2YB6VMCUN2UYX4QD5A3FALDRU7UJJ6OWBKOW"; + +const oldSession: OldWalletSession = { + accountId: ACCOUNT, + network: "testnet", + connected: true, + authMethod: "passkey", + createdAt: "2026-07-16T10:00:00.000Z", + lastActiveAt: "2026-07-16T10:30:00.000Z", +}; + +const newSession: NewWalletSession = { + accountId: ACCOUNT, + network: "mainnet", + connected: true, + authMethod: "passkey", + createdAt: "2026-07-16T10:00:00.000Z", + activity: { + lastActiveAt: "2026-07-16T10:30:00.000Z", + lastActiveNetwork: "mainnet", + }, +}; + +// Checklist item 1: backward compatibility. +describe("isStoredSession accepts both shapes (backward compatibility)", () => { + it("accepts a session in the OLD shape", () => { + expect(isStoredSession(oldSession)).toBe(true); + }); + + it("accepts a session in the NEW shape", () => { + expect(isStoredSession(newSession)).toBe(true); + }); + + it("still rejects genuinely invalid data", () => { + expect(isStoredSession(null)).toBe(false); + expect(isStoredSession(undefined)).toBe(false); + expect(isStoredSession("a string")).toBe(false); + expect(isStoredSession({})).toBe(false); + expect(isStoredSession({ nope: true })).toBe(false); + }); + + it("rejects a session missing a core field", () => { + const { accountId: _omitted, ...missingAccount } = oldSession; + expect(isStoredSession(missingAccount)).toBe(false); + }); + + it("rejects a session with an invalid network", () => { + expect(isStoredSession({ ...oldSession, network: "devnet" })).toBe(false); + }); + + it("rejects a session carrying neither lastActiveAt nor activity", () => { + const { lastActiveAt: _omitted, ...neither } = oldSession; + expect(isStoredSession(neither)).toBe(false); + }); +}); + +// Checklist item 2: migration helper usage. +describe("migrateSession upgrades old data on read", () => { + it("moves lastActiveAt into the structured activity field", () => { + const migrated = migrateSession(oldSession); + expect(migrated.activity.lastActiveAt).toBe("2026-07-16T10:30:00.000Z"); + }); + + it("falls back to the session's own network for the new lastActiveNetwork field", () => { + expect(migrateSession(oldSession).activity.lastActiveNetwork).toBe("testnet"); + expect(migrateSession({ ...oldSession, network: "mainnet" }).activity.lastActiveNetwork).toBe( + "mainnet", + ); + }); + + it("preserves every other field unchanged", () => { + const migrated = migrateSession(oldSession); + expect(migrated.accountId).toBe(oldSession.accountId); + expect(migrated.network).toBe(oldSession.network); + expect(migrated.connected).toBe(oldSession.connected); + expect(migrated.authMethod).toBe(oldSession.authMethod); + expect(migrated.createdAt).toBe(oldSession.createdAt); + }); + + it("drops the superseded flat lastActiveAt field", () => { + expect("lastActiveAt" in migrateSession(oldSession)).toBe(false); + }); + + it("is idempotent — migrating an already-current session changes nothing", () => { + const once = migrateSession(oldSession); + const twice = migrateSession(once); + expect(twice).toEqual(once); + }); + + it("returns an already-current session as-is", () => { + expect(migrateSession(newSession)).toBe(newSession); + }); + + it("is pure — it does not mutate its input", () => { + const input: OldWalletSession = { ...oldSession }; + migrateSession(input); + expect(input.lastActiveAt).toBe("2026-07-16T10:30:00.000Z"); + expect("activity" in input).toBe(false); + }); +}); + +describe("isCurrentShape", () => { + it("distinguishes the new shape from the old", () => { + expect(isCurrentShape(newSession)).toBe(true); + expect(isCurrentShape(oldSession)).toBe(false); + }); +}); + +// Checklist item 3: the tests a migration PR is required to carry. +describe("loadSession: the read path restore() would use", () => { + it("accepts old-shape data and returns it migrated", () => { + const loaded = loadSession(oldSession); + expect(loaded).not.toBeNull(); + expect(loaded!.activity).toEqual({ + lastActiveAt: "2026-07-16T10:30:00.000Z", + lastActiveNetwork: "testnet", + }); + }); + + it("round-trips new-shape data unchanged (no double-application)", () => { + expect(loadSession(newSession)).toEqual(newSession); + }); + + it("returns null for invalid data rather than throwing", () => { + expect(loadSession({ nope: true })).toBeNull(); + expect(loadSession(null)).toBeNull(); + expect(loadSession("garbage")).toBeNull(); + }); + + it("a migrated session re-loaded stays stable", () => { + const once = loadSession(oldSession); + expect(loadSession(once)).toEqual(once); + }); +}); diff --git a/contrib/examples/issue-282-session-schema-migration-checklist/session-schema-migration-checklist.ts b/contrib/examples/issue-282-session-schema-migration-checklist/session-schema-migration-checklist.ts new file mode 100644 index 0000000..1f6341b --- /dev/null +++ b/contrib/examples/issue-282-session-schema-migration-checklist/session-schema-migration-checklist.ts @@ -0,0 +1,160 @@ +// Self-contained reference for issue #282: a checklist for safely rolling out +// a BREAKING change to the stored session schema, backed by a runnable worked +// example of the migration pattern the checklist prescribes. +// +// See CHECKLIST.md in this folder for the checklist itself. This file is the +// executable half: it implements the hypothetical schema change the checklist +// walks through, so the guidance is demonstrably correct rather than only +// asserted. +// +// THE HYPOTHETICAL CHANGE: replace the flat `lastActiveAt: string` field on +// WalletSession with a structured `activity: { lastActiveAt, lastActiveNetwork }`, +// so a stored session records which network the user was last active on. +// +// WHY THIS NEEDS A MIGRATION: WalletSession is persisted by the CONSUMER's app +// (localStorage, or their own SessionStorageAdapter), not by the SDK's own +// process. A user can carry a session written by an older SDK for months +// before their app upgrades. src/session.ts's restore() is deliberately +// fail-soft — unreadable storage means "disconnected", never a crash — so an +// unmigrated breaking change silently signs every existing user out on their +// next app load, with no error surfaced anywhere. +// +// Run with: npx tsx session-schema-migration-checklist.ts + +export type Network = "testnet" | "mainnet"; + +/** The session shape as stored by the PREVIOUS release. */ +export interface OldWalletSession { + accountId: string; + network: Network; + connected: boolean; + authMethod: "passkey"; + createdAt: string; + lastActiveAt: string; +} + +/** The session shape after the hypothetical breaking change. */ +export interface NewWalletSession { + accountId: string; + network: Network; + connected: boolean; + authMethod: "passkey"; + createdAt: string; + activity: { + lastActiveAt: string; + lastActiveNetwork: Network; + }; +} + +/** Fields common to both shapes — what the guard can check before branching. */ +function hasCoreFields(v: Record): boolean { + return ( + typeof v.accountId === "string" && + v.accountId.length > 0 && + (v.network === "testnet" || v.network === "mainnet") && + typeof v.connected === "boolean" && + v.authMethod === "passkey" && + typeof v.createdAt === "string" + ); +} + +/** + * CHECKLIST ITEM 1 (backward compatibility): the guard accepts BOTH shapes. + * + * A session written by the previous release must still be recognised, or + * restore() drops it and the user is silently signed out. Genuinely invalid + * data must still be rejected — the guard is loosened to accept one extra + * known shape, not loosened into accepting anything. + */ +export function isStoredSession(value: unknown): boolean { + if (typeof value !== "object" || value === null) return false; + const v = value as Record; + if (!hasCoreFields(v)) return false; + + // New shape. + if (typeof v.activity === "object" && v.activity !== null) { + const a = v.activity as Record; + return typeof a.lastActiveAt === "string"; + } + // Old shape — still valid, migrated by migrateSession below. + return typeof v.lastActiveAt === "string"; +} + +/** True when `value` is already in the current (new) shape. */ +export function isCurrentShape(value: unknown): value is NewWalletSession { + if (typeof value !== "object" || value === null) return false; + const v = value as Record; + return typeof v.activity === "object" && v.activity !== null; +} + +/** + * CHECKLIST ITEM 2 (migration helper): upgrade an old-shape session in memory. + * + * Properties the checklist requires of this function, all exercised by the + * tests in this folder: + * + * - PURE and SYNCHRONOUS — no network, no passkey prompt. A migration that + * needs either is a sign the change needs a softer rollout, not a bigger + * migration function. + * - IDEMPOTENT — running it on an already-current session returns it + * unchanged, so repeated restore() calls and future chained migrations + * stay safe. + * - DEFINED FALLBACK for the new field — `lastActiveNetwork` did not exist + * in old data, so it falls back to the session's own `network`, the best + * available evidence, rather than being left undefined for downstream + * code to trip over. + */ +export function migrateSession(stored: OldWalletSession | NewWalletSession): NewWalletSession { + if (isCurrentShape(stored)) return stored; // already current — idempotent + + const { lastActiveAt, ...rest } = stored; + return { + ...rest, + activity: { + lastActiveAt, + lastActiveNetwork: rest.network, + }, + }; +} + +/** + * The read path a consumer's storage adapter / restore() would use: + * validate, then migrate. Returns `null` for anything unrecognised, matching + * src/session.ts's fail-soft posture. + * + * CHECKLIST ITEM 2 also requires the migration run on READ, not write — a + * consumer should never have to run an explicit "migrate my users" step. + */ +export function loadSession(raw: unknown): NewWalletSession | null { + if (!isStoredSession(raw)) return null; + return migrateSession(raw as OldWalletSession | NewWalletSession); +} + +function main() { + const oldSession: OldWalletSession = { + accountId: "CAFIATCEAZJTGQQKFL3N2YB6VMCUN2UYX4QD5A3FALDRU7UJJ6OWBKOW", + network: "testnet", + connected: true, + authMethod: "passkey", + createdAt: "2026-07-16T10:00:00.000Z", + lastActiveAt: "2026-07-16T10:30:00.000Z", + }; + + const migrated = loadSession(oldSession); + console.log("old shape -> migrated:", JSON.stringify(migrated?.activity)); + + // Idempotent: migrating the result again changes nothing. + const twice = loadSession(migrated); + console.log("migrated twice equal :", JSON.stringify(twice) === JSON.stringify(migrated)); + + // A session already in the new shape round-trips untouched. + console.log("new shape passthrough :", JSON.stringify(loadSession(migrated)?.activity)); + + // Garbage is still rejected — the guard was widened, not disabled. + console.log("garbage rejected :", loadSession({ nope: true }) === null); + console.log("null rejected :", loadSession(null) === null); +} + +if (import.meta.url === `file://${process.argv[1]}`) { + main(); +}