Epic 9: Bug fixes & core workflow stability

docs/planning/retrospectives/epic-9-retro-2026-03-10.md

@@ -0,0 +1,241 @@
+# Epic 9 Retrospective: Bug Fixes & Core Workflow Stability
+
+**Date:** 2026-03-10
+**Facilitator:** Bob (Scrum Master)
+**Participants:** Simon (Project Lead), Alice (Product Owner), Charlie (Senior Dev), Dana (QA Engineer), Elena (Junior Dev)
+
+---
+
+## Epic Summary
+
+| Metric | Value |
+|--------|-------|
+| **Epic Name** | Bug Fixes & Core Workflow Stability |
+| **Stories Completed** | 4/4 (100%) |
+| **Story Points** | 14 (5 + 5 + 2 + 2) |
+| **Estimated Duration** | 7-10 days |
+| **Actual Duration** | ~4 days (Mar 6 - Mar 9) |
+| **Review Findings** | 0 HIGH, 0 MEDIUM, 5 LOW (all advisory) |
+| **Production Incidents** | 0 |
+| **Blockers Encountered** | 0 |
+| **CI Failures on Merge** | 4 (3 fix commits required) |
+| **Tests Added** | 15 new tests (141 → 156) |
+
+**Stories Delivered:**
+1. Story 9.1: Fix Application Status Transitions & Add Draft Status — Root cause: missing frontend UI (not backend bug). Added status dropdown to detail page, Draft migration, status select on create form.
+2. Story 9.2: Fix Interview Status Tracking & Dashboard Display — Fixed dashboard count (query interviews table directly), added interview status column, auto-status transition on interview creation, color-coded badges.
+3. Story 9.3: Fix Session Expiry Redirect & Re-authentication — Fixed all 3 signOut call sites to use `signOut({ redirect: false })` + manual navigation, added isSigningOut race condition guard, callbackUrl validation.
+4. Story 9.4: Application Form Error Display — Wired error display to 6 missing fields, added formError banner for non-validation errors, backend-to-frontend field name mapping.
+
+**Business Outcomes:**
+- Application status transitions work via UI dropdown
+- Dashboard shows accurate interview counts from interviews table
+- Creating an interview auto-transitions application status to "Interview"
+- Interview status badges (scheduled/completed/cancelled/awaiting outcome) display across all views
+- Application form shows inline errors on all fields and general error banner
+- Session expiry redirects to clean URL (partial — re-authentication still broken)
+
+---
+
+## What Went Well
+
+### 1. Execution Speed
+Delivered 14 story points in ~4 days against a 7-10 day estimate. Third consecutive epic finishing under estimate. Established patterns made execution fast and predictable.
+
+### 2. Clean Reviews
+3 of 4 stories approved on first review. Only Story 9.3 required one round of changes (3 LOW items, all resolved same day). Zero HIGH or MEDIUM findings across the entire epic.
+
+### 3. Root Cause Investigation Discipline
+Story 9.1's thorough investigation revealed the "status transition bug" was actually a missing frontend feature. This insight carried forward to Story 9.2, allowing the team to focus on the real fix rather than chasing phantom backend bugs.
+
+### 4. Test Commitment Maintained
+15 new tests added across all 4 stories (141 → 156). Tests-per-story commitment from Epic 7 retro fully maintained for the third consecutive epic.
+
+### 5. Deploy Before Retro
+Epic 9 deployed to production before retrospective (Epic 8 action item fulfilled).
+
+---
+
+## What Could Improve
+
+### 1. CI Broke 4 Times on Merge (Priority: HIGH)
+
+**Issue:** PR 54 required 3 fix commits after 4 CI failures:
+- TypeScript index signature error on `statusVariantMap` — `pnpm dev` didn't catch what `pnpm build` (strict mode in Docker) catches
+- Backend test DB schema missing `rate_limits` table and `status` column — CI test schema is manually maintained and wasn't updated with new migrations
+
+**Impact:** Wasted CI minutes, delayed merge, required additional commits to fix.
+
+**Root Cause:** Two gaps: (1) local dev (`pnpm dev`) is more lenient than production build (`pnpm build`), (2) CI test database uses a static schema that must be manually updated for each migration.
+
+**Resolution:**
+- Dev agent runs `pnpm build` locally before pushing
+- Automate CI test DB schema to run migrations instead of static schema
+
+### 2. Visual Quality Not Caught in Review (Priority: MEDIUM)
+
+**Issue:** Story 9.1 shipped a status dropdown with poor UI that Simon had to fix during Story 9.2. Code review checked correctness but not visual output.
+
+**Impact:** Rework required in subsequent story.
+
+**Root Cause:** Review process checks code patterns and logic but doesn't include rendering the UI.
+
+**Resolution:** Add visual review step — every UI story's review includes a screenshot or manual render check before approval.
+
+### 3. Session Expiry Re-authentication Still Broken (Priority: HIGH)
+
+**Issue:** Production smoke testing during this retrospective revealed that while Story 9.3 fixed the redirect URL format (no CSRF tokens), **re-authentication fails** when credentials are submitted from `/login?error=SessionExpired&callbackUrl=%2Fapplications`. Same credentials work fine on clean `/login`.
+
+**Impact:** Users who experience session expiry cannot log back in without manually navigating to `/login`.
+
+**Resolution:** Must fix before starting Epic 10. The `error` or `callbackUrl` URL params may be interfering with NextAuth's credential sign-in handler.
+
+### 4. Duplicate CI Runs (Priority: LOW)
+
+**Issue:** CI workflow triggers on both `push` and `pull_request` without branch filtering, causing duplicate runs on every feature branch push that has an open PR.
+
+**Impact:** Wasted GitHub Actions minutes.
+
+**Resolution:** Scope `push` trigger to `main` only in `.github/workflows/ci.yml`.
+
+---
+
+## Epic 8 Action Items Follow-Through
+
+| # | Action | Status | Evidence |
+|---|--------|--------|----------|
+| 1 | Draft → design → refine workflow | N/A | Bug fix epic, no feature design needed |
+| 2 | Deploy before retro | ✅ Done | Epic 9 deployed to production before retro |
+| 3 | Five-state UI checklist | N/A | Bug fix epic, no new UI screens |
+| 4 | Tests required per story | ✅ Done | 15 new tests across all 4 stories |
+| 5 | ARIA lint rules in CI | ❌ Not done | Still pending from Epic 7 |
+
+---
+
+## Technical Debt
+
+### From Epic 9 Reviews (LOW)
+
+| # | Item | Source |
+|---|------|--------|
+| 1 | Duplicated `getInterviewDisplayStatus` in `columns.tsx` and `interview-card-list.tsx` | Story 9.2 |
+| 2 | `getApplicationStatuses()` cache has no invalidation | Story 9.1 |
+| 3 | Dashboard `statusCounts` map doesn't initialize "draft" | Story 9.1 |
+| 4 | Test 3.1 in Story 9.4 lacks DOM rendering assertions for field-level errors | Story 9.4 |
+
+### Carry-Forward from Previous Epics
+
+| # | Item | Source |
+|---|------|--------|
+| 1 | Touch target audit (44px minimum) | Epic 7 |
+| 2 | Empty state components | Epic 7 |
+| 3 | Form inputMode/autoComplete completeness | Epic 7 |
+| 4 | ARIA lint rules in CI | Epic 7 |
+
+---
+
+## Key Insights
+
+### 1. Production Smoke Testing Catches What Automated Tests Miss
+The retrospective's production smoke test found 2 issues (session expiry re-auth failure, Draft status missing) that passed all 156 automated tests and all code reviews. Always test the deployed artifact.
+
+### 2. "Bug vs Feature" Labeling Doesn't Matter
+2 of 4 "bugs" were actually missing features. From the user's perspective, broken functionality is broken regardless of whether it's a code defect or a design gap. Focus on fixing what's broken.
+
+### 3. Local Dev Environment Diverges from CI/Production
+`pnpm dev` is more lenient than `pnpm build`. CI test schema is static while production uses migrations. These gaps cause avoidable CI failures.
+
+### 4. Visual Review is a Gap
+Code review catches logic and pattern issues but not visual quality. UI stories need a render check as part of the review process.
+
+---
+
+## Action Items
+
+### Process Improvements
+
+| # | Action | Owner | Deadline | Success Criteria |
+|---|--------|-------|----------|-----------------|
+| 1 | Run `pnpm build` locally before pushing | Dev agent | Starting Epic 10 | Zero build-only CI failures |
+| 2 | Add visual review step to story reviews | Bob + Simon | Before Epic 10 stories ready-for-dev | UI stories include render check |
+| 3 | Automate CI test DB schema (run migrations) | Charlie | Before Epic 10 | New migrations don't break CI |
+| 4 | Fix duplicate CI runs (scope push to main) | Charlie | Before Epic 10 | Single CI run per feature branch push |
+
+### Carry-Forward
+
+| # | Action | Status |
+|---|--------|--------|
+| 1 | Draft → design → refine workflow | Active — apply to Story 10.2 |
+| 2 | Tests required per story | ✅ Continuing |
+| 3 | ARIA lint rules in CI | Active — pending since Epic 7 |
+
+---
+
+## Critical Path
+
+| # | Item | Owner | Priority |
+|---|------|-------|----------|
+| 1 | **Fix session expiry re-authentication** — credentials login fails on `/login?error=SessionExpired&callbackUrl=...` | Dev agent | **BLOCKER** — must fix before Epic 10 |
+
+---
+
+## Production Smoke Test Results
+
+Conducted during retrospective using Chrome DevTools MCP against jobditto.com.
+
+| # | Test | Result | Notes |
+|---|------|--------|-------|
+| 1 | Create application with status selection | ✅ Pass | Status dropdown works |
+| 2 | Change status via detail page dropdown | ✅ Pass | Saved → Applied, toast confirmed |
+| 3 | Dashboard interview count accurate | ✅ Pass | Shows correct count |
+| 4 | Auto-status transition on interview creation | ✅ Pass | Applied → Interview automatically |
+| 5 | Interview "Awaiting Outcome" display | ✅ Pass | Past date + no outcome renders correctly |
+| 6 | Session expiry re-authentication | ❌ FAIL | Credentials rejected on session expiry URL |
+| 7 | Inline field error display | ✅ Pass | Invalid URL shows error with role="alert" |
+| 8 | Error clears on correction | ✅ Pass | Fixing input clears error, enables submit |
+
+---
+
+## Readiness Assessment
+
+| Area | Status | Notes |
+|------|--------|-------|
+| Testing & Quality | ✅ Good | 156 tests passing, production smoke tested |
+| Deployment | ✅ Deployed | Live on jobditto.com |
+| Stakeholder Acceptance | ✅ Accepted | Simon tested and approved |
+| Technical Health | ⚠️ Issue | Session expiry re-auth broken in production |
+| Unresolved Blockers | ⚠️ 1 Blocker | Session expiry fix required before Epic 10 |
+
+---
+
+## Next Steps
+
+1. **Fix session expiry re-authentication** — BLOCKER for Epic 10
+2. **Fix CI: automate test DB schema + scope push trigger to main**
+3. **Add visual review step to story review process**
+4. **Begin Epic 10: Security & Access Hardening** once blocker resolved
+
+---
+
+## Project Journey
+
+| Epic | Name | Stories |
+|------|------|---------|
+| 1 | Enhanced Application Management | 6 |
+| 2 | Deep Interview Management | 12 |
+| 0.1 | Design System Update | 1 |
+| 3 | Technical Assessment Tracking | 8 |
+| 4 | Workflow Automation & Timeline | 6 |
+| 5 | Search, Discovery & Data Management | 5 |
+| 6 | Polish, Performance & Production Readiness | 10 |
+| 7 | CI/CD Pipeline & Production Deployment | 3 |
+| 8 | Multi-Provider Authentication & Account Security | 3 |
+| 9 | Bug Fixes & Core Workflow Stability | 4 |
+| **Total** | | **58 stories** |
+
+**Production:** jobditto.com | **Status:** Live
+
+---
+
+_This retrospective was facilitated using the BMad Method retrospective workflow._
+_Format: BMad Retrospective Workflow v1.0_

docs/planning/sprint-status.yaml

@@ -135,6 +135,7 @@ development_status:
   9-2-fix-interview-status-tracking-and-dashboard: done
   9-3-fix-session-expiry-redirect: done
   9-4-application-form-error-display: done
+  epic-9-retrospective: done
 
   # Epic 10: Security & Access Hardening
   epic-10: backlog

frontend/src/app/(auth)/login/__tests__/page.test.tsx

@@ -5,10 +5,11 @@ import LoginPage from "../page";
 
 const mockPush = jest.fn();
 const mockSignIn = jest.fn();
+let mockSearchParams = new URLSearchParams();
 
 jest.mock("next/navigation", () => ({
   useRouter: () => ({ push: mockPush }),
-  useSearchParams: () => new URLSearchParams(),
+  useSearchParams: () => mockSearchParams,
 }));
 
 jest.mock("next-auth/react", () => ({
@@ -33,6 +34,7 @@ jest.mock("../../components/oauth-buttons", () => ({
 describe("LoginPage", () => {
   beforeEach(() => {
     jest.clearAllMocks();
+    mockSearchParams = new URLSearchParams();
   });
 
   it("renders email and password fields", () => {
@@ -100,6 +102,7 @@ describe("LoginPage", () => {
         email: "test@example.com",
         password: "password123",
         redirect: false,
+        callbackUrl: "/",
       });
     });
   });
@@ -133,4 +136,76 @@ describe("LoginPage", () => {
       ).toBeInTheDocument();
     });
   });
+
+  it("passes valid internal callbackUrl from URL params to signIn", async () => {
+    mockSearchParams = new URLSearchParams("callbackUrl=/applications");
+    mockSignIn.mockResolvedValue({ error: null });
+    const user = userEvent.setup();
+    render(<LoginPage />);
+
+    await user.type(screen.getByLabelText(/email/i), "test@example.com");
+    await user.type(screen.getByLabelText(/password/i), "password123");
+    await user.click(screen.getByRole("button", { name: /sign in/i }));
+
+    await waitFor(() => {
+      expect(mockSignIn).toHaveBeenCalledWith("credentials", {
+        email: "test@example.com",
+        password: "password123",
+        redirect: false,
+        callbackUrl: "/applications",
+      });
+      expect(mockPush).toHaveBeenCalledWith("/applications");
+    });
+  });
+
+  it("sanitizes external callbackUrl to / to prevent open redirect", async () => {
+    mockSearchParams = new URLSearchParams("callbackUrl=//evil.com");
+    mockSignIn.mockResolvedValue({ error: null });
+    const user = userEvent.setup();
+    render(<LoginPage />);
+
+    await user.type(screen.getByLabelText(/email/i), "test@example.com");
+    await user.type(screen.getByLabelText(/password/i), "password123");
+    await user.click(screen.getByRole("button", { name: /sign in/i }));
+
+    await waitFor(() => {
+      expect(mockSignIn).toHaveBeenCalledWith("credentials", {
+        email: "test@example.com",
+        password: "password123",
+        redirect: false,
+        callbackUrl: "/",
+      });
+      expect(mockPush).toHaveBeenCalledWith("/");
+    });
+  });
+
+  it("sanitizes absolute external callbackUrl to /", async () => {
+    mockSearchParams = new URLSearchParams("callbackUrl=https://evil.com");
+    mockSignIn.mockResolvedValue({ error: null });
+    const user = userEvent.setup();
+    render(<LoginPage />);
+
+    await user.type(screen.getByLabelText(/email/i), "test@example.com");
+    await user.type(screen.getByLabelText(/password/i), "password123");
+    await user.click(screen.getByRole("button", { name: /sign in/i }));
+
+    await waitFor(() => {
+      expect(mockSignIn).toHaveBeenCalledWith("credentials", {
+        email: "test@example.com",
+        password: "password123",
+        redirect: false,
+        callbackUrl: "/",
+      });
+      expect(mockPush).toHaveBeenCalledWith("/");
+    });
+  });
+
+  it("shows session expired message when error=SessionExpired in URL", () => {
+    mockSearchParams = new URLSearchParams("error=SessionExpired&callbackUrl=/applications");
+    render(<LoginPage />);
+
+    expect(
+      screen.getByText("Your session has expired. Please sign in again."),
+    ).toBeInTheDocument();
+  });
 });

frontend/src/app/(auth)/login/page.tsx

@@ -32,7 +32,7 @@ const LoginForm = () => {
 
     const urlError = searchParams.get('error');
     const rawCallbackUrl = searchParams.get('callbackUrl');
-    const callbackUrl = rawCallbackUrl?.startsWith('/') ? rawCallbackUrl : '/';
+    const callbackUrl = rawCallbackUrl?.startsWith('/') && !rawCallbackUrl.startsWith('//') ? rawCallbackUrl : '/';
 
     useEffect(() => {
         if (urlError === 'SessionExpired') {
@@ -45,6 +45,7 @@ const LoginForm = () => {
             email: data.email,
             password: data.password,
             redirect: false,
+            callbackUrl,
         });
 
         if (result?.error) {