feat(login) - mock implementation

[kfirstri]

Summary

Implement basic login mechanism in the CLI, we don't have server endpoints yet so this really just mocks it

[github-actions]

🚀 Package Preview Available!

---

Install this PR's preview build with npm:

npm i @base44-preview/cli@0.0.1-pr.5.2dd0b1b

Prefer not to change any import paths? Install using npm alias so your code still imports base44:

npm i "base44@npm:@base44-preview/cli@0.0.1-pr.5.2dd0b1b"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "base44": "npm:@base44-preview/cli@0.0.1-pr.5.2dd0b1b"
  }
}

• 📦 Preview Package: @base44-preview/cli@0.0.1-pr.5.2dd0b1b
• 🔗 View this commit on GitHub

---

_{Preview published to npm registry — try new features instantly!}

[claude]

Pull Request Review: Mock Login Implementation

Summary

This PR implements a mock device code authentication flow for the CLI. The implementation demonstrates a good understanding of OAuth device flow patterns and includes proper structure with TypeScript path aliases, Zod validation, and error handling.

✅ Strengths

Code Organization

• Well-structured: Clear separation between CLI commands (src/cli/commands/auth/login.ts), API client logic (src/core/api/auth/client.ts), and schemas (src/core/api/auth/schema.ts)
• Good use of path aliases: The TypeScript path aliases (@api/*, @config/*, @core/*) make imports cleaner. The addition of tsc-alias properly handles these at build time
• Modular functions: Login logic is broken into focused functions (generateAndDisplayDeviceCode, waitForAuthentication, saveAuthData)

Type Safety & Validation

• Strong typing: Proper use of Zod schemas for runtime validation with DeviceCodeResponseSchema and TokenResponseSchema
• Custom error classes: Well-defined AuthApiError and AuthValidationError with proper typing

Error Handling

• Comprehensive error handling: Each function has proper try-catch blocks with specific error types
• Good error messages: User-friendly error messages throughout (e.g., "Authentication timed out. Please try again.")

⚠️ Issues & Concerns

1. Critical: Memory Leak in Mock Implementation (src/core/api/auth/client.ts:13-16)

The deviceCodeToTokenMap is a module-level Map that never cleans up expired entries. While it's a mock, this could cause issues in development:

const deviceCodeToTokenMap = new Map<
  string,
  { startTime: number; readyAfter: number }
>();

Issue: If a user starts the login flow but never completes it, the entry persists in memory forever. In a real server scenario, this would be a memory leak.

Recommendation: Add cleanup for expired entries or add a comment clarifying this is acceptable for the mock.

2. Bug: Race Condition in Token Retrieval (src/cli/commands/auth/login.ts:53-96)

The tokenResponse variable is set inside the pWaitFor callback but checked after:

let tokenResponse: TokenResponse | null = null;

try {
  await pWaitFor(async () => {
    const result = await getTokenFromDeviceCode(deviceCode);
    if (result !== null) {
      tokenResponse = result; // Set inside callback
      return true;
    }
    return false;
  }, { ... });
} catch (error) { ... }

if (!tokenResponse) { // Check after pWaitFor
  throw new Error("Failed to retrieve authentication token.");
}

Issue: The final check at line 94 is redundant and could only be reached if pWaitFor returns without throwing but also without setting tokenResponse, which shouldn't happen with the current logic. However, this pattern is fragile.

Recommendation: Either remove the redundant check or restructure to return the token directly from the pWaitFor callback.

3. Poor Error Handling in runCommand (src/cli/utils/runCommand.ts:19-21)

catch (e) {
  console.log("reporting error...");
  console.error(e);
}

Issues:

• Using console.log and console.error directly instead of the @clack/prompts API (should use log.error())
• The message "reporting error..." is confusing - it's not actually reporting anywhere
• Doesn't re-throw the error, which might hide failures
• Inconsistent with the rest of the codebase that uses @clack/prompts

Recommendation:

catch (e) {
  log.error(e instanceof Error ? e.message : String(e));
  process.exit(1);
}

4. Security: Hardcoded Mock Values (src/core/api/auth/client.ts:31-32)

userCode: "ABCD-1234",
verificationUrl: "https://app.base44.com/verify",

Issue: While this is a mock, hardcoded values should be clearly documented or configurable for testing different scenarios.

Recommendation: Add a comment explaining these are mock values, or use environment variables for flexibility during development.

5. Inconsistent Error Re-throwing (src/core/api/auth/client.ts:96-107)

catch (error) {
  if (error instanceof AuthValidationError) {
    throw error;
  }
  if (error instanceof AuthApiError) {
    throw error; // Why re-throw here?
  }
  throw new AuthApiError(...);
}

Issue: The code re-throws AuthApiError unchanged, then catches non-specific errors and wraps them in AuthApiError. This is redundant.

Recommendation: Remove the AuthApiError check:

catch (error) {
  if (error instanceof AuthValidationError || error instanceof AuthApiError) {
    throw error;
  }
  throw new AuthApiError(...);
}

6. Missing Type Safety (src/cli/commands/auth/login.ts:29-43)

Error handling uses multiple instanceof checks, but the error could be anything. The final catch-all is good, but the pattern is repetitive.

Recommendation: Consider a helper function for consistent error handling across auth commands.

📊 Test Coverage

Issue: No tests found for the new authentication logic.

Recommendation: Add unit tests for:

• generateDeviceCode() and getTokenFromDeviceCode() mock functions
• Error handling in login flow
• Timeout behavior in waitForAuthentication()
• Custom error classes

🔒 Security Considerations

• ✅ No hardcoded real credentials
• ✅ Proper token storage through writeAuth() (assuming it's secure)
• ⚠️ Mock implementation stores device codes in memory - document that this is for development only
• ✅ Timeout prevents indefinite waiting

🚀 Performance

• ✅ Proper use of async/await
• ✅ Reasonable polling interval (2000ms)
• ✅ Timeout prevents infinite loops
• ℹ️ The mock delays (1000ms in client.ts) are appropriate for simulation

📝 Suggestions for Improvement

Minor Issues:

1. Trailing newline inconsistency: login.ts:135 has a trailing newline while other auth files don't
2. Package.json formatting: The dependency order is maintained well
3. TypeScript config: Good addition of path aliases, but consider documenting them in README.md

Before Merging:

1. Fix the error handling in runCommand.ts to use @clack/prompts consistently
2. Add a comment in client.ts documenting that the Map is intentionally not cleaned up since this is a mock
3. Consider adding basic tests, even for mock functionality
4. Document the mock behavior and how to replace it with real API calls in the future

✨ Overall Assessment

This is a solid mock implementation that demonstrates good architectural patterns. The code is well-organized, type-safe, and shows understanding of OAuth device flows. The main concerns are around error handling consistency and the lack of tests.

Recommendation: Approve with minor changes requested (error handling in runCommand.ts and documentation of mock behavior).

---

Great work on the structure! Once the error handling is polished and some tests are added, this will be a strong foundation for the real authentication implementation.

[claude]

Pull Request Review: feat(login) - mock implementation

Overview

This PR implements a mock OAuth device code flow for the CLI login functionality. The implementation is well-structured and demonstrates good architectural decisions. However, there are several issues that should be addressed before merging.

---

🔴 Critical Issues

1. Invalid Email in Mock Data (src/core/api/auth/client.ts:79)

email: "sd.com",

Issue: This is not a valid email address and will fail Zod validation.

Fix: Use a valid email format:

email: "test@example.com",

2. Memory Leak in Production (src/core/api/auth/client.ts:13-16)

const deviceCodeToTokenMap = new Map<
  string,
  { startTime: number; readyAfter: number }
>();

Issue: Module-level Map that never cleans up expired entries. Failed/abandoned login attempts will accumulate in memory indefinitely.

Fix: Implement cleanup for expired entries or use a TTL-based cache library.

---

⚠️ High Priority Issues

3. Exit Code Not Set on runCommand Errors (src/cli/utils/runCommand.ts:11-21)

The error handler logs the error and calls process.exit(1), which is good. However, consider:

• The exit(1) behavior is correct for CLI error handling
• Ensure this is the desired behavior for all command failures

4. Missing Test Coverage

No tests were added for:

• The new auth API client functions
• Error handling paths
• The device code polling mechanism
• Token validation

Recommendation: Add unit tests, especially for the error handling logic and validation flows.

---

💡 Code Quality & Best Practices

5. Inconsistent Error Handling in client.ts

In getTokenFromDeviceCode (lines 97-99), you explicitly check for both error types:

if (error instanceof AuthValidationError || error instanceof AuthApiError) {
  throw error;
}

But in generateDeviceCode (lines 48-50), you only check for AuthValidationError. Consider standardizing this pattern.

6. Path Aliases Configuration

Good addition of TypeScript path aliases (@api/*, @config/*, etc.) and tsc-alias for runtime resolution. This improves code maintainability.

Note: Ensure all developers run npm install to get tsc-alias as it's now required for builds.

7. Hardcoded Mock Values

While appropriate for a mock implementation, consider:

• Adding a comment indicating this is temporary mock data
• Making the delay/timeout values configurable for testing

Example:

// Mock implementation - replace with actual API call
const mockResponse: DeviceCodeResponse = {
  deviceCode,
  userCode: "ABCD-1234", // TODO: Generate random code
  verificationUrl: "https://app.base44.com/verify",
  expiresIn: 600,
};

---

✅ Positive Aspects

1. Good Error Architecture: Custom error classes (AuthApiError, AuthValidationError) with proper typing
2. Proper Async/Await Usage: Clean async patterns throughout
3. Zod Validation: Runtime validation at API boundaries is excellent
4. User Experience: Good use of spinners and logging for user feedback
5. Separation of Concerns: Well-organized code structure with clear separation between CLI, core, and API layers
6. Type Safety: Strong typing with TypeScript and Zod inferred types

---

🔒 Security Considerations

8. Token Storage

The PR doesn't show changes to writeAuth() implementation. Ensure that:

• Tokens are stored securely (not in plain text if possible)
• File permissions are restricted (e.g., chmod 600)
• Token rotation/refresh is planned for the real implementation

9. Device Code Cleanup

The Map cleanup on line 94 is good, but expired codes should also be cleaned up to prevent timing attacks or abuse.

---

🎯 Performance Considerations

10. Polling Interval

The 2-second polling interval (src/cli/commands/auth/login.ts:76) is reasonable but:

• Consider implementing exponential backoff for production
• The 600-second timeout might be too long for CLI UX

11. Unnecessary Delays in Mock

The delay(1000) calls simulate network latency, which is good for testing UX, but add explicit comments that these should be removed when implementing real HTTP calls.

---

📝 Minor Issues

12. Trailing Newlines Removed: Lines removed from logout.ts:27 and whoami.ts:24 (minor style inconsistency)
13. Import Organization: Consider grouping imports (external packages, then internal modules)

---

🧪 Testing Recommendations

Before merging, add:

1. Unit tests for generateDeviceCode() and getTokenFromDeviceCode()
2. Test error handling paths
3. Test Zod validation failures
4. Integration test for the full login flow

---

📋 Checklist for Next Steps

• Fix invalid email in mock data (Critical)
• Implement Map cleanup for expired entries (Critical)
• Add unit tests for new auth functionality
• Add comments marking temporary mock implementation
• Consider token storage security
• Document the device code flow for future implementers

---

Summary

This is a solid foundation for the login feature with good architectural decisions. The critical issues are minor and easy to fix. Once the invalid email is corrected and memory management is addressed, this will be in good shape for a mock implementation.

Recommendation: Request changes for the critical issues, then approve.

[claude]

PR Review: feat(login) - mock implementation

Summary

This PR implements a basic device code OAuth flow for CLI authentication with a mock implementation. The code is well-structured with good separation of concerns, proper error handling, and type safety using Zod schemas.

Positive Highlights

• Good architecture: Clear separation between CLI layer (login.ts), API client (client.ts), and schemas
• Strong type safety: Proper use of Zod for runtime validation of API responses
• Error handling: Custom error classes (AuthApiError, AuthValidationError) with good error context
• User experience: Helpful spinner messages and clear user instructions
• Code organization: Well-structured functions with single responsibilities
• Path aliases: Good use of TypeScript path aliases (@api, @config, @core) for cleaner imports

Issues & Recommendations

1. Critical: Memory Leak in Mock Implementation

Location: src/core/api/auth/client.ts:13-16

The deviceCodeToTokenMap is a module-level Map that never cleans up failed/expired authentication attempts.

const deviceCodeToTokenMap = new Map<
  string,
  { startTime: number; readyAfter: number }
>();

Issue: If users start authentication but don't complete it, the entries remain in memory forever.

Recommendation: Add cleanup logic for expired entries or use a TTL-based cache. For the real implementation, this won't be needed as the state will be server-side.

---

2. Bug: Typo in Property Name

Location: src/core/api/auth/client.ts:73

if (elapsed < deviceInfo.readyAfter) {

The property name appears to be readyAfter but the error message from the diff suggests it might be truncated to readyAft in the actual file. This needs verification.

---

3. Code Quality: Duplicate Error Handling Logic

Location: src/cli/commands/auth/login.ts:31-43 and login.ts:66-74

The same error handling pattern is repeated in both generateAndDisplayDeviceCode() and waitForAuthentication():

if (error instanceof AuthValidationError) {
  const issues = error.issues.map((i) => i.message).join(", ");
  throw new Error(`Invalid response from server: ${issues}`);
}
if (error instanceof AuthApiError) {
  throw new Error(`API error: ${error.message}`);
}

Recommendation: Extract this into a utility function like handleAuthError() to follow DRY principles:

function handleAuthError(error: unknown, context: string): never {
  if (error instanceof AuthValidationError) {
    const issues = error.issues.map((i) => i.message).join(", ");
    throw new Error(`${context}: Invalid response from server: ${issues}`);
  }
  if (error instanceof AuthApiError) {
    throw new Error(`${context}: ${error.message}`);
  }
  throw new Error(
    `${context}: ${error instanceof Error ? error.message : String(error)}`
  );
}

---

4. Performance: Polling Interval Too Aggressive

Location: src/cli/commands/auth/login.ts:77

interval: 2000,  // 2 second polling

Issue: 2-second polling for device code flow is quite aggressive. OAuth 2.0 Device Flow spec (RFC 8628) recommends 5+ seconds.

Recommendation: Increase to 5000ms (5 seconds) to reduce unnecessary API calls and be more respectful of server resources.

---

5. Test Coverage

Location: N/A

Issue: No tests were added for this significant new feature. The mock implementation makes this very testable!

Recommendation: Add unit tests for:

• Schema validation (valid/invalid responses)
• Error handling paths
• Timeout scenarios
• Successful authentication flow
• Spinner state management

Example test structure:

describe('generateDeviceCode', () => {
  it('should return valid device code response', async () => {
    const response = await generateDeviceCode();
    expect(response).toHaveProperty('deviceCode');
    expect(response).toHaveProperty('userCode');
    expect(response.expiresIn).toBeGreaterThan(0);
  });

  it('should validate response schema', async () => {
    // Test with invalid mock data
  });
});

---

6. Security: Token Storage

Location: src/cli/commands/auth/login.ts:103-107

While writeAuth() implementation isn't in this PR, ensure tokens are stored securely:

• Use OS keychain/credential manager when possible
• File permissions should be 0600 (read/write for owner only)
• Consider encrypting tokens at rest

---

7. Minor: Inconsistent Spinner Stop Messages

Location: src/cli/commands/auth/login.ts

s.stop("Device code generated");  // Line 21
s.stop("Authentication completed!");  // Line 92 (with exclamation)
s.stop("Authentication failed");  // Line 82 (in error path)

Recommendation: Be consistent with punctuation and tone. Either always use exclamation marks for success or never.

---

8. Code Quality: Magic Numbers

Location: src/core/api/auth/client.ts:26,78

readyAfter: 5000,  // What does this represent?
token: "mock-token-" + Date.now(),  // Good for uniqueness but consider using crypto.randomUUID()

Recommendation: Extract magic numbers to named constants and use crypto.randomUUID() for better token generation:

const MOCK_AUTH_DELAY_MS = 5000;  // Simulates server processing time
const MOCK_DEVICE_CODE_EXPIRY_SECONDS = 600;

// And use crypto for tokens
token: `mock-token-${crypto.randomUUID()}`,

---

9. TypeScript: Consider Narrowing Error Types

Location: src/cli/commands/auth/login.ts:86

if (error instanceof Error) {
  throw error;
}

This catches all Error types but then re-throws them. Consider being more specific about which errors should propagate vs be handled.

---

10. Dependencies: Node Version Bump

Location: package.json:53

The Node.js requirement was bumped from >=18.0.0 to >=20.0.0.

Recommendation: Document WHY this change was necessary. Was it for p-wait-for compatibility? If not strictly required, consider keeping Node 18 support for wider compatibility.

---

Additional Observations

Good Practices Followed ✅

• Proper async/await usage
• No floating promises
• Good function naming
• TypeScript strict mode enabled
• ESM modules with proper .js extensions in imports

Build Configuration

The addition of tsc-alias to the build process is correct for resolving path aliases. Good catch!

---

Security Assessment

Overall: Low risk for a mock implementation, but some considerations for production:

• ✅ No obvious injection vulnerabilities
• ✅ Input validation with Zod schemas
• ⚠️ Ensure token storage is secure (mentioned above)
• ⚠️ In production, use HTTPS only (URLs should be validated)
• ⚠️ Add rate limiting when implementing real API calls

---

Performance Considerations

• Polling interval could be optimized (mentioned above)
• Mock delays (1000ms) are reasonable for simulation
• Memory leak issue needs addressing before production

---

Recommendations Priority

High Priority:

1. Fix the memory leak in deviceCodeToTokenMap
2. Add test coverage for critical paths
3. Verify/fix the readyAfter/readyAft typo
4. Increase polling interval to 5 seconds

Medium Priority:
5. Extract duplicate error handling logic
6. Add named constants for magic numbers
7. Document Node version requirement

Low Priority:
8. Consistent spinner message punctuation
9. Use crypto.randomUUID() for mock tokens

---

Verdict

This is solid foundational work for the authentication system! The architecture is clean, types are strong, and error handling is thorough. The main concerns are around test coverage and a few implementation details that should be addressed before this pattern is used in production.

Suggested Actions Before Merge:

1. Add basic unit tests
2. Fix memory leak or add TODO comment
3. Increase polling interval
4. Address the property name typo

Great job on the implementation! 🚀

[claude]

PR Review: Mock Login Implementation

Summary

This PR implements a mock device code OAuth flow for the CLI login functionality. The implementation follows the OAuth device authorization grant pattern commonly used by CLIs like GitHub CLI and Azure CLI.

Code Quality & Best Practices

✅ Strengths

1. Well-structured architecture: The separation between API client (src/core/api/auth/), CLI commands, and utilities follows good separation of concerns.

2. Type safety: Strong use of TypeScript with Zod schemas for runtime validation (DeviceCodeResponseSchema, TokenResponseSchema).

3. Error handling: Custom error classes (AuthApiError, AuthValidationError) provide clear error semantics and proper error propagation through the command handler in runCommand.ts:10-22.

4. Path aliases: Good use of TypeScript path aliases (@api/*, @config/*, @core/*) with tsc-alias for cleaner imports.

5. User experience: The runTask utility provides good visual feedback with spinners, and the device code flow clearly communicates steps to the user.

6. Code cleanup: Simplified logout.ts and whoami.ts by removing unnecessary try-catch blocks since error handling is centralized.

⚠️ Issues & Concerns

1. Memory Leak in Mock Implementation (High Priority)

Location: src/core/api/auth/client.ts:13-16

const deviceCodeToTokenMap = new Map<
  string,
  { startTime: number; readyAfter: number }
>();

Issue: The module-level Map will accumulate entries indefinitely if users don't complete authentication. Only successful token retrieval cleans up the entry (line 94).

Impact: In a real server implementation this would cause memory leaks. While this is mock code, it demonstrates a problematic pattern.

Recommendation:

• Add cleanup for expired device codes
• Document that this is mock-only behavior
• Consider using a timeout to clean up abandoned authentication attempts

2. Missing Test Coverage (High Priority)

Issue: No test files found for the new authentication logic.

Impact:

• No validation of the OAuth device flow logic
• Error handling paths are untested
• Schema validation edge cases are uncovered
• Refactoring risk is high

Recommendation: Add tests for:

• Device code generation and validation
• Token retrieval with various timing scenarios
• Error cases (validation failures, expired codes)
• Integration tests for the full login flow

3. Hardcoded Mock Values (Medium Priority)

Location: src/core/api/auth/client.ts:26,31-32,79-80

Issue: Mock values are hardcoded throughout the implementation:

• readyAfter: 5000 (line 26)
• userCode: "ABCD-1234" (line 31)
• verificationUrl: "https://app.base44.com/verify" (line 32)
• email: "stam@lala.com" (line 79)

Recommendation:

• Move mock configuration to a separate config file or environment variables
• Use environment-based feature flags to make it easier to swap in real API calls later
• Document which parts are mock vs. production-ready

4. Error Message Quality

Location: src/cli/utils/runCommand.ts:11-20

Issue: The error handling catches multiple error types but the messages could be more actionable:

} else {
  log.error(String(e));
}

Recommendation: Provide more context and suggestions for resolution, especially for validation errors.

5. Missing Input Validation

Location: src/cli/commands/auth/login.ts:59-61

Issue: getTokenFromDeviceCode is called with deviceCode but there's no validation that the device code is in the expected format before making the call.

Recommendation: Add basic input validation before the polling loop starts.

Security Concerns

⚠️ Moderate Issues

1. Token Storage: The PR doesn't show how tokens are stored in writeAuth(). Ensure tokens are:

   • Stored with appropriate file permissions (0600)
   • Not logged or exposed in error messages
   • Cleared from memory after use

2. Mock Verification URL: The URL https://app.base44.com/verify should be validated to ensure it's the correct production domain to prevent phishing when this transitions from mock to real implementation.

3. No Token Expiration: The mock TokenResponse doesn't include token expiration or refresh tokens. Consider adding these fields now for forward compatibility.

Performance Considerations

1. Polling Interval: The 2-second polling interval (src/cli/commands/auth/login.ts:53) is reasonable for device code flows. Consider making it configurable.

2. Artificial Delays: The mock implementation includes delay(1000) calls (lines 20, 63 in client.ts). These are fine for mocking but should be removed when implementing real API calls.

3. p-wait-for Timeout: The timeout is based on expiresIn * 1000 (600 seconds = 10 minutes), which is appropriate for device code flows.

Architecture & Design

✅ Good Patterns

1. Separation of Concerns: Login flow broken into logical functions:

   • generateAndDisplayDeviceCode()
   • waitForAuthentication()
   • saveAuthData()

2. Reusable Utilities: runTask is a nice abstraction that can be reused across other commands.

3. Error Boundaries: Centralized error handling in runCommand prevents error handling duplication.

💡 Suggestions

1. Extract Magic Numbers: Constants like polling interval (2000ms), readyAfter (5000ms), and expiration (600s) should be extracted as named constants.

2. Consider Dependency Injection: The auth client could accept configuration to make testing easier and support multiple environments.

Dependencies

New Dependencies Review

1. p-wait-for@6.0.0: ✅ Well-maintained package for polling patterns, appropriate choice
2. tsc-alias@1.8.16: ✅ Necessary for path alias resolution in compiled output

Node Version Bump

• Changed from >=18.0.0 to >=20.0.0: This is fine as Node 18 is nearing end-of-life, but ensure this is documented in any deployment guides.

Recommendations Summary

Must Fix Before Merge

1. ❗ Add test coverage for authentication flows
2. ❗ Document that this is mock implementation in code comments
3. ❗ Address memory leak in deviceCodeToTokenMap (at minimum, add a comment about cleanup strategy)

Should Fix Soon

1. Extract hardcoded mock values to configuration
2. Add input validation for device codes
3. Extract magic numbers to named constants
4. Verify token storage security in writeAuth()

Nice to Have

1. Add JSDoc comments to public API functions
2. Consider adding token expiration fields to TokenResponse schema
3. Add integration tests for the full authentication flow
4. Consider making polling interval configurable

Overall Assessment

This is a solid mock implementation that demonstrates good understanding of the OAuth device code flow. The code is well-structured with proper type safety and error handling. The main concerns are:

1. Lack of tests - this should be addressed before merge
2. Memory management in the mock implementation
3. Need for documentation distinguishing mock from production-ready code

The PR is functional and follows good patterns, but would benefit from the improvements listed above, particularly around testing and documentation.

Recommendation: Request changes for test coverage before merging. The implementation itself is good but needs validation through tests.