fix: app name resolution

[solimander]

Summary

Fixes app name resolution to check remote profiles instead of only the legacy local registry (which only contained apps created before profile names were added). Adds 24-hour caching to speed up commands and prevent rate limiting.

Changes

• AppResolver checks remote profiles first, falls back to local registry
• 24-hour disk cache for profile names in GlobalConfig
• Cache invalidated on app deploy and app profile set
• Prevents parallel API requests from triggering rate limits

Impact

• App name resolution now works for apps with remote profiles
• Subsequent commands are instant after initial cache
• No more "app not found" errors for apps with profiles

[claude]

Claude finished @solimander's task —— View job

---

PR Review Complete ✅

I've thoroughly analyzed this PR that centralizes app name resolution with AppResolver and profile caching. Here's my comprehensive review:

🌟 Overall Assessment: Excellent Implementation

This is a well-architected solution that addresses a real usability issue. The code quality is high, with good separation of concerns and proper error handling.

✅ Strengths

Architecture & Design

• Clean separation: New AppResolver centralizes all app name resolution logic
• Proper caching: 24-hour TTL with smart cache invalidation on deploy/profile changes
• Interface adoption: Added iface.AppNameResolver interface for loose coupling
• Fallback strategy: Remote profiles first, then local registry (legacy support)

Performance Improvements

• Batch processing: API calls use batching with parallel execution in getProfileNamesForApps
• Single resolver instance: Commands reuse the same resolver to avoid duplicate API calls
• Disk-based cache: Prevents repeated network calls within 24-hour window

Code Quality

• Comprehensive error handling: Graceful degradation when API calls fail
• Good documentation: Clear function comments explaining behavior
• Case-insensitive matching: Improves user experience for name resolution
• Clean refactoring: Removed old name.go and consolidated logic appropriately

⚠️ Areas for Improvement

Minor Issues

1. Thread Safety (pkg/commands/utils/app_resolver.go:193):

   // Potential race condition if multiple goroutines call ensureCacheInitialized
   if r.cacheInitialized {
       return nil
   }

   Recommendation: Add a mutex or use sync.Once for initialization.

2. Error Handling (pkg/commands/utils/app_resolver.go:196):

   _ = common.SaveProfileCache(r.environmentName, r.profileNames)

   Recommendation: Log cache save failures for debugging.

3. Magic Numbers (pkg/commands/utils/app_resolver.go:121):

   for i := 2; i <= 100; i++ {

   Recommendation: Define MaxNameRetries = 100 as a constant.

4. Cache Key Collision (pkg/common/global_config.go:203):
   The cache uses environment name as key, but if two environments have the same name from different sources, they could collide.

Performance Considerations

1. API Request Batching: Current batch size appears to be MaxAppsPerRequest but this constant isn't visible in the files reviewed. Ensure it's appropriately sized.

2. Goroutine Leak Prevention: The parallel API calls in getProfileNamesForApps look correct, but consider adding context cancellation for long-running operations.

🔒 Security Assessment

• ✅ No security vulnerabilities identified
• ✅ Proper input validation for app IDs and names
• ✅ No sensitive data exposure in caching
• ✅ Safe string operations with proper trimming and validation

🧪 Test Coverage Concern

Missing test coverage for the new AppResolver functionality. Given the complexity and importance of this component, I recommend adding:

// Suggested test cases:
- TestAppResolver_ResolveAppID_ValidHexAddress
- TestAppResolver_ResolveAppID_ProfileNameMatch  
- TestAppResolver_ResolveAppID_LocalRegistryFallback
- TestAppResolver_GetAppName_CacheHit
- TestAppResolver_FindAvailableName_Incremental
- TestAppResolver_CacheInitialization_ThreadSafety

📋 Code Quality Checklist

Todo List

• Read CLAUDE.md for repo guidelines
• Analyze the new AppResolver implementation
• Review caching mechanism in GlobalConfig
• Check removed name.go functionality
• Review modified command files for integration
• Assess code quality and best practices
• Check for potential bugs or issues
• Evaluate performance considerations
• Review security concerns
• Assess test coverage

🎯 Recommendations

1. Add thread safety to AppResolver initialization
2. Add comprehensive tests for the new resolver functionality
3. Consider adding metrics to monitor cache hit rates
4. Document the 24-hour TTL decision - consider making it configurable

🚀 Impact Assessment

This PR successfully addresses the original problem:

• ✅ App name resolution now works for apps with remote profiles
• ✅ Commands are fast after initial cache load
• ✅ No more "app not found" errors for profile-enabled apps
• ✅ Maintains backward compatibility with legacy local registry

Recommendation: APPROVE - This is a solid implementation that significantly improves user experience while maintaining good code quality and performance.

---