🐛 Wire authService and projectService into TDD server

[Robdel12]

Summary

The TDD dashboard Projects and Builds tabs were showing "Not signed in" even when the user was authenticated via vizzly login. This was because the TDD command only passed configService to the server manager, leaving authService and projectService undefined.

Root cause: In src/commands/tdd.js:124, only configService was passed to createServerManager(). The HTTP server's auth and project routers checked for these services and returned "Service unavailable" when they were undefined.

Changes:

• Add auth-service.js - wraps auth operations (isAuthenticated, whoami, initiateDeviceFlow, etc.) for the HTTP server
• Add project-service.js - wraps project operations (listProjects, listMappings, getRecentBuilds, etc.) for the HTTP server
• Wire both services into the TDD command alongside configService
• Add unit tests for both new services

Test plan

• Run vizzly login to authenticate
• Run vizzly tdd start
• Open http://localhost:47392 and navigate to Projects tab
• Verify user is shown as signed in (not "Not signed in")
• Verify project mappings load correctly
• Run unit tests: node --test tests/services/auth-service.test.js tests/services/project-service.test.js

[Robdel12]

@claude rereview please

[claude]

Claude finished @Robdel12's task in 1m 36s —— View job

---

Code Review Complete ✓

I've reviewed the PR changes and the implementation looks solid! This fix properly addresses the root cause by wiring authService and projectService into the TDD server.

Summary

Problem: TDD dashboard showed "Not signed in" even when authenticated because only configService was passed to createServerManager().

Solution: Created two new service wrapper modules (auth-service.js and project-service.js) and wired them into the TDD command alongside the existing configService.

Code Review Findings

✅ Strengths

1. Clean Service Architecture - Both new services follow the functional dependency injection pattern consistently used throughout the codebase
2. Excellent Documentation - Each service has comprehensive JSDoc comments explaining the interface contract and error handling behavior
3. Proper Error Handling - Services gracefully handle unauthenticated states (returning false or [] instead of throwing)
4. Testability - Both services support dependency injection for testing (injectable httpClient, tokenStore, mappingStore)
5. Comprehensive Test Coverage - 280 lines of auth service tests and 371 lines of project service tests covering all edge cases
6. HTTP Client Caching - Smart optimization in project-service.js:42-44 creating the HTTP client once at service creation rather than per-request
7. Consistent with Existing Code - Mirrors the pattern used in config-service.js

📝 Observations & Minor Suggestions

src/services/auth-service.js:

• Line 39: apiUrl is assigned but never reassigned - could use const instead of let for clarity
• Line 43: Same for httpClient - could be const
• Line 47: Same for tokenStore - could be const

src/services/project-service.js:

• Lines 40, 44, 56: Same let vs const opportunity for immutable bindings
• Line 62-77: The createOAuthClient() helper is well-designed but creates a new wrapper object on every call. This is fine since it's lightweight, but worth noting.

src/commands/tdd.js:

• Line 126-127: Services are created without passing any config/options (using defaults). This works but means these services will always use production API URL even in dev mode. Consider whether apiUrl should be passed from config:

  let authService = createAuthService({ apiUrl: config.apiUrl });
  let projectService = createProjectService({ apiUrl: config.apiUrl });

src/commands/run.js:

• Line 209-211: Good comment explaining why run command doesn't need these services (no interactive dashboard). This is correct architecture.

tests/services/auth-service.test.js & project-service.test.js:

• Excellent test coverage with proper mocking
• Tests verify both happy paths and error cases
• Good use of test helpers from test-helpers.js

tests/tui/cli.test.js:

• Just added documentation comment - good improvement for clarity

🎯 Architecture Assessment

The service layer approach is the right pattern here:

1. Separation of Concerns - HTTP routers depend on service interfaces, not direct implementations
2. Dependency Injection - Services are created in commands and passed to server manager
3. Testability - Services can be mocked in tests
4. Flexibility - Different commands can pass different service configurations

The alternative (accessing global config/auth directly in routers) would create tight coupling and make testing harder.

✅ Test Plan Verification

The manual test plan is appropriate:

• ✅ Authenticate via vizzly login
• ✅ Start TDD server
• ✅ Verify Projects tab shows signed-in state
• ✅ Run unit tests

The unit tests are thorough and cover:

• All public methods on both services
• Authenticated and unauthenticated states
• Error conditions
• HTTP client caching behavior

Recommendation

Approve with optional refinements. The PR solves the stated problem correctly. The only suggestion is considering whether to pass config.apiUrl to the service constructors in tdd.js:126-127 to respect the user's API URL configuration (especially for development/testing scenarios).

---