Merge pull request #297 from ArnasDon/fix/account-context-schema-cache

fix(auth): load account context without an embedded FK join (#294)

src/hooks/use-auth.tsx

@@ -131,12 +131,7 @@ export function AuthProvider({ children }: { children: ReactNode }) {
       const { data, error } = await supabase
         .from("profiles")
         .select(
-          // `account:accounts!inner(id, name)` — explicit join on the
-          // single FK profiles.account_id → accounts.id. `!inner` so a
-          // missing account collapses to null rather than a half-
-          // populated row (shouldn't happen post-017 NOT NULL, but
-          // belt-and-braces against forks running older schemas).
-          "id, full_name, email, avatar_url, role, beta_features, account_id, account_role, account:accounts!inner(id, name, default_currency)",
+          "id, full_name, email, avatar_url, role, beta_features, account_id, account_role",
         )
         .eq("user_id", userId)
         .maybeSingle();
@@ -152,27 +147,40 @@ export function AuthProvider({ children }: { children: ReactNode }) {
       }
 
       if (data) {
-        // Supabase's typed client surfaces an embedded `!inner` row
-        // as either an object or a single-element array depending on
-        // the schema's inferred cardinality — normalise to the object
-        // form before reading.
-        const accountRaw = Array.isArray(data.account)
-          ? data.account[0] ?? null
-          : (data.account as {
-              id: string;
-              name: string;
-              default_currency: string | null;
-            } | null);
-        // Narrow default_currency defensively: forks running pre-021
-        // schemas won't have the column, so a missing/null value reads
-        // as the safe USD fallback rather than crashing the picker.
-        const accountRow: AccountSummary | null = accountRaw
-          ? {
-              id: accountRaw.id,
-              name: accountRaw.name,
-              default_currency: accountRaw.default_currency ?? DEFAULT_CURRENCY,
+        // Load the account with a plain lookup by id instead of an
+        // embedded FK join. The embed (`account:accounts!inner(...)`)
+        // forces PostgREST to resolve the profiles.account_id →
+        // accounts.id relationship from its schema cache; a stale cache
+        // (common right after a migration adds the FK) makes it fail
+        // hard with PGRST200 and blanks the whole profile — the user
+        // then loses account context everywhere (issue #294). A point
+        // lookup by id needs no relationship inference, so the profile
+        // (with account_id / account_role) still resolves even if the
+        // account name lookup itself can't.
+        let accountRow: AccountSummary | null = null;
+        if (data.account_id) {
+          const { data: account, error: accountErr } = await supabase
+            .from("accounts")
+            // default_currency added in migration 021; narrowed to the
+            // USD fallback below for older schemas where it reads null.
+            .select("id, name, default_currency")
+            .eq("id", data.account_id)
+            .maybeSingle();
+          if (accountErr) {
+            console.error("[AuthProvider] fetchAccount error:", {
+              message: accountErr.message,
+              details: accountErr.details,
+              hint: accountErr.hint,
+              code: accountErr.code,
+            });
+          } else if (account) {
+            accountRow = {
+              id: account.id,
+              name: account.name,
+              default_currency: account.default_currency ?? DEFAULT_CURRENCY,
+            };
+          }
         }
-          : null;
 
         // Narrow the DB enum into our AccountRole union. The DB
         // constraint should make this unconditional, but a future

src/lib/auth/account.test.ts

@@ -0,0 +1,176 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+
+// getCurrentAccount resolves the caller's account context. The
+// regression this file guards (issue #294): account loading must NOT
+// depend on a PostgREST embedded FK join (`accounts!inner`), because a
+// stale schema cache makes that embed fail hard and blanks the whole
+// context. It must instead read the profile and then the account with
+// two plain point queries.
+
+// ------------------------------------------------------------
+// Chainable Supabase query-builder mock. Each `.from(table)` hands back
+// a thenable builder pre-loaded with the result queued for that table,
+// so we can assert which tables were queried and with what filters.
+// ------------------------------------------------------------
+interface BuilderCall {
+  table: string;
+  columns?: string;
+  eqArgs: [string, unknown][];
+}
+
+function makeClient(opts: {
+  user: { id: string } | null;
+  userErr?: unknown;
+  byTable: Record<string, { data: unknown; error: unknown }>;
+}) {
+  const calls: BuilderCall[] = [];
+
+  const from = (table: string) => {
+    const call: BuilderCall = { table, eqArgs: [] };
+    calls.push(call);
+    const builder = {
+      select(columns: string) {
+        call.columns = columns;
+        return builder;
+      },
+      eq(col: string, val: unknown) {
+        call.eqArgs.push([col, val]);
+        return builder;
+      },
+      maybeSingle() {
+        return Promise.resolve(
+          opts.byTable[table] ?? { data: null, error: null },
+        );
+      },
+    };
+    return builder;
+  };
+
+  return {
+    calls,
+    client: {
+      auth: {
+        getUser: () =>
+          Promise.resolve({
+            data: { user: opts.user },
+            error: opts.userErr ?? null,
+          }),
+      },
+      from,
+    },
+  };
+}
+
+const createClient = vi.fn();
+vi.mock("@/lib/supabase/server", () => ({
+  createClient: () => createClient(),
+}));
+
+const { getCurrentAccount, UnauthorizedError, ForbiddenError } = await import(
+  "./account"
+);
+
+afterEach(() => {
+  vi.clearAllMocks();
+});
+
+describe("getCurrentAccount", () => {
+  it("resolves context via a plain accounts lookup, not an embedded join", async () => {
+    const { client, calls } = makeClient({
+      user: { id: "user-1" },
+      byTable: {
+        profiles: {
+          data: { account_id: "acct-1", account_role: "owner" },
+          error: null,
+        },
+        accounts: { data: { id: "acct-1", name: "Acme" }, error: null },
+      },
+    });
+    createClient.mockReturnValue(client);
+
+    const ctx = await getCurrentAccount();
+
+    expect(ctx).toMatchObject({
+      userId: "user-1",
+      accountId: "acct-1",
+      role: "owner",
+      account: { id: "acct-1", name: "Acme" },
+    });
+
+    // Two queries: profiles by user_id, then accounts by id. Neither
+    // selects an embedded relationship — the regression guard.
+    expect(calls.map((c) => c.table)).toEqual(["profiles", "accounts"]);
+    expect(calls[0].columns).not.toMatch(/accounts!/);
+    expect(calls[0].eqArgs).toEqual([["user_id", "user-1"]]);
+    expect(calls[1].columns).not.toMatch(/accounts!/);
+    expect(calls[1].eqArgs).toEqual([["id", "acct-1"]]);
+  });
+
+  it("throws UnauthorizedError when there is no session", async () => {
+    const { client } = makeClient({ user: null, byTable: {} });
+    createClient.mockReturnValue(client);
+    await expect(getCurrentAccount()).rejects.toBeInstanceOf(UnauthorizedError);
+  });
+
+  it("maps a profiles query error to 'Could not load account context'", async () => {
+    const { client } = makeClient({
+      user: { id: "user-1" },
+      byTable: {
+        profiles: { data: null, error: { code: "PGRST200" } },
+      },
+    });
+    createClient.mockReturnValue(client);
+    await expect(getCurrentAccount()).rejects.toThrow(
+      "Could not load account context",
+    );
+  });
+
+  it("maps an accounts query error to 'Could not load account context'", async () => {
+    // The exact #294 shape if the embed were still in play, but now on
+    // the decoupled accounts lookup: profile resolves, account read errors.
+    const { client } = makeClient({
+      user: { id: "user-1" },
+      byTable: {
+        profiles: {
+          data: { account_id: "acct-1", account_role: "admin" },
+          error: null,
+        },
+        accounts: { data: null, error: { code: "PGRST200" } },
+      },
+    });
+    createClient.mockReturnValue(client);
+    const err = await getCurrentAccount().catch((e) => e);
+    expect(err).toBeInstanceOf(ForbiddenError);
+    expect(err.message).toBe("Could not load account context");
+  });
+
+  it("rejects a profile not linked to an account", async () => {
+    const { client } = makeClient({
+      user: { id: "user-1" },
+      byTable: {
+        profiles: { data: { account_id: null, account_role: null }, error: null },
+      },
+    });
+    createClient.mockReturnValue(client);
+    await expect(getCurrentAccount()).rejects.toThrow(
+      "Profile is not linked to an account",
+    );
+  });
+
+  it("rejects an account_id that resolves to no readable account", async () => {
+    const { client } = makeClient({
+      user: { id: "user-1" },
+      byTable: {
+        profiles: {
+          data: { account_id: "acct-1", account_role: "viewer" },
+          error: null,
+        },
+        accounts: { data: null, error: null },
+      },
+    });
+    createClient.mockReturnValue(client);
+    await expect(getCurrentAccount()).rejects.toThrow(
+      "Profile is not linked to an account",
+    );
+  });
+});

src/lib/auth/account.ts

@@ -114,14 +114,9 @@ export async function getCurrentAccount(): Promise<AccountContext> {
     throw new UnauthorizedError();
   }
 
-  // Selecting through the FK gives us the account name in one
-  // query — `account:accounts!inner(id,name)` is Supabase's
-  // explicit-join syntax. `!inner` so a NULL account_id (which
-  // shouldn't exist) yields no row and trips the guard below
-  // rather than silently returning a half-populated profile.
   const { data, error } = await supabase
     .from("profiles")
-    .select("account_id, account_role, account:accounts!inner(id, name)")
+    .select("account_id, account_role")
     .eq("user_id", user.id)
     .maybeSingle();
 
@@ -129,7 +124,7 @@ export async function getCurrentAccount(): Promise<AccountContext> {
     console.error("[getCurrentAccount] profile fetch error:", error);
     throw new ForbiddenError("Could not load account context");
   }
-  if (!data || !data.account_id || !data.account_role || !data.account) {
+  if (!data || !data.account_id || !data.account_role) {
     // Pre-migration profile, or a manual insert that skipped the
     // signup trigger. The user is authenticated but the app has
     // no way to scope their queries — treat as forbidden.
@@ -142,16 +137,38 @@ export async function getCurrentAccount(): Promise<AccountContext> {
     throw new ForbiddenError(`Unknown account role: ${data.account_role}`);
   }
 
-  // Supabase's typed client returns related rows as an array even
-  // for `!inner` single-record joins; normalise to a single object.
-  const accountRow = Array.isArray(data.account) ? data.account[0] : data.account;
+  // Load the account with a plain point lookup by id rather than an
+  // embedded FK join (`account:accounts!inner(...)`). The embed forces
+  // PostgREST to resolve the profiles.account_id → accounts.id
+  // relationship from its schema cache; when that cache is stale — a
+  // common Supabase state right after a migration adds the FK, or when
+  // migrations are applied out of band — the embed fails hard with
+  // PGRST200 ("could not find a relationship … in the schema cache")
+  // and takes down the entire account context (issue #294). A lookup by
+  // id needs no relationship inference and is gated by the same accounts
+  // RLS, so it stays robust against cache staleness and older schemas.
+  const { data: account, error: accountErr } = await supabase
+    .from("accounts")
+    .select("id, name")
+    .eq("id", data.account_id)
+    .maybeSingle();
+
+  if (accountErr) {
+    console.error("[getCurrentAccount] account fetch error:", accountErr);
+    throw new ForbiddenError("Could not load account context");
+  }
+  if (!account) {
+    // account_id points at no readable account row — orphaned profile
+    // or an RLS gap. Same "can't scope this user" outcome as above.
+    throw new ForbiddenError("Profile is not linked to an account");
+  }
 
   return {
     supabase,
     userId: user.id,
     accountId: data.account_id,
     role: data.account_role,
-    account: { id: accountRow.id, name: accountRow.name },
+    account: { id: account.id, name: account.name },
   };
 }