Skip to content

Commit e1539b8

Browse files
authored
fix: prevent duplicate task generation button for late-joining users (#228)
* fix(ui): prevent duplicate task generation button for late-joining users This addresses a UX issue where users who log into a project where tasks have already been generated still see the "Generate Task Breakdown" button, which fails with a 400 error when clicked. Changes: - Backend: Make generate-tasks endpoint idempotent by returning {success: true, tasks_already_exist: true} instead of 400 error when tasks already exist - Frontend: Check for existing tasks on component mount during state initialization (in fetchProgress with initializePrdState) - Frontend: Handle idempotent backend response gracefully in handleGenerateTaskBreakdown - Types: Add optional tasks_already_exist field to API response type This implements defensive programming on both sides: 1. Frontend checks task existence on mount (covers late-joining users) 2. Backend handles duplicate requests gracefully (covers edge cases) 3. Error handler transitions to "tasks ready" state on idempotent response TDD: Tests written first, all 95 frontend + 9 backend tests passing. * test(e2e): add late-joining user tests for task generation Addresses a critical E2E test coverage gap: no tests verified UI state for users who arrive AFTER WebSocket events have occurred. Changes: - Add test_late_joining_user.spec.ts with scenarios: - Tasks already exist → should see "Review Tasks" not "Generate Tasks" - PRD already complete → should see "View PRD" with correct state - Idempotent API call → backend returns success, not error - Update seed-test-data.py to create "planning" phase project: - Set project phase to "planning" instead of "discovery" - Set discovery state to "completed" - Seed PRD content in memory table Why this gap existed: - All existing E2E tests followed real-time happy path flow - Tests assumed user was present during WebSocket events - Conditional skip patterns (test.skip when element not visible) masked the issue instead of catching it This ensures future similar bugs are caught by E2E tests. * fix(e2e): correct projects table schema in seed script The seed script was using incorrect columns (slug, updated_at) that don't exist in the projects table. Updated to use the correct schema: id, name, description, user_id, workspace_path, phase, created_at This enables proper seeding of Project 2 for late-joining user tests. * fix(e2e): complete Project 2 seed data for late-joining user tests The dashboard was crashing for Project 2 due to missing required fields: - Added `status` field (was null, causing .toUpperCase() error) - Added workspace directory creation - Added project-agent assignments - Fixed tasks table columns (assigned_to not assigned_agent) Also updated debug-error.spec.ts to use E2E_TEST_PROJECT_PLANNING_ID for testing Project 2 scenarios. * fix: address code review feedback for late-joining user feature - Fix agent ID mismatch: backend-001 → backend-worker-001, frontend-001 → frontend-specialist-001 - Add taskStateInitialized flag to prevent race condition during async task check - Add user notification when tasks already exist (tasksAlreadyExistMessage) - Simplify E2E test assertions with explicit expect() calls instead of conditionals - API type already had tasks_already_exist field (verified, no change needed) * test(e2e): add curated smoke test suite with @smoke tagging - Update test:smoke script to filter by @smoke tag (Chromium only) - Tag 4 critical-path tests as @smoke: - Auth: login with valid credentials - Project: create new project via UI - Dashboard: display all main sections - Late-joining: show Tasks Ready when tasks exist - Remove WebSocket test from smoke suite (flaky, needs investigation) - Smoke suite runs in ~45s for fast CI feedback * fix(websocket): queue subscriptions when socket not yet open Root cause: subscribe() silently did nothing when called before the WebSocket was OPEN. AgentStateProvider calls connect() then immediately subscribe(), but the socket is still CONNECTING, causing the subscribe message to never be sent. Fix: Add pendingSubscriptions queue that holds project IDs until the socket opens, then sends all queued subscriptions in onopen handler. This fixes the WebSocket E2E test regression and re-enables it as a smoke test. * fix(e2e): align tasks_p2 schema with 22-column tasks table format Update Project 2 task seeding to use the full tasks table schema: - Add all 22 columns (issue_id, task_number, depends_on, etc.) - Include realistic values for tokens, quality gates, timestamps - Match the format used for Project 1 tasks This ensures consistency between projects and prevents potential schema mismatches that could cause test failures. * fix(ux): convert tasks-already-exist notification to fixed-position toast Convert the inline notification for "tasks already exist" (idempotent response) to a fixed-position toast notification in the bottom-right corner. This ensures the feedback is always visible regardless of scroll position. - Moved notification outside main content flow - Added fixed positioning (bottom-4 right-4) - Added shadow and slide-in animation - Maintains accessibility attributes (role="status", aria-live="polite") * test: add missing API mocks for task-generation-section tests The tests expecting task-generation-section to appear were missing mocks for mockGetPRD and mockTasksList. These are required because the component now waits for taskStateInitialized before showing the "Generate Task Breakdown" button (prevents button flash on mount). * fix(e2e): correct project 2 seeding schema alignment Two fixes for seed-test-data.py: 1. Add missing is_active column to project 2 agent assignments - Schema requires 5 columns: project_id, agent_id, role, is_active, assigned_at - Was using 4 columns, causing NOT NULL constraint errors 2. Add table_exists guard for project 2 memory table inserts - Discovery state and PRD content inserts now check TABLE_MEMORY exists - Matches defensive pattern used for project 1 * fix(react): prevent state update on unmounted component Replace inline setTimeout in handleGenerateTaskBreakdown with a useEffect that properly manages the timeout lifecycle. This prevents the React warning "Can't perform a React state update on an unmounted component" if the component unmounts before the 3-second timeout completes. The useEffect: - Watches tasksAlreadyExistMessage state - Sets timeout only when message is true - Returns cleanup function that clears the timeout on unmount
1 parent d2bfd90 commit e1539b8

File tree

13 files changed

+820
-25
lines changed

13 files changed

+820
-25
lines changed

codeframe/ui/routers/discovery.py

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -692,13 +692,19 @@ async def generate_tasks(
692692
"Please complete PRD generation first.",
693693
)
694694

695-
# Check if tasks already exist to prevent duplicate generation
695+
# Check if tasks already exist - return idempotent success instead of error
696+
# This improves UX for users who join late and miss WebSocket events
696697
existing_tasks = db.get_project_tasks(project_id)
697698
if existing_tasks:
698-
raise HTTPException(
699-
status_code=400,
700-
detail="Tasks have already been generated for this project.",
699+
logger.info(
700+
f"Tasks already exist for project {project_id} "
701+
f"(count: {len(existing_tasks)}). Returning idempotent success."
701702
)
703+
return {
704+
"success": True,
705+
"message": "Tasks have already been generated for this project.",
706+
"tasks_already_exist": True,
707+
}
702708

703709
# Verify API key is available
704710
api_key = os.environ.get("ANTHROPIC_API_KEY")

tests/api/test_generate_tasks_endpoint.py

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -191,8 +191,13 @@ def test_background_task_receives_correct_parameters(
191191
# Note: In TestClient, background tasks are executed synchronously
192192
# so we can verify the function was called correctly
193193

194-
def test_returns_400_when_tasks_already_exist(self, api_client):
195-
"""Test endpoint returns 400 when tasks have already been generated."""
194+
def test_returns_200_with_flag_when_tasks_already_exist(self, api_client):
195+
"""Test endpoint is idempotent - returns 200 with tasks_already_exist flag.
196+
197+
Instead of returning 400 error when tasks exist, the endpoint should
198+
be idempotent and return success with a flag indicating tasks already
199+
exist. This improves UX for users who join late and miss WebSocket events.
200+
"""
196201
# ARRANGE
197202
db = get_db_from_client(api_client)
198203
project_id = db.create_project("test-project", "Test Project")
@@ -216,10 +221,12 @@ def test_returns_400_when_tasks_already_exist(self, api_client):
216221
f"/api/projects/{project_id}/discovery/generate-tasks"
217222
)
218223

219-
# ASSERT
220-
assert response.status_code == 400
221-
detail = response.json()["detail"].lower()
222-
assert "already" in detail or "exist" in detail
224+
# ASSERT - idempotent endpoint returns 200 with flag
225+
assert response.status_code == 200
226+
data = response.json()
227+
assert data["success"] is True
228+
assert data["tasks_already_exist"] is True
229+
assert "already" in data["message"].lower()
223230

224231

225232
class TestGenerateTasksEndpointIntegration:

tests/e2e/debug-error.spec.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ import { test } from '@playwright/test';
1010
import { loginUser } from './test-utils';
1111

1212
const FRONTEND_URL = process.env.FRONTEND_URL || 'http://localhost:3001';
13-
const PROJECT_ID = process.env.E2E_TEST_PROJECT_ID || '1';
13+
// Use Project 2 for testing late-joining user scenarios (matches late-joining tests)
14+
const PROJECT_ID = process.env.E2E_TEST_PROJECT_PLANNING_ID || '2';
1415

1516
test('capture browser console errors', async ({ page }) => {
1617
// Login first to access protected routes
@@ -33,8 +34,12 @@ test('capture browser console errors', async ({ page }) => {
3334
});
3435

3536
// Navigate to dashboard
36-
console.log('\n=== Navigating to dashboard ===');
37-
await page.goto(`${FRONTEND_URL}/projects/${PROJECT_ID}`);
37+
const targetUrl = `${FRONTEND_URL}/projects/${PROJECT_ID}`;
38+
console.log(`\n=== Navigating to dashboard ===`);
39+
console.log(`Target URL: ${targetUrl}`);
40+
console.log(`PROJECT_ID: ${PROJECT_ID}`);
41+
await page.goto(targetUrl);
42+
console.log(`Current URL after navigation: ${page.url()}`);
3843

3944
// Wait a bit to see what happens
4045
await page.waitForTimeout(5000);

tests/e2e/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
"private": true,
66
"scripts": {
77
"test": "playwright test",
8-
"test:smoke": "playwright test --project=chromium",
8+
"test:smoke": "playwright test --grep @smoke --project=chromium",
99
"test:headed": "playwright test --headed",
1010
"test:debug": "playwright test --debug",
1111
"test:chromium": "playwright test --project=chromium",

tests/e2e/seed-test-data.py

Lines changed: 181 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1179,10 +1179,11 @@ def seed_test_data(db_path: str, project_id: int):
11791179
print(f"✅ Seeded {count}/3 discovery state entries for project {project_id}")
11801180

11811181
# ========================================
1182-
# 8. Update Project Phase for Discovery Tests
1182+
# 8. Update Project Phase for Discovery Tests (Project 1)
11831183
# ========================================
11841184
# Set project to 'discovery' phase so discovery UI renders correctly
1185-
print("📋 Setting project phase to 'discovery'...")
1185+
# This project is used by test_start_agent_flow.spec.ts and other discovery tests
1186+
print("📋 Setting project phase to 'discovery' for discovery tests...")
11861187
try:
11871188
cursor.execute(
11881189
"UPDATE projects SET phase = 'discovery' WHERE id = ?",
@@ -1192,6 +1193,184 @@ def seed_test_data(db_path: str, project_id: int):
11921193
except sqlite3.Error as e:
11931194
print(f"⚠️ Failed to update project phase: {e}")
11941195

1196+
# ========================================
1197+
# 9. Create Second Project for Late-Joining User Tests (Project 2)
1198+
# ========================================
1199+
# Create a separate project in 'planning' phase with completed PRD and tasks
1200+
# This enables testing late-joining user scenarios where tasks already exist
1201+
print("\n📦 Creating second project for late-joining user tests...")
1202+
planning_project_id = 2
1203+
1204+
# Create workspace directory for Project 2 (required for dashboard to load)
1205+
workspace_path_p2 = os.path.join(E2E_TEST_ROOT, ".codeframe", "workspaces", str(planning_project_id))
1206+
os.makedirs(workspace_path_p2, exist_ok=True)
1207+
print(f" 📁 Created workspace: {workspace_path_p2}")
1208+
1209+
# Use INSERT OR REPLACE to ensure project exists with correct data
1210+
# This handles both fresh installs and re-runs
1211+
# NOTE: Both 'status' and 'phase' must be set - status is used in Dashboard header
1212+
cursor.execute(
1213+
"""
1214+
INSERT OR REPLACE INTO projects (id, name, description, user_id, workspace_path, status, phase, created_at)
1215+
VALUES (?, ?, ?, ?, ?, ?, ?, ?)
1216+
""",
1217+
(
1218+
planning_project_id,
1219+
"e2e-planning-project",
1220+
"Test project in planning phase with tasks (for late-joining user tests)",
1221+
1, # test user
1222+
workspace_path_p2,
1223+
"planning", # status - required for Dashboard to render (not null)
1224+
"planning", # phase - project lifecycle phase
1225+
now_ts,
1226+
),
1227+
)
1228+
print(f"✅ Created/updated project {planning_project_id} in 'planning' phase")
1229+
1230+
# Add completed discovery state for project 2 (guard against missing memory table)
1231+
if table_exists(cursor, TABLE_MEMORY):
1232+
discovery_entries_p2 = [
1233+
(planning_project_id, "discovery_state", "state", "completed", now_ts, now_ts),
1234+
]
1235+
for entry in discovery_entries_p2:
1236+
cursor.execute(
1237+
"""
1238+
INSERT OR REPLACE INTO memory (project_id, category, key, value, created_at, updated_at)
1239+
VALUES (?, ?, ?, ?, ?, ?)
1240+
""",
1241+
entry,
1242+
)
1243+
1244+
# Add PRD content for project 2
1245+
prd_content = """# Project Requirements Document
1246+
1247+
## Overview
1248+
This is a test PRD for late-joining user E2E tests.
1249+
1250+
## Features
1251+
1. User authentication
1252+
2. Project management
1253+
3. Task tracking
1254+
1255+
## Technical Requirements
1256+
- FastAPI backend
1257+
- Next.js frontend
1258+
- SQLite database
1259+
1260+
## Timeline
1261+
Sprint 1: Core features
1262+
Sprint 2: Testing and polish
1263+
"""
1264+
cursor.execute(
1265+
"""
1266+
INSERT OR REPLACE INTO memory (project_id, category, key, value, created_at, updated_at)
1267+
VALUES (?, 'prd', 'content', ?, ?, ?)
1268+
""",
1269+
(planning_project_id, prd_content, now_ts, now_ts),
1270+
)
1271+
else:
1272+
print(f"⚠️ Warning: {TABLE_MEMORY} table doesn't exist, skipping project 2 discovery state and PRD")
1273+
1274+
# Clear existing tasks for project 2 before re-seeding (ensures clean state)
1275+
cursor.execute("DELETE FROM tasks WHERE project_id = ?", (planning_project_id,))
1276+
1277+
# Add tasks for project 2 (so late-joining user tests can verify task state)
1278+
# Using full 22-column tasks schema to match Project 1 format
1279+
# Schema: id, project_id, issue_id, task_number, parent_issue_number, title, description,
1280+
# status, assigned_to, depends_on, can_parallelize, priority, workflow_step,
1281+
# requires_mcp, estimated_tokens, actual_tokens, created_at, completed_at,
1282+
# commit_sha, quality_gate_status, quality_gate_failures, requires_human_approval
1283+
tasks_p2 = [
1284+
(
1285+
None, # id (auto-increment)
1286+
planning_project_id,
1287+
None, # issue_id
1288+
"T001", # task_number
1289+
None, # parent_issue_number
1290+
"Implement user authentication", # title
1291+
"Set up JWT-based authentication", # description
1292+
"completed", # status
1293+
"backend-worker-001", # assigned_to
1294+
None, # depends_on
1295+
0, # can_parallelize
1296+
3, # priority (high)
1297+
1, # workflow_step
1298+
0, # requires_mcp
1299+
5000, # estimated_tokens
1300+
4800, # actual_tokens
1301+
now_ts, # created_at
1302+
now_ts, # completed_at
1303+
"abc123", # commit_sha
1304+
"passed", # quality_gate_status
1305+
None, # quality_gate_failures
1306+
0, # requires_human_approval
1307+
),
1308+
(
1309+
None, # id (auto-increment)
1310+
planning_project_id,
1311+
None, # issue_id
1312+
"T002", # task_number
1313+
None, # parent_issue_number
1314+
"Create project dashboard", # title
1315+
"Build the main dashboard UI", # description
1316+
"in_progress", # status
1317+
"frontend-specialist-001", # assigned_to
1318+
"1", # depends_on (depends on T001)
1319+
1, # can_parallelize
1320+
2, # priority (medium)
1321+
2, # workflow_step
1322+
0, # requires_mcp
1323+
8000, # estimated_tokens
1324+
3500, # actual_tokens
1325+
now_ts, # created_at
1326+
None, # completed_at
1327+
None, # commit_sha
1328+
None, # quality_gate_status
1329+
None, # quality_gate_failures
1330+
0, # requires_human_approval
1331+
),
1332+
]
1333+
for task in tasks_p2:
1334+
cursor.execute(
1335+
"""
1336+
INSERT INTO tasks (
1337+
id, project_id, issue_id, task_number, parent_issue_number, title, description,
1338+
status, assigned_to, depends_on, can_parallelize, priority, workflow_step,
1339+
requires_mcp, estimated_tokens, actual_tokens, created_at, completed_at,
1340+
commit_sha, quality_gate_status, quality_gate_failures, requires_human_approval
1341+
)
1342+
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
1343+
""",
1344+
task,
1345+
)
1346+
1347+
cursor.execute(
1348+
"SELECT COUNT(*) FROM tasks WHERE project_id = ?",
1349+
(planning_project_id,),
1350+
)
1351+
task_count = cursor.fetchone()[0]
1352+
print(f"✅ Seeded {task_count} tasks for project {planning_project_id}")
1353+
1354+
# Add project-agent assignments for project 2 (required for Dashboard to render)
1355+
# Clear existing assignments first
1356+
cursor.execute("DELETE FROM project_agents WHERE project_id = ?", (planning_project_id,))
1357+
# Use the same agents seeded for project 1
1358+
# Schema: project_id, agent_id, role, is_active, assigned_at (5 columns - must match project 1)
1359+
project_agent_assignments_p2 = [
1360+
(planning_project_id, "backend-worker-001", "developer", 1, now_ts),
1361+
(planning_project_id, "frontend-specialist-001", "developer", 1, now_ts),
1362+
]
1363+
for assignment in project_agent_assignments_p2:
1364+
cursor.execute(
1365+
"""
1366+
INSERT INTO project_agents (project_id, agent_id, role, is_active, assigned_at)
1367+
VALUES (?, ?, ?, ?, ?)
1368+
""",
1369+
assignment,
1370+
)
1371+
print(f"✅ Seeded {len(project_agent_assignments_p2)} project-agent assignments for project {planning_project_id}")
1372+
print(f"✅ Set E2E_TEST_PROJECT_PLANNING_ID={planning_project_id}")
1373+
11951374
# Commit all changes
11961375
conn.commit()
11971376
print(f"\n✅ Test data seeding complete for project {project_id}!")

tests/e2e/test_auth_flow.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ test.describe('Authentication Flow', () => {
137137
});
138138

139139
test.describe('Login Success', () => {
140-
test('should login successfully with valid credentials', async ({ page }) => {
140+
test('should login successfully with valid credentials @smoke', async ({ page }) => {
141141
await page.goto('/login');
142142

143143
// Fill in credentials

tests/e2e/test_dashboard.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ test.describe('Dashboard - Sprint 10 Features', () => {
148148
]);
149149
});
150150

151-
test('should display all main dashboard sections', async () => {
151+
test('should display all main dashboard sections @smoke', async () => {
152152
// Verify main dashboard elements are visible with waits
153153
const header = page.locator('[data-testid="dashboard-header"]');
154154
await header.waitFor({ state: 'visible', timeout: 15000 });
@@ -287,7 +287,7 @@ test.describe('Dashboard - Sprint 10 Features', () => {
287287
await expect(page.locator('[data-testid="total-cost-display"]')).toBeAttached();
288288
});
289289

290-
test('should receive real-time updates via WebSocket', async () => {
290+
test('should receive real-time updates via WebSocket @smoke', async () => {
291291
// Step 1: Verify WebSocket backend endpoint is ready
292292
await waitForWebSocketReady(BACKEND_URL);
293293

0 commit comments

Comments
 (0)