🦋 Changeset detected

Latest commit: acb1120

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
agents-manage-api	Ready	Preview	Comment	Oct 16, 2025 5:54am
agents-manage-ui	Ready	Preview	Comment	Oct 16, 2025 5:54am
agents-run-api	Ready	Preview	Comment	Oct 16, 2025 5:54am

1 Skipped Deployment

Project	Deployment	Preview	Comments	Updated (UTC)
agents-docs	Skipped			Oct 16, 2025 5:54am

Claude finished @sarah-inkeep's task —— View job

PR Review: Error Pages Improvements

Reviewing the error handling improvements across agents-manage-ui. This PR touches 19 files focusing on better error pages, styling, and message handling.

Review Progress:

• Read repository guidelines (CLAUDE.md)
• Analyze core error component changes (full-page-error.tsx)
• Review error.tsx pages for consistency
• Examine page.tsx modifications
• Check TypeScript types and error handling patterns
• Evaluate UX and customer impact
• Provide comprehensive feedback and recommendations

Summary

This PR significantly improves error handling across the agents-manage-ui application by centralizing error display logic, enhancing UX with better messaging and styling, and ensuring proper error propagation. The changes show thoughtful architecture improvements with consistent patterns.

✅ Strengths

1. Excellent Error Architecture

• Centralized error handling through the FullPageError component
• Consistent error boundaries across all routes with semantic context
• Smart error status code extraction from multiple sources (cause, message, direct property)

2. Superior User Experience

• Status code-specific messaging (404, 403, 401, 500, 503)
• Contextual error messages using the context prop
• Offline detection for better connection error handling
• Proper retry mechanisms with fallback options

3. Clean Component Design

• Well-structured props interface with good defaults
• Flexible retry handling (reset, onRetry, or window reload)
• Accessible design with proper semantic HTML and styling

🚨 Critical Issues

1. Type Safety Violations (full-page-error.tsx:38, 43)

// Current unsafe type casting
statusCode = (error.cause as any).status;
statusCode = (error as any).status;

Recommendation: Implement proper type guards:

function hasStatusInCause(error: Error): error is Error & { cause: { status: number } } {
  return typeof error.cause === 'object' && 
         error.cause !== null && 
         'status' in error.cause && 
         typeof error.cause.status === 'number';
}

function hasStatus(error: Error): error is Error & { status: number } {
  return 'status' in error && typeof (error as any).status === 'number';
}

// Then use:
if (hasStatusInCause(error)) {
  statusCode = error.cause.status;
} else if (hasStatus(error)) {
  statusCode = error.status;
}

2. Regex Duplication (full-page-error.tsx:39-40)

// Inefficient double regex execution
if (error.message.match(/^\[(\d{3})\]/)) {
  const match = error.message.match(/^\[(\d{3})\]/);
  statusCode = match ? Number.parseInt(match[1], 10) : undefined;
}

Recommendation:

const match = error.message.match(/^\[(\d{3})\]/);
if (match) {
  statusCode = Number.parseInt(match[1], 10);
}

⚠️ Architecture Considerations

1. Error Propagation Strategy
The PR successfully shifts from catching and overwriting errors to proper propagation. However, consider adding:

• Error Logging: Structured error logging for debugging
• Error Reporting: Integration with error monitoring (Sentry, LogRocket)
• Error Recovery: More sophisticated retry strategies for different error types

2. Accessibility & Internationalization

// Consider adding ARIA labels and i18n support
<h1 className="text-lg text-muted-foreground font-mono uppercase" 
    role="heading" 
    aria-level={1}>
  {title}
</h1>

3. Error Context Enhancement
The context prop is excellent but could be extended:

interface ErrorContext {
  resource: string;
  action?: 'loading' | 'saving' | 'deleting';
  metadata?: Record<string, string>;
}

🔧 Minor Improvements

1. Network Error Detection

// Current browser-only check
typeof navigator !== 'undefined' && !navigator.onLine

// Consider server-side network error patterns
const isNetworkError = error.message.includes('fetch') || 
                       error.message.includes('network') ||
                       error.code === 'NETWORK_ERROR';

2. Consistent Error Boundaries
All error.tsx files follow the same pattern, which is great. Consider extracting common patterns:

// Create a higher-order component for consistent error handling
function withErrorBoundary(Component, context, navigationPath) {
  return function ErrorBoundaryWrapper(props) {
    return (
      <ErrorBoundary>
        <Component {...props} />
      </ErrorBoundary>
    );
  };
}

📋 Testing Recommendations

Per CLAUDE.md requirements, this PR needs comprehensive tests:

// Missing: tests for the enhanced error component
describe('FullPageError', () => {
  it('should extract status code from error.cause', () => {
    const error = new Error('Test error');
    error.cause = { status: 404 };
    // Test status code extraction
  });
  
  it('should handle offline scenarios', () => {
    // Mock navigator.onLine = false
    // Test offline error display
  });
  
  it('should provide contextual error messages', () => {
    // Test different context values
  });
});

🎯 Customer Impact Assessment

Positive Impact:

• Much clearer error messages for users
• Better visual hierarchy with status codes
• Contextual navigation options
• Improved accessibility

Considerations:

• Ensure error messages don't expose sensitive information
• Consider user journey recovery paths
• Monitor error rates to validate improvements

📝 Additional Recommendations

1. Changeset Required
The changeset-bot indicates no changeset was added. This UI improvement should have a patch changeset:

# Create changeset
pnpm changeset add
# Select @inkeep/agents-manage-ui: patch

2. Error Analytics
Consider adding error tracking:

useEffect(() => {
  if (error && statusCode) {
    analytics.track('error_displayed', {
      statusCode,
      context,
      errorMessage: error.message,
      timestamp: new Date().toISOString()
    });
  }
}, [error, statusCode, context]);

🎉 Conclusion

This is a well-architected improvement to error handling that significantly enhances user experience. The centralized approach, consistent patterns, and thoughtful UX design are excellent. Address the type safety issues and consider the testing requirements, and this will be a solid enhancement to the platform.

The shift from catching/overwriting errors to proper propagation aligns well with the PR title and demonstrates good engineering judgment.

@claude please review again

Claude finished @sarah-inkeep's task —— View job

PR Review: Error Pages Improvements (Re-Review)

Reviewing the latest changes after your updates addressing my previous feedback...

Review Checklist:

• Check if previous critical issues were addressed
• Review type safety improvements
• Examine regex optimization
• Analyze any new changes since last review
• Verify error handling patterns
• Provide comprehensive feedback

🎉 Excellent Work! Previous Issues Resolved

I'm pleased to see that ALL critical issues from my previous review have been properly addressed:

✅ Type Safety Fixed (full-page-error.tsx)

Previous Issue: Unsafe type casting with (error.cause as any).status

Current Implementation: Proper type guard implemented!

function hasStatusCode(obj: unknown): obj is { status: number } {
  return (
    typeof obj === 'object' &&
    obj !== null &&
    'status' in obj &&
    typeof (obj as { status: unknown }).status === 'number'
  );
}

// Clean usage:
if (hasStatusCode(error.cause)) {
  statusCode = error.cause.status;
}

This is exactly the pattern I recommended. Well done! 👍

✅ Regex Optimization Fixed

Previous Issue: Double regex execution

// Old inefficient code:
if (error.message.match(/^\[(\d{3})\]/)) {
  const match = error.message.match(/^\[(\d{3})\]/);

Current Implementation: Single regex execution!

const match = error.message.match(/^\[(\d{3})\]/);
if (match) {
  statusCode = Number.parseInt(match[1], 10);
}

Perfect optimization!

📊 Significant Architectural Improvements Since Last Review

1. Error Handling Refactor (commit acb1120)

You've made a major architectural improvement by moving error handling from error boundary files (error.tsx) directly into the page components. This is brilliant because:

• Better control flow: Errors are caught exactly where they occur
• Contextual error messages: Each page can provide specific context (e.g., "agents", "credentials")
• Proper navigation links: Error pages maintain appropriate back links
• Reduced file count: Eliminated 13 redundant error.tsx files

Example of the new pattern:

// agents-manage-ui/src/app/[tenantId]/projects/[projectId]/agents/page.tsx
try {
  agents = await fetchAgents(tenantId, projectId);
} catch (error) {
  return <FullPageError error={error as Error} context="agents" />;
}

2. Global 404 Handler

Added a proper app-level not-found.tsx replacing the project-specific one:

// agents-manage-ui/src/app/not-found.tsx
export default function NotFound() {
  return (
    <FullPageError
     
      description="The page you are looking for does not exist or has been moved."
      statusCode={404}
      showRetry={false}
    />
  );
}

3. Structured Error Class

The ApiError class properly implements error cause chaining:

export class ApiError extends Error {
  constructor(error: ApiErrorData, status: number) {
    super(`[${status}] ${error.message}`, {
      cause: { code: error.code, status },
    });
    // ...
  }
}

🌟 Additional Improvements Observed

1. Enhanced Context Support: The context prop is now used consistently across all error displays, providing users with clear understanding of what resource failed.

2. Proper Error Propagation: As per the PR title, errors are now properly passed through instead of being caught and overwritten.

3. Clean Changeset: Added proper changeset file marking this as a patch update.

🔧 Minor Suggestions for Future Consideration

While the current implementation is solid, here are some ideas for future enhancements:

1. Error Telemetry: Consider adding error tracking for production monitoring:

// In full-page-error.tsx
useEffect(() => {
  if (error && statusCode && process.env.NODE_ENV === 'production') {
    // Track error occurrence for monitoring
    trackError({ statusCode, context, message: error.message });
  }
}, [error, statusCode, context]);

2. Internationalization Ready: The structure is already well-suited for i18n if needed later:

// Future enhancement possibility
const errorMessages = {
  404: t('errors.notFound'),
  403: t('errors.forbidden'),
  // ...
};

3. Retry Strategy Enhancement: Consider adding exponential backoff for retry attempts in the future.

✅ Conclusion

This is an exemplary refactoring that significantly improves the error handling architecture. The changes demonstrate:

• Thoughtful engineering: Moving from scattered error boundaries to centralized, contextual error handling
• Attention to feedback: All critical issues from the previous review were properly addressed
• User experience focus: Better error messages with appropriate context and navigation options
• Code maintainability: Reduced duplication and cleaner architecture

The PR successfully achieves its goal of improving error pages with better styling, messaging, and proper error propagation. The type safety improvements and optimizations show careful attention to code quality.

This PR is ready to merge! Great work on the improvements! 🚀

View job