Review fixes for #365: gate exemptions, lossless autosave, migration fallback, add-in OAuth hardening

backend/src/__tests__/integration/user.routes.test.ts

@@ -48,15 +48,20 @@ const {
 // ---------------------------------------------------------------------------
 type QueryResult = { data: unknown; error: unknown };
 
+// A table entry may be a queue of results: each query consumes the next one,
+// and the last repeats. Lets tests drive the selectProfile fallback cascade
+// (first select fails with 42703, the retry succeeds).
 let supabaseState: {
-    tables: Record<string, QueryResult>;
+    tables: Record<string, QueryResult | QueryResult[]>;
+    updates: Record<string, unknown[]>;
     adminGetUserById: QueryResult;
     adminDeleteUser: { error: unknown };
 };
 
 function resetSupabaseState() {
     supabaseState = {
         tables: {},
+        updates: {},
         adminGetUserById: {
             data: { user: { id: "u1", factors: [] } },
             error: null,
@@ -67,7 +72,13 @@ function resetSupabaseState() {
 resetSupabaseState();
 
 function resultForTable(table: string): QueryResult {
-    return supabaseState.tables[table] ?? { data: null, error: null };
+    const entry = supabaseState.tables[table];
+    if (Array.isArray(entry)) {
+        return entry.length > 1
+            ? (entry.shift() as QueryResult)
+            : (entry[0] ?? { data: null, error: null });
+    }
+    return entry ?? { data: null, error: null };
 }
 
 function makeQuery(table: string) {
@@ -95,6 +106,12 @@ function makeQuery(table: string) {
         "contains",
     ];
     for (const m of chain) q[m] = vi.fn(() => q);
+    // Record update payloads so tests can assert what a route WROTE (the
+    // per-table result stub only models what queries return).
+    q.update = vi.fn((payload: unknown) => {
+        (supabaseState.updates[table] ??= []).push(payload);
+        return q;
+    });
     q.single = vi.fn(() => Promise.resolve(resultForTable(table)));
     q.maybeSingle = vi.fn(() => Promise.resolve(resultForTable(table)));
     q.then = (
@@ -304,6 +321,85 @@ describe("user.routes", () => {
             expect(requireMfaIfEnrolled).not.toHaveBeenCalled();
         });
 
+        it("keeps saved preferences on a database without the onboarding migration", async () => {
+            // Replicated live (PR #365 review): with the 20260821 columns
+            // dropped, the profile select failed on "jurisdiction", skipped
+            // every fallback tier, and silently reset legal_research_us and
+            // quick_actions_visible to defaults. The retry tier must preserve
+            // the user's saved values and report legacy-exempt onboarding.
+            const preMigrationRow = {
+                display_name: "Ada",
+                organisation: "Acme",
+                message_credits_used: 3,
+                credits_reset_date: "2999-01-01T00:00:00.000Z",
+                tier: "Pro",
+                title_model: null,
+                tabular_model: "gemini-3-flash-preview",
+                mfa_on_login: false,
+                legal_research_us: false,
+                quick_actions_visible: false,
+            };
+            supabaseState.tables.user_profiles = [
+                {
+                    data: null,
+                    error: {
+                        code: "42703",
+                        message:
+                            "column user_profiles.jurisdiction does not exist",
+                    },
+                },
+                { data: preMigrationRow, error: null },
+            ];
+
+            const res = await request(app)
+                .get("/user/profile")
+                .set(...AUTH);
+
+            expect(res.status).toBe(200);
+            expect(res.body).toMatchObject({
+                legalResearchUs: false,
+                quickActionsVisible: false,
+                onboardingComplete: true,
+                onboardingVersion: 0,
+                passwordSet: false,
+                jurisdiction: null,
+                practiceAreas: [],
+            });
+        });
+
+        it("keeps live onboarding columns when only migration 02 is missing", async () => {
+            // password_set_at (20260821_02) missing must NOT drop the
+            // migration-01 columns that DO exist — otherwise a new user on
+            // such a database would report onboardingComplete: true and
+            // skip onboarding entirely.
+            const migration01Row = profileRow({ onboarding_version: null });
+            delete (migration01Row as Record<string, unknown>).password_set_at;
+            supabaseState.tables.user_profiles = [
+                {
+                    data: null,
+                    error: {
+                        code: "42703",
+                        message:
+                            "column user_profiles.password_set_at does not exist",
+                    },
+                },
+                { data: migration01Row, error: null },
+            ];
+
+            const res = await request(app)
+                .get("/user/profile")
+                .set(...AUTH);
+
+            expect(res.status).toBe(200);
+            expect(res.body).toMatchObject({
+                jurisdiction: "Singapore",
+                practiceAreas: ["Corporate and M&A"],
+                onboardingComplete: false,
+                onboardingVersion: null,
+                passwordSet: false,
+            });
+        });
+
         it("returns 500 with detail when the profile load errors", async () => {
             supabaseState.tables.user_profiles = {
                 data: null,
@@ -519,6 +615,35 @@ describe("user.routes", () => {
 
             expect(res.status).toBe(200);
         });
+
+        // display_name and organisation are injected into every chat's
+        // system prompt, so their size must be bounded like the other
+        // personalisation fields. Truncation (not rejection) mirrors
+        // handle_new_user's left(..., 200) and keeps any over-long value
+        // written before the cap editable rather than stuck.
+        it.each([
+            ["displayName", "display_name"],
+            ["organisation", "organisation"],
+        ] as const)(
+            "truncates %s to 200 characters",
+            async (field, column) => {
+                supabaseState.tables.user_profiles = {
+                    data: profileRow(),
+                    error: null,
+                };
+
+                const res = await request(app)
+                    .patch("/user/profile")
+                    .set(...AUTH)
+                    .send({ [field]: "x".repeat(250) });
+
+                expect(res.status).toBe(200);
+                const written = supabaseState.updates.user_profiles?.at(-1) as
+                    | Record<string, unknown>
+                    | undefined;
+                expect(written?.[column]).toBe("x".repeat(200));
+            },
+        );
     });
 
     describe("POST /user/onboarding", () => {

backend/src/lib/__tests__/userSettings.test.ts

@@ -122,3 +122,77 @@ describe("getUserModelSettings router-model allowlist", () => {
         expect(settings.tabular_model).toBe("claude-sonnet-5");
     });
 });
+
+// A database without the 20260821 onboarding migration rejects the widened
+// select outright (42703). The retry with the pre-migration column set must
+// preserve the user's saved models and legal-research choice — silently
+// resetting them was the severe half of the un-migrated-DB bug.
+function retryingProfileDb(
+    first: { data: unknown; error: unknown },
+    second: { data: unknown; error: unknown },
+) {
+    const results = [first, second];
+    const chain: Record<string, unknown> = {};
+    for (const method of ["from", "select", "eq"]) {
+        chain[method] = vi.fn(() => chain);
+    }
+    chain.single = vi.fn(async () => results.shift() ?? second);
+    return chain as never;
+}
+
+describe("getUserModelSettings on an un-migrated database", () => {
+    it("retries without the onboarding columns and keeps saved settings", async () => {
+        const settings = await getUserModelSettings(
+            "user-1",
+            retryingProfileDb(
+                {
+                    data: null,
+                    error: {
+                        code: "42703",
+                        message:
+                            "column user_profiles.jurisdiction does not exist",
+                    },
+                },
+                {
+                    data: {
+                        title_model: "claude-haiku-4-5",
+                        tabular_model: "claude-sonnet-5",
+                        legal_research_us: false,
+                    },
+                    error: null,
+                },
+            ),
+        );
+
+        expect(settings.title_model).toBe("claude-haiku-4-5");
+        expect(settings.tabular_model).toBe("claude-sonnet-5");
+        expect(settings.legal_research_us).toBe(false);
+        expect(settings.personalisation).toMatchObject({
+            displayName: null,
+            practiceAreas: [],
+        });
+    });
+
+    it("falls back to defaults when the retry also fails", async () => {
+        const settings = await getUserModelSettings(
+            "user-1",
+            retryingProfileDb(
+                {
+                    data: null,
+                    error: {
+                        code: "42703",
+                        message:
+                            "column user_profiles.jurisdiction does not exist",
+                    },
+                },
+                {
+                    data: null,
+                    error: { code: "42703", message: "even older database" },
+                },
+            ),
+        );
+
+        expect(settings.legal_research_us).toBe(true);
+        expect(settings.title_model).toBeTruthy();
+    });
+});

backend/src/lib/userSettings.ts

@@ -65,7 +65,24 @@ export async function getUserModelSettings(
         getStoredUserApiKeys(userId, client),
         getAllUserRouterModels(userId, client),
     ]);
-    const data = profileResult.data;
+    let data = profileResult.data;
+
+    // A database that predates the 20260821 onboarding migration rejects the
+    // select above outright (unknown column), which would silently fall every
+    // caller back to default models and re-enable US legal research for users
+    // who turned it off. Retry with the pre-migration column set so saved
+    // settings keep working; personalisation simply stays empty.
+    if (profileResult.error?.code === "42703") {
+        const legacy = await client
+            .from("user_profiles")
+            .select("title_model, tabular_model, legal_research_us")
+            .eq("user_id", userId)
+            .single();
+        // A second failure (a database even older than the pre-migration
+        // shape) keeps data null and falls through to the defaults below —
+        // the pre-retry behavior, now explicit instead of accidental.
+        data = legacy.error ? null : (legacy.data as typeof data);
+    }
 
     // A stored preference can name a router model the user has since removed
     // from (or never had in) their saved selection — e.g. a hand-crafted

backend/src/routes/user.ts

@@ -181,6 +181,21 @@ function mcpOAuthPopupCsp(nonce: string) {
 
 const PROFILE_SELECT =
     "display_name, organisation, jurisdiction, practice_setting, professional_title, practice_areas, onboarding_version, password_set_at, message_credits_used, credits_reset_date, tier, title_model, tabular_model, mfa_on_login, legal_research_us, quick_actions_visible";
+// PROFILE_SELECT minus the 20260821 onboarding / password-capability columns,
+// for databases that have not applied those migrations yet. Migration 02
+// (password_set_at) gets its own tier so a database that applied 01 but not
+// 02 keeps its live onboarding/personalisation columns.
+const PROFILE_SELECT_NO_PASSWORD =
+    "display_name, organisation, jurisdiction, practice_setting, professional_title, practice_areas, onboarding_version, message_credits_used, credits_reset_date, tier, title_model, tabular_model, mfa_on_login, legal_research_us, quick_actions_visible";
+const PROFILE_SELECT_NO_ONBOARDING =
+    "display_name, organisation, message_credits_used, credits_reset_date, tier, title_model, tabular_model, mfa_on_login, legal_research_us, quick_actions_visible";
+const ONBOARDING_PROFILE_COLUMNS = [
+    "jurisdiction",
+    "practice_setting",
+    "professional_title",
+    "practice_areas",
+    "onboarding_version",
+];
 const PROFILE_SELECT_NO_QUICK_ACTIONS =
     "display_name, organisation, message_credits_used, credits_reset_date, tier, title_model, tabular_model, mfa_on_login, legal_research_us";
 const PROFILE_SELECT_NO_LEGAL =
@@ -217,8 +232,47 @@ async function selectProfile(
             ? await fullQuery.single()
             : await fullQuery.maybeSingle();
     if (!full.error) return full;
+    let cascadeError: unknown = full.error;
+
+    // A database that predates the 20260821 migrations rejects the full
+    // select on the first of the new columns, which would otherwise skip
+    // every tier below (they key on *their* new column's name) and land on
+    // a select that silently resets the legal-research and quick-action
+    // preferences to defaults. Two retry tiers, most-migrated first:
+    // missing only password_set_at (migration 02) keeps the live
+    // onboarding columns; missing the migration-01 columns drops them all,
+    // and serializeProfile treats the absent fields as legacy-exempt —
+    // matching what the migration's backfill would write.
+    if (isMissingProfileColumn(cascadeError, "password_set_at")) {
+        const prePasswordQuery = db
+            .from("user_profiles")
+            .select(PROFILE_SELECT_NO_PASSWORD)
+            .eq("user_id", userId);
+        const prePassword =
+            mode === "single"
+                ? await prePasswordQuery.single()
+                : await prePasswordQuery.maybeSingle();
+        if (!prePassword.error) return prePassword;
+        cascadeError = prePassword.error;
+    }
+    if (
+        ONBOARDING_PROFILE_COLUMNS.some((column) =>
+            isMissingProfileColumn(cascadeError, column),
+        )
+    ) {
+        const preOnboardingQuery = db
+            .from("user_profiles")
+            .select(PROFILE_SELECT_NO_ONBOARDING)
+            .eq("user_id", userId);
+        const preOnboarding =
+            mode === "single"
+                ? await preOnboardingQuery.single()
+                : await preOnboardingQuery.maybeSingle();
+        if (!preOnboarding.error) return preOnboarding;
+        cascadeError = preOnboarding.error;
+    }
 
-    if (isMissingProfileColumn(full.error, "quick_actions_visible")) {
+    if (isMissingProfileColumn(cascadeError, "quick_actions_visible")) {
         const previousQuery = db
             .from("user_profiles")
             .select(PROFILE_SELECT_NO_QUICK_ACTIONS)
@@ -639,14 +693,21 @@ function validateProfilePayload(body: unknown):
     if (!personalisation.ok) return personalisation;
     Object.assign(update, personalisation.update);
 
+    // Both fields flow into every chat's system prompt via
+    // buildUserPersonalisationPrompt, so an unbounded value would inflate
+    // token cost on every message. Truncate (not reject) at 200 characters:
+    // that is exactly what the signup trigger (handle_new_user's
+    // left(..., 200)) does to the same columns, and rejection would strand
+    // any over-long value written before this cap existed.
     if ("displayName" in raw) {
         if (raw.displayName !== null && typeof raw.displayName !== "string") {
             return {
                 ok: false,
                 detail: "displayName must be a string or null",
             };
         }
-        update.display_name = raw.displayName?.trim() || null;
+        update.display_name =
+            raw.displayName?.trim().slice(0, 200) || null;
     }
 
     if ("organisation" in raw) {
@@ -656,7 +717,8 @@ function validateProfilePayload(body: unknown):
                 detail: "organisation must be a string or null",
             };
         }
-        update.organisation = raw.organisation?.trim() || null;
+        update.organisation =
+            raw.organisation?.trim().slice(0, 200) || null;
     }
 
     if ("tabularModel" in raw) {