# Next Steps – Action Plan for Critical Fixes

**Date**: November 21, 2025  
**Status**: Ready for implementation

---

## ✅ COMPLETED IN THIS SESSION

### ISSUE-007: Health Check Status Code
- **Status**: FIXED ✅
- **File**: `apps/api/src/server.ts`
- **Change**: Return HTTP 503 when Redis or Firestore unavailable (was always 200)
- **Impact**: Load balancers now properly drain traffic from degraded servers
- **Test**: Run `curl http://localhost:3000/api/health` (should return 503 if Redis down)

**Verification**:
```bash
pnpm type-check  # ✅ Pass
```

---

## 🔴 HIGH PRIORITY (This Week)

### ISSUE-005: Auth Role Precedence (SECURITY)
**File**: `apps/api/src/middleware/auth.middleware.ts`  
**Complexity**: Medium (2-4 hours)

**What to do**:
1. Define explicit role precedence:
   ```
   Precedence for 'admin' flag:
   1. Firestore admin=true/false → ALWAYS use (local control)
   2. Fallback to Auth0 claim (IdP source of truth)
   3. Fallback to roles array parsing
   ```
2. Add warning log when Firestore conflicts with Auth0
3. Add unit tests for scenarios:
   - `admin=false` in Firestore → should revoke even if Auth0 says admin
   - `roles=['admin']` in Firestore → should grant admin
   - Missing Firestore data → should use Auth0

**Acceptance Criteria**:
- [ ] Firestore admin field is source of truth for revocation
- [ ] Auth0 is fallback when Firestore doesn't specify
- [ ] Unit tests pass (all merge scenarios)
- [ ] Type-check passes
- [ ] No console.errors in test output

---

### ISSUE-004: CRDT Decompression Silent Fallback (DATA INTEGRITY)
**File**: `apps/api/src/modules/collab/collab.service.ts`  
**Complexity**: Medium (2-3 hours)

**What to do**:
1. Change lines 130-145 to handle decompression failure properly
2. **Option A (Recommended)**: Throw error and let caller handle (forces fresh sync)
   ```typescript
   if (this.compressionEnabled && cachedState.length > 0) {
     try {
       stateBuffer = await gunzipAsync(cachedState)
     } catch (err) {
       logger.error('CRITICAL: Y.Doc decompression failed', { planId, error: err })
       throw new Error(`CRDT state corrupted for plan ${planId}`)
     }
   }
   ```
3. Add monitoring/alert for this error
4. Document the decision in comments

**Acceptance Criteria**:
- [ ] No silent fallback to corrupted data
- [ ] Clear error message when decompression fails
- [ ] Monitoring alert configured in Cloud Run
- [ ] Type-check passes
- [ ] Integration test with forced decompression failure

---

### ISSUE-006: OpenAI Response Validation (INPUT HANDLING)
**File**: `apps/api/src/services/openai.service.ts`  
**Complexity**: Low (1-2 hours)

**What to do**:
1. Add array size validation (max 20 POIs):
   ```typescript
   if (!Array.isArray(parsed) || parsed.length === 0 || parsed.length > 20) {
     throw new Error(`Expected 1-20 POIs, got ${parsed.length}`)
   }
   ```
2. Validate each POI strictly:
   - `name`: must be string, max 200 chars, not empty
   - `lat/lng`: must be valid numbers, not fallback
   - `category`: must be in allowed list
3. Test with edge cases (empty array, 100 POIs, malformed JSON)

**Acceptance Criteria**:
- [ ] Array size checked (max 20)
- [ ] Invalid coordinates rejected (no fallback)
- [ ] Strings sanitized (max 200 chars)
- [ ] Unit tests pass with edge cases
- [ ] Type-check passes

---

## 🟡 MEDIUM PRIORITY (Next 2 Weeks)

### ISSUE-009: Frontend Async Init Race Condition
**File**: `apps/web/src/app/main.ts`  
**Complexity**: Medium (1-2 hours)

**Decision Needed**: 
- Option A: Block mount until auth ready (slower but correct)
- Option B: Show loading skeleton (faster but requires UI)

**Suggested Implementation** (Option A – safer):
```typescript
const app = createApp(App)
app.use(pinia)
app.use(router)

// Initialize auth BEFORE mount
const authStore = useAuthStore()
const redirectTarget = await authStore.initialize()

app.mount('#app')

if (redirectTarget && redirectTarget !== router.currentRoute.value.fullPath) {
  await router.replace(redirectTarget)
}
```

**Acceptance Criteria**:
- [ ] App doesn't mount until auth initialized
- [ ] Router guards work correctly
- [ ] No 401 errors due to missing interceptors
- [ ] E2E tests pass (navigation works)

---

### ISSUE-008: Module Registration Error Handling
**File**: `apps/api/src/core/modules.ts`  
**Complexity**: Medium (1-2 hours)

**What to do**:
1. Wrap module registrations in try-catch or use Promise.allSettled()
2. Collect all errors and report together
3. Fail app startup if any module registration fails

```typescript
const results = await Promise.allSettled([
  app.register(plannerModule, { prefix: '/api/planner' }),
  app.register(aiModule, { prefix: '/api/ai' }),
  // ...
])

const failures = results.filter(r => r.status === 'rejected')
if (failures.length > 0) {
  logger.error('Module registration failures', { 
    count: failures.length,
    errors: failures.map(r => r.reason)
  })
  throw new Error(`${failures.length} modules failed to register`)
}
```

**Acceptance Criteria**:
- [ ] All modules register or app fails loudly
- [ ] Error message lists which modules failed
- [ ] No partially initialized state
- [ ] Unit test for module failure scenario

---

### ISSUE-010: Gamification Event Emitter Singleton
**File**: `apps/api/src/core/modules.ts`  
**Complexity**: Medium (2-3 hours)

**What to do**:
1. Create singleton event emitter at app startup (not per-module)
2. Pass reference to gamification module
3. Ensure listeners registered before app ready

**Decision Needed**: 
- Keep current EventEmitter pattern OR
- Use Redis pub/sub for distributed events (better for multiple server instances)

**Acceptance Criteria**:
- [ ] Event emitter is singleton
- [ ] Listeners persist across module registrations
- [ ] Events reach all listeners even if registered late
- [ ] Monitoring for events published (counter)

---

## 📋 LOW PRIORITY (Nice to Have)

- [ ] Remove all console.log (replace with logger)
- [ ] Verify sharingRoutesCompat is still used
- [ ] Add integration tests for CRDT decompression
- [ ] Document auth system (Auth0 vs Firestore)
- [ ] Create system diagram for state flow

---

## Testing Strategy

### Unit Tests to Add
```bash
# Auth roles merging
apps/api/src/middleware/__tests__/auth.middleware.test.ts

# CRDT decompression
apps/api/src/modules/collab/__tests__/collab.service.decompression.test.ts

# OpenAI validation
apps/api/src/services/__tests__/openai.service.validation.test.ts

# Module registration errors
apps/api/src/core/__tests__/modules.error-handling.test.ts
```

### Integration Tests
```bash
# CRDT sync with Redis failure
# OpenAI response with extreme edge cases
# Auth role revocation flow
# Frontend router guards before auth ready
```

### Manual Testing Checklist
- [ ] Admin revokes their own admin role → can't access admin panel
- [ ] CRDT sync continues working with Redis flaky
- [ ] App doesn't render before auth ready
- [ ] Health check returns 503 when Redis down

---

## Deployment Plan

### Pre-Deployment
1. Run tests: `pnpm test`
2. Type-check: `pnpm type-check`
3. Lint: `pnpm lint`
4. Build: `pnpm build`

### Deployment
1. Deploy health check fix first (ISSUE-007) ✅ DONE
2. Deploy auth fixes (ISSUE-005)
3. Deploy CRDT fixes (ISSUE-004)
4. Deploy validation (ISSUE-006)
5. Monitor: Check Sentry, Cloud Run logs, Redis latency

### Rollback Plan
- If CRDT corruption detected: Revert ISSUE-004 and restore from last backup
- If auth bypass detected: Revert ISSUE-005
- If health check breaks deployments: Revert ISSUE-007

---

## Monitoring & Alerts

Configure these alerts:
```
1. CRDT decompression failures (rate > 0)
2. Auth0 vs Firestore role conflicts (rate > 5/min)
3. OpenAI response validation errors (rate > 10/min)
4. Module registration failures (rate > 0)
5. Redis health endpoint returns 503 (sustained > 5min)
```

---

## Documentation Updates

- [ ] Update `.github/instructions/FOCUS.instructions.md` with auth system explanation
- [ ] Document CRDT sync failure handling strategy
- [ ] Document role precedence in auth.middleware.ts
- [ ] Add architecture decision log for event emitter pattern

---

## Success Criteria

After all fixes applied:
- ✅ No silent error fallbacks in critical paths
- ✅ Role precedence clearly defined and tested
- ✅ Input validation for all external APIs
- ✅ Health checks return correct status codes
- ✅ Module registration fails loudly
- ✅ Frontend doesn't race with auth initialization
- ✅ Type-check, lint, tests all pass
- ✅ Zero new security findings in code review

---

## Questions for Team

1. **ISSUE-004**: Should CRDT decompression failure throw or reset to empty?
2. **ISSUE-005**: Can we do role precedence design session?
3. **ISSUE-009**: Should we block app mount until auth, or show loading?
4. **ISSUE-010**: Should events use Redis pub/sub for multi-instance setup?

---

**Next Session**: Implement fixes in order: 005 → 004 → 006 → 009 → 008 → 010

