Audit follow-up: correctness fixes, usability upgrades, and cleanup

.env.example

@@ -22,7 +22,6 @@ NEXTAUTH_SECRET=""                       # auto-filled by `npm run setup` (or: o
 
 # --- Admin & Security -----------------------------------------
 ADMIN_USER_IDS=""                        # comma-separated user UUIDs allowed to access /admin
-CRON_SECRET=""                           # auto-filled by `npm run setup` — shared secret for cron/internal endpoints
 
 # WARNING: ALLOW_TEST_AUTH makes the app trust the X-User-Id header
 # as the authenticated identity. It exists ONLY for automated tests.
@@ -64,6 +63,7 @@ AI_CACHE_TTL_SECONDS="3600"
 AI_DAILY_COST_CAP_USD="1.0"              # max spend per user per day (default $1)
 AI_MONTHLY_COST_CAP_USD="10.0"           # max spend per user per month (default $10)
 AI_DISABLED="false"                      # emergency kill switch — set "true" to block all AI calls
+# MOCK_AI_MALFORMED="1"                  # test-only: makes the mock provider emit malformed output (never set outside tests)
 
 # --- Background Worker ----------------------------------------
 JOB_WORKER_CONCURRENCY="2"

README.md

@@ -37,7 +37,7 @@ Then open http://localhost:3000. The defaults need no API keys or external accou
 
 To be blunt about the default: mock mode exists for tests. With `AI_PROVIDER` set to `mock`, sessions get template questions and canned feedback, which exercises the machinery but teaches you nothing. To actually study, set `AI_PROVIDER` to `openai` and add an `OPENAI_API_KEY` in `.env`.
 
-What `npm run setup` does: copies `.env.example` to `.env`, generates `NEXTAUTH_SECRET`, `TOKEN_ENC_KEY`, and `CRON_SECRET`, starts PostgreSQL through Docker, and applies all migrations. It is idempotent and never overwrites a value you set yourself.
+What `npm run setup` does: copies `.env.example` to `.env`, generates `NEXTAUTH_SECRET` and `TOKEN_ENC_KEY`, starts PostgreSQL through Docker, and applies all migrations. It is idempotent and never overwrites a value you set yourself.
 
 Notes:
 

docker-compose.yml

@@ -26,7 +26,6 @@ services:
       BASE_URL: ${BASE_URL:-http://localhost:3000}
       NEXTAUTH_URL: ${NEXTAUTH_URL:-http://localhost:3000}
       NEXTAUTH_SECRET: ${NEXTAUTH_SECRET:?Set NEXTAUTH_SECRET in .env or environment}
-      CRON_SECRET: ${CRON_SECRET:-}
       ADMIN_USER_IDS: ${ADMIN_USER_IDS:-}
       GOOGLE_PROVIDER: ${GOOGLE_PROVIDER:-fake}
       AI_PROVIDER: ${AI_PROVIDER:-mock}

next.config.js

@@ -27,13 +27,6 @@ const nextConfig = {
         },
       ],
     },
-    {
-      source: "/sw.js",
-      headers: [
-        { key: "Service-Worker-Allowed", value: "/" },
-        { key: "Cache-Control", value: "no-cache" },
-      ],
-    },
   ],
 }
 

package.json

@@ -46,7 +46,6 @@
     "@types/nodemailer": "^8.0.0",
     "@types/pg": "^8.20.0",
     "@types/react": "^19.2.14",
-    "@types/web-push": "^3.6.4",
     "@vitejs/plugin-react": "^6.0.1",
     "dotenv": "^17.4.2",
     "eslint": "^8.57.1",

prisma/migrations/20260716120000_drop_notification_prefs_and_dead_columns/migration.sql

@@ -0,0 +1,6 @@
+-- Drop the unused notification preferences table (feature was never shipped)
+DROP TABLE "notification_preferences";
+
+-- Drop dead columns from user_game_state
+ALTER TABLE "user_game_state" DROP COLUMN "display_name";
+ALTER TABLE "user_game_state" DROP COLUMN "leaderboard_visible";

prisma/schema.prisma

@@ -589,15 +589,13 @@ model Achievement {
 model UserGameState {
   id                   String   @id @default(uuid())
   userId               String   @unique @map("user_id")
-  displayName          String?  @map("display_name")
   dailyXpGoal          Int      @default(50) @map("daily_xp_goal")
   streakFreezes        Int      @default(0) @map("streak_freezes")
   streakFreezeUsedDate String?  @map("streak_freeze_used_date") // YYYY-MM-DD
   studyStartTime       String   @default("09:00") @map("study_start_time")
   studyEndTime         String   @default("17:00") @map("study_end_time")
   dailyStudyCap        Int      @default(180) @map("daily_study_cap")
   onboardingComplete   Boolean  @default(false) @map("onboarding_complete")
-  leaderboardVisible   Boolean  @default(true) @map("leaderboard_visible")
   timezone             String? // IANA timezone for streak day boundaries; null = UTC
   createdAt            DateTime @default(now()) @map("created_at")
   updatedAt            DateTime @updatedAt @map("updated_at")
@@ -617,9 +615,8 @@ model User {
   createdAt     DateTime  @default(now()) @map("created_at")
   updatedAt     DateTime  @updatedAt @map("updated_at")
 
-  notificationPreference NotificationPreference?
-  chatMessages           ChatMessage[]
-  verificationTokens     VerificationToken[]
+  chatMessages       ChatMessage[]
+  verificationTokens VerificationToken[]
 
   @@map("users")
 }
@@ -641,25 +638,6 @@ model ChatMessage {
   @@map("chat_messages")
 }
 
-// ── Notification Preferences ────────────────────────────────────
-
-model NotificationPreference {
-  id              String   @id @default(uuid())
-  userId          String   @unique @map("user_id")
-  emailReminders  Boolean  @default(true) @map("email_reminders")
-  pushReminders   Boolean  @default(true) @map("push_reminders")
-  reminderTime    String   @default("09:00") @map("reminder_time") // HH:MM
-  streakReminders Boolean  @default(true) @map("streak_reminders")
-  weeklyDigest    Boolean  @default(true) @map("weekly_digest")
-  emailAddress    String?  @map("email_address") // override if different from User.email
-  createdAt       DateTime @default(now()) @map("created_at")
-  updatedAt       DateTime @updatedAt @map("updated_at")
-
-  user User @relation(fields: [userId], references: [id], onDelete: Cascade)
-
-  @@map("notification_preferences")
-}
-
 // ── Verification Tokens (email verify, password reset) ──────────
 
 model VerificationToken {

scripts/setup.mjs

@@ -6,7 +6,7 @@
  *
  * What it does:
  *   1. Copies .env.example -> .env (never overwrites an existing .env).
- *   2. Generates NEXTAUTH_SECRET, TOKEN_ENC_KEY and CRON_SECRET where they
+ *   2. Generates NEXTAUTH_SECRET and TOKEN_ENC_KEY where they
  *      are empty (user-set values are never touched).
  *   3. Starts the Postgres container via Docker Compose (if Docker is
  *      available) and waits for its healthcheck.
@@ -26,7 +26,7 @@ const envPath = join(root, ".env");
 const examplePath = join(root, ".env.example");
 
 // ---------------------------------------------------------------------------
-// 0. Node version check (Next.js 15 + this script need Node 20.19+)
+// 0. Node version check (Next.js 14 + this script need Node 20.19+)
 // ---------------------------------------------------------------------------
 const [nodeMajor, nodeMinor] = process.versions.node.split(".").map(Number);
 if (nodeMajor < 20 || (nodeMajor === 20 && nodeMinor < 19)) {
@@ -81,7 +81,6 @@ if (existsSync(envPath)) {
 const SECRET_GENERATORS = {
   NEXTAUTH_SECRET: () => randomBytes(32).toString("base64"),
   TOKEN_ENC_KEY: () => randomBytes(32).toString("hex"),
-  CRON_SECRET: () => randomBytes(16).toString("hex"),
 };
 
 let envText = readFileSync(envPath, "utf8");

src/__tests__/content-prompts-validation.test.ts

@@ -206,6 +206,32 @@ describe("generateContentAwarePrompts output validation", () => {
     expect(mcq.meta?.key_points).toEqual(["point one", "point two"]);
   });
 
+  it("sanitizes difficulty: rounds floats, clamps to 1..5, and defaults non-finite/non-number values to 1", async () => {
+    mockRunTaskRaw({
+      prompts: [
+        { ...validPrompt(1), difficulty: 3.6 }, // float → rounded
+        { ...validPrompt(2), difficulty: NaN }, // NaN → 1
+        { ...validPrompt(3), difficulty: Infinity }, // non-finite → 1
+        { ...validPrompt(4), difficulty: "4" }, // string → 1 (not coerced)
+        { ...validPrompt(5), difficulty: 99 }, // above range → 5
+        { ...validPrompt(6), difficulty: -2 }, // below range → 1
+        { ...validPrompt(7), difficulty: undefined }, // missing → 1
+        { ...validPrompt(8), difficulty: 0.4 }, // rounds to 0 → clamped to 1
+      ],
+    });
+
+    const result = await generateContentAwarePrompts(DEFAULT_PARAMS);
+
+    expect(result).not.toBeNull();
+    expect(result!.map((p) => p.difficulty)).toEqual([4, 1, 1, 1, 5, 1, 1, 1]);
+    // Every persisted difficulty is a valid Int in range for the Prisma column
+    for (const p of result!) {
+      expect(Number.isInteger(p.difficulty)).toBe(true);
+      expect(p.difficulty).toBeGreaterThanOrEqual(1);
+      expect(p.difficulty).toBeLessThanOrEqual(5);
+    }
+  });
+
   it("returns null when runTask throws", async () => {
     runTaskMock.mockRejectedValue(new Error("provider down"));
 

src/__tests__/feedback-persistence.test.ts

@@ -227,14 +227,25 @@ describe("feedback persistence and eager generation", () => {
       expect(runTaskMock).not.toHaveBeenCalled();
     });
 
-    it("persists the empty-results response when search finds nothing", async () => {
+    it("persists an explicit no-sources terminal state when search finds nothing", async () => {
       searchChunksMock.mockResolvedValue([]);
 
       const result = await generateFeedback("user-1", "attempt-1");
 
-      expect(result).toEqual({ status: "OK", excerpts: [] });
+      // no_sources marks the empty result as terminal — clients stop polling
+      // and render an honest fallback instead of an eternal spinner.
+      expect(result).toEqual({ status: "OK", excerpts: [], no_sources: true });
       expect(store.row?.feedbackStatus).toBe("READY");
       expect(store.row?.feedbackJson).toEqual(result);
+
+      // Refetch returns the persisted terminal state verbatim — no
+      // regeneration, no renewed searching.
+      searchChunksMock.mockClear();
+      runTaskMock.mockClear();
+      const second = await generateFeedback("user-1", "attempt-1");
+      expect(second).toEqual(result);
+      expect(searchChunksMock).not.toHaveBeenCalled();
+      expect(runTaskMock).not.toHaveBeenCalled();
     });
 
     it("does not persist or claim for unscored (EXAM-phase) attempts", async () => {

src/__tests__/feedback-polling.test.ts

@@ -0,0 +1,106 @@
+/**
+ * Unit tests for the client-side feedback poll loop (pollFeedback).
+ *
+ * The live-audit bug: an empty/failed feedback generation left the review
+ * panel spinning forever. The contract under test: PENDING is the ONLY
+ * non-terminal status — any other result (OK with content, OK + no_sources,
+ * UNAVAILABLE) resolves the poll immediately, the poll budget yields null
+ * instead of spinning, and cancellation stops the loop without a result.
+ */
+import { describe, it, expect, vi, beforeEach } from "vitest";
+import { pollFeedback } from "@/app/s/[sessionId]/feedback-poll";
+
+const fetchMock = vi.fn();
+vi.stubGlobal("fetch", fetchMock);
+
+function jsonResponse(body: unknown, ok = true) {
+  return { ok, json: async () => body };
+}
+
+const notCancelled = () => false;
+
+describe("pollFeedback terminal semantics", () => {
+  beforeEach(() => {
+    fetchMock.mockReset();
+  });
+
+  it("resolves on the first non-PENDING result (single fetch)", async () => {
+    const payload = { status: "OK", excerpts: [], explanation: "because osmosis" };
+    fetchMock.mockResolvedValue(jsonResponse(payload));
+
+    const result = await pollFeedback("attempt-1", notCancelled, 5, 1);
+
+    expect(result).toEqual(payload);
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
+  it("treats the explicit no-sources state as terminal — no further polling", async () => {
+    const payload = { status: "OK", excerpts: [], no_sources: true };
+    fetchMock.mockResolvedValue(jsonResponse(payload));
+
+    const result = await pollFeedback("attempt-1", notCancelled, 5, 1);
+
+    expect(result).toEqual(payload);
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
+  it("treats UNAVAILABLE (generation failure) as terminal", async () => {
+    const payload = { status: "UNAVAILABLE", excerpts: [] };
+    fetchMock.mockResolvedValue(jsonResponse(payload));
+
+    const result = await pollFeedback("attempt-1", notCancelled, 5, 1);
+
+    expect(result).toEqual(payload);
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
+  it("keeps polling through PENDING, then returns the terminal result", async () => {
+    const terminal = { status: "OK", excerpts: [], no_sources: true };
+    fetchMock
+      .mockResolvedValueOnce(jsonResponse({ status: "PENDING", excerpts: [] }))
+      .mockResolvedValueOnce(jsonResponse({ status: "PENDING", excerpts: [] }))
+      .mockResolvedValueOnce(jsonResponse(terminal));
+
+    const result = await pollFeedback("attempt-1", notCancelled, 5, 1);
+
+    expect(result).toEqual(terminal);
+    expect(fetchMock).toHaveBeenCalledTimes(3);
+  });
+
+  it("returns null (not an infinite spin) when the poll budget is exhausted", async () => {
+    fetchMock.mockResolvedValue(jsonResponse({ status: "PENDING", excerpts: [] }));
+
+    const result = await pollFeedback("attempt-1", notCancelled, 3, 1);
+
+    expect(result).toBeNull();
+    expect(fetchMock).toHaveBeenCalledTimes(3);
+  });
+
+  it("returns null without fetching when already cancelled", async () => {
+    const result = await pollFeedback("attempt-1", () => true, 5, 1);
+
+    expect(result).toBeNull();
+    expect(fetchMock).not.toHaveBeenCalled();
+  });
+
+  it("stops mid-loop when cancellation flips during a PENDING stretch", async () => {
+    let cancelled = false;
+    fetchMock.mockImplementation(async () => {
+      cancelled = true; // cancel after the first response comes back
+      return jsonResponse({ status: "PENDING", excerpts: [] });
+    });
+
+    const result = await pollFeedback("attempt-1", () => cancelled, 5, 1);
+
+    expect(result).toBeNull();
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
+  it("propagates a fetch error so the caller can settle into a fallback", async () => {
+    fetchMock.mockResolvedValue(jsonResponse({ error: "Internal server error" }, false));
+
+    await expect(pollFeedback("attempt-1", notCancelled, 5, 1)).rejects.toThrow(
+      "Internal server error",
+    );
+  });
+});