✨ Refactor TDD dashboard to use TanStack Query and add remote builds page

[Robdel12]

Summary

• Replace custom hooks with TanStack Query for cleaner data fetching and caching
• Add new /builds route for browsing remote builds and downloading baselines
• Extract HTTP server into modular routers for better maintainability
• Add unified API client for frontend
• Improve error messages for baseline download failures
• Remove recent builds card from projects page (now has dedicated route)

Test plan

• All 792 tests pass
• Lint passes with 0 errors/warnings
• Build succeeds
• Manual testing of TDD dashboard views
• Manual testing of baseline download from remote builds

Note: Requires corresponding backend changes to deploy the /api/build/:projectSlug/:buildId/tdd-baselines endpoint.

[claude]

Pull Request Review: TanStack Query Refactor + Remote Builds

Overview

This is a well-architected refactoring that modernizes the TDD dashboard with industry-standard patterns. The core changes—migrating to TanStack Query and modularizing the HTTP server—improve both maintainability and user experience.

🎯 Strengths

Architecture & Design

• Modular router system: The extraction of HTTP server logic into focused routers (src/server/routers/*) is excellent. Each router has a single responsibility, making the codebase much easier to navigate and test.
• Unified API client: The new src/reporter/src/api/client.js provides a clean, namespaced interface (api.tdd., api.cloud., etc.) that is well-documented and type-hinted.
• Query hooks organization: Grouping queries by domain (use-tdd-queries.js, use-cloud-queries.js, etc.) follows React Query best practices.

New Features

• Remote builds view: The /builds route is a powerful addition that lets users browse and download baselines from cloud builds—great for team collaboration.
• OAuth + API token dual auth: The fallback pattern in ProjectService (try OAuth, fall back to API token) ensures a smooth UX whether users are logged in or using tokens.

Code Quality

• Proper error handling: The fetchJson utility in api/client.js attaches status codes and response data to errors, enabling better error UX.
• Static vs live mode: The query hooks correctly detect static mode and disable polling/mutations—prevents confusing errors in static reports.
• Concurrent service resolution: The fix in ServiceContainer (lines 58-83) stores promises to handle concurrent get() calls is a smart solution to race conditions.

⚠️ Issues & Concerns

1. Circular Dependency Risk in Container (Medium Severity)

Location: src/container/index.js:203-223

projectService now depends on authService, and tddService also depends on authService. While the new promise-based circular dependency detection should prevent infinite loops, this creates tight coupling.

Impact: If authService later needs to depend on projectService (e.g., to fetch user projects during auth), you will have a circular dependency.

Recommendation:

• Consider whether authService truly belongs as a constructor dependency vs. being passed at call-time
• Document the dependency graph to prevent future circular issues
• Add integration tests that verify container startup succeeds

[claude]

⚠️ Issues & Concerns (continued)

2. Missing Error Boundary in React App (Medium Severity)

Location: src/reporter/src/components/app-router.jsx

The TanStack Query migration introduces async errors that could crash the entire React app if unhandled. Currently, errors are caught per-query, but a top-level error boundary is missing.

Example failure case: If the query client itself throws during hydration or a network error occurs during a mutation, the entire app could white-screen.

Recommendation: Wrap the entire app in an ErrorBoundary component

3. Unsafe Console.log in Production Code (Low Severity)

Location: src/reporter/src/hooks/queries/use-tdd-queries.js:127

Using console.log in production code is generally discouraged. While this is in a no-op mutation, it should either use a proper logging utility or be removed entirely (rejection is truly a no-op).

4. Potential Memory Leak in Query Polling (Low-Medium Severity)

Location: src/reporter/src/hooks/queries/use-tdd-queries.js:38-42

Queries poll every 2 seconds by default in live mode. If the user navigates away but the component is not unmounted (e.g., in a tab they switched from), polling continues indefinitely.

Recommendation:

• Add refetchOnWindowFocus: always to resume fresh polling when user returns
• Consider refetchIntervalInBackground: false to pause polling when tab is hidden
• Document the polling behavior in component comments

5. Type Safety Gap in API Client (Low Severity)

Location: src/reporter/src/api/client.js

The API client lacks TypeScript/JSDoc type definitions for response shapes. This makes it harder for consumers to know what they will receive.

Recommendation: Add JSDoc type definitions for common response shapes (Build, Project, Screenshot, etc.)

6. Missing Input Validation in Downloads (Medium Severity)

Location: src/services/tdd-service.js:613-632

The downloadBaselinesWithAuth method constructs API endpoints from user-provided slugs without validation. If projectSlug or buildId contain special characters or path traversal sequences, this could cause unexpected API calls.

Recommendation: Add validation similar to the screenshot name sanitization for slug format validation

7. Race Condition in Baseline Downloads (Low Severity)

Location: src/reporter/src/components/views/builds-view.jsx:95-106

The downloadingBuildId state tracking means only one download can be in progress at a time across the entire UI. If a user clicks multiple download buttons quickly, subsequent clicks will be ignored silently.

Recommendation: Use a Set of downloading IDs instead of a single ID to allow concurrent downloads

[claude]

🔍 Performance & Security Considerations

Performance

Query Deduplication ✅ TanStack Query automatically deduplicates concurrent requests to the same endpoint—excellent for the polling scenario.

Bundle Size ⚠️ Adding TanStack Query adds approximately 13KB gzipped. Given the benefits, this is acceptable, but monitor bundle size as you add more dependencies.

Polling Impact ⚠️ Multiple active queries polling every 1-2 seconds could impact battery life on mobile devices viewing the dashboard. Consider increasing intervals or using WebSocket for real-time updates.

Security

Path Traversal Prevention ✅ Good use of sanitizeScreenshotName and safePath in tdd-service.js (lines 668-682)

API Token Exposure ✅ Tokens are properly stored in global config, not exposed in frontend code

CORS Configuration ✅ CORS middleware is properly configured in src/server/middleware/cors.js

Recommendation: Rate Limiting - Consider adding rate limiting to the download endpoint to prevent abuse (e.g., max 1 download per 5 seconds per user)

📝 Test Coverage

What is Covered

• ✅ All 792 tests pass
• ✅ Lint passes with 0 warnings

What is Missing

I recommend adding tests for:

1. Service container concurrent access - Verify the promise-based resolution prevents race conditions
2. Query hook error states - Test what happens when API calls fail
3. Static vs live mode switching - Ensure mutations are properly disabled
4. Download baseline OAuth flow - Integration test for the new OAuth download path
5. Router edge cases - 404 handling, malformed request bodies, etc.

📚 Documentation Needs

1. Migration guide - Document how the old hooks map to new query hooks for future contributors
2. Query invalidation patterns - When/how queries are invalidated could be documented
3. Router architecture - Add a README in src/server/routers/ explaining the pattern
4. API client usage - Examples of how to use the new api.* namespaces

[claude]

✅ Minor Suggestions

1. Query keys organization (src/reporter/src/lib/query-keys.js:25): Consider namespacing by feature for better organization

2. Error messages: The error message "Request failed" in fetchJson (line 30) is vague. Include the URL in error messages

3. Component split: builds-view.jsx is 414 lines. Consider splitting into smaller components (BuildCard.jsx, ProjectCard.jsx, OrganizationSection.jsx)

4. Duplicate code: The time formatting logic (getTimeAgo) in builds-view.jsx:71 should be extracted to a utility

🎯 Verdict

Recommendation: ✅ Approve with minor fixes

This is high-quality work that significantly improves the codebase. The issues identified are manageable and do not block merging:

Before merging:

1. Add input validation for download API slugs (security issue ✅ Add comprehensive test suite #6)
2. Remove console.log or use proper logging (♻️ Refactor server architecture with new handlers #3)
3. Add error boundary to React app (♻️ Run lint before tests #2)

Post-merge (can be follow-up PRs):

1. Add integration tests for container concurrent access
2. Implement rate limiting for downloads
3. Add JSDoc types to API client
4. Split large components

Great work on this refactor! The new architecture sets a solid foundation for future dashboard features. 🚀

---

Review generated for PR #85 - vizzly-testing/cli