fix(sentry): null-coalesce PostgREST details/hint to prevent TypeError (#945)

PostgREST 500 responses return `details: null` and `hint: null`.
The `isPostgrestError` duck-type check uses the `in` operator which
returns true when the key exists regardless of value type. Calling
`null.startsWith()` or `null.includes()` in `isTransientNetworkError`
and `isSupabaseAuthLockError` crashes the retry logic, turning a
retryable Supabase 500 into an unhandled rejection.

- Update `isPostgrestError` type guard to reflect nullable fields
- Add `?? ""` null coalescing in isTransientNetworkError,
  isSupabaseAuthLockError, and captureSupabaseError
- Add regression tests for null details/hint
- Document null-safety convention in .agents/conventions.md

Closes #944

Co-authored-by: Ona <no-reply@ona.com>

Author: sw-factory-automations

.agents/conventions.md

@@ -254,6 +254,19 @@ proper `Error` before sending to Sentry so they get proper stack traces and
 grouping. The `isPostgrestError` duck-type check in `src/lib/sentry.ts` also
 handles both shapes.
 
+**Null-safety for PostgREST fields:** PostgREST error responses may return
+`details: null` and `hint: null` (e.g. PGRST500 server errors). The `in` operator
+in `isPostgrestError` returns `true` when the key exists regardless of value type.
+Always null-coalesce when reading these fields:
+
+```typescript
+// ✅ Correct — handles null from PostgREST 500 responses
+const details = isPostgrestError(error) ? (error.details ?? "") : "";
+
+// ❌ Wrong — crashes with TypeError: null.startsWith() / null.includes()
+const details = isPostgrestError(error) ? error.details : "";
+```
+
 ### Always use `captureApiError` for internal API fetch errors
 
 Non-Supabase fetch calls (e.g. `/api/pages/…/versions`) must use `captureApiError`

src/lib/sentry.ts

@@ -56,7 +56,7 @@ export function isE2ETestRequest(event: ErrorEvent): boolean {
  */
 function isPostgrestError(
   error: unknown,
-): error is { message: string; code: string; details: string; hint: string } {
+): error is { message: string; code: string; details: string | null; hint: string | null } {
   if (error == null || typeof error !== "object") return false;
   return (
     "message" in error &&
@@ -317,7 +317,8 @@ const NODE_FETCH_CAUSE_PATTERNS = [
  */
 export function isSupabaseAuthLockError(error: Error): boolean {
   const msg = error.message;
-  const details = isPostgrestError(error) ? error.details : "";
+  // PostgREST 500 responses may have `details: null` — coalesce to ""
+  const details = isPostgrestError(error) ? (error.details ?? "") : "";
 
   return (
     msg.includes("Lock broken by another request") ||
@@ -429,7 +430,8 @@ export function isSupabaseAuthLockContention(event: ErrorEvent): boolean {
  */
 export function isTransientNetworkError(error: Error): boolean {
   const msg = error.message;
-  const details = isPostgrestError(error) ? error.details : "";
+  // PostgREST 500 responses may have `details: null` — coalesce to ""
+  const details = isPostgrestError(error) ? (error.details ?? "") : "";
 
   // Browser-style fetch errors.
   // The Supabase client may append the hostname in parentheses, e.g.
@@ -548,8 +550,8 @@ export function captureSupabaseError(
 
   if (isPostgrestError(error)) {
     extra.code = error.code;
-    extra.details = error.details;
-    extra.hint = error.hint;
+    extra.details = error.details ?? "";
+    extra.hint = error.hint ?? "";
   }
 
   // Wrap plain objects in a proper Error for Sentry grouping

src/lib/sentry.unit.test.ts

@@ -243,6 +243,21 @@ describe("isTransientNetworkError", () => {
     });
     expect(isTransientNetworkError(error as unknown as Error)).toBe(true);
   });
+
+  // --- Null details regression test (MEMO-2E / #944) ---
+  // PostgREST 500 responses return `details: null`. The `in` operator in
+  // isPostgrestError passes (key exists), but `null.startsWith()` crashes.
+
+  it("does not crash when PostgREST error has details: null (#944 MEMO-2E)", () => {
+    const error = { message: "server error", code: "XX000", details: null, hint: null };
+    expect(() => isTransientNetworkError(error as unknown as Error)).not.toThrow();
+    expect(isTransientNetworkError(error as unknown as Error)).toBe(false);
+  });
+
+  it("still detects transient fetch error when details is null but message matches (#944 MEMO-2E)", () => {
+    const error = { message: "TypeError: fetch failed", code: "XX000", details: null, hint: null };
+    expect(isTransientNetworkError(error as unknown as Error)).toBe(true);
+  });
 });
 
 describe("isSchemaNotFoundError", () => {
@@ -605,6 +620,14 @@ describe("isSupabaseAuthLockError", () => {
     });
     expect(isSupabaseAuthLockError(error)).toBe(false);
   });
+
+  // --- Null details regression test (MEMO-2E / #944) ---
+
+  it("does not crash when PostgREST error has details: null (#944 MEMO-2E)", () => {
+    const error = { message: "server error", code: "XX000", details: null, hint: null };
+    expect(() => isSupabaseAuthLockError(error as unknown as Error)).not.toThrow();
+    expect(isSupabaseAuthLockError(error as unknown as Error)).toBe(false);
+  });
 });
 
 describe("captureSupabaseError", () => {
@@ -894,6 +917,20 @@ describe("captureSupabaseError", () => {
     expect(opts.level).toBeUndefined();
     expect(opts.extra.code).toBe("PGRST500");
   });
+
+  // --- Null details/hint regression test (MEMO-2E / #944) ---
+
+  it("does not crash when PostgREST error has details: null and hint: null (#944 MEMO-2E)", async () => {
+    const error = { message: "server error", code: "XX000", details: null, hint: null };
+    expect(() => captureSupabaseError(error as unknown as Error, "page-tree:fetch-pages")).not.toThrow();
+    await flush();
+
+    expect(captureExceptionMock).toHaveBeenCalledOnce();
+    const [, opts] = captureExceptionMock.mock.calls[0];
+    expect(opts.extra.details).toBe("");
+    expect(opts.extra.hint).toBe("");
+    expect(opts.extra.code).toBe("XX000");
+  });
 });
 
 function makeSentryEvent(