Feature/feed discovery

[rulasfia]

Feed Discovery

[rulasfia]

/oc generate PR description

[opencode-agent]

Based on my analysis of the commits and files changed in this PR, here is the generated PR description:

---

Summary

This PR implements an RSS feed discovery feature that allows users to discover RSS feeds by entering a website URL. The feature integrates with the Feedsearch.dev API to automatically find available feeds on any given domain.

Changes

New Features

• Feed Discovery Component (apps/web/src/components/settings/feed-discovery.tsx): New UI component that provides a search interface for discovering RSS feeds from a website URL. Features include:

  • Manual search submission with loading states
  • Rate limit error handling with countdown display
  • Feed preview cards showing title, description, item count, and podcast indicators
  • Feed selection integration with the existing add feed flow

• Feed Discovery Server Function (apps/web/src/lib/server/feed-discovery-sfn.ts): Server-side function that:

  • Integrates with Feedsearch.dev API for feed discovery
  • Implements caching (5-minute TTL) to reduce API calls
  • Adds user-based rate limiting (10 requests per minute per user)
  • Includes comprehensive Sentry tracing for monitoring

• Rate Limiter Utility (apps/web/src/lib/rate-limiter.ts): New sliding window rate limiter implementation with:

  • Per-user request tracking
  • Automatic cleanup of expired entries
  • Configurable request limits and time windows

• Feed Discovery Query (apps/web/src/lib/queries/feed-discovery-query.ts): React Query integration for feed discovery with caching

Updates

• Add Feed Dialog (apps/web/src/components/settings/add-feed-dialog.tsx): Refactored to use tabs, adding a new "Discover Feed" tab alongside the existing manual URL entry
• Tabs Component (apps/web/src/components/ui/tabs.tsx): Minor styling improvements and code cleanup

Technical Details

• Uses feedsearch.dev API for feed discovery (configurable via FEEDSEARCH_API_URL env var)
• Rate limiting prevents API abuse (10 requests/minute per user)
• 5-minute cache for discovered feeds to improve performance
• Supports podcast detection and feed metadata display
• Full Sentry instrumentation for error tracking and performance monitoring

Testing

• Manual testing for feed discovery with various websites
• Rate limiting verified with repeated requests
• Error handling tested for invalid URLs and API failures

opencode session  |  github run

[github-actions]

Now I have all the context needed. Let me provide a comprehensive review:

Code Review: Feature/feed-discovery

Summary

This PR adds RSS feed discovery functionality using the Feedsearch.dev API, with rate limiting and caching mechanisms. Overall, the implementation is solid with good use of existing patterns in the codebase.

---

Issues Found

1. Bug: In-Memory Rate Limiter Won't Work in Production ⚠️ HIGH

File: apps/web/src/lib/rate-limiter.ts

The RateLimiter class uses an in-memory Map, which will not work in a serverless or multi-process environment. Each server request may hit a different process, making rate limiting ineffective.

// Current implementation - problematic for production
private requests: Map<string, RateLimitEntry> = new Map();

Recommendation: Consider using Redis or database-backed rate limiting for production, or add a warning comment documenting this limitation.

---

2. Bug: Cache Also In-Memory ⚠️ MEDIUM

File: apps/web/src/lib/server/feed-discovery-sfn.ts

Same issue as above - the SimpleCache is in-memory and won't persist across requests in a distributed environment.

const feedDiscoveryCache = new SimpleCache<DiscoveredFeed[]>(5 * 60 * 1000);

---

3. Security: User ID Exposure in Error Messages ⚠️ MEDIUM

File: apps/web/src/lib/server/feed-discovery-sfn.ts

When rate limiting is triggered, the error message includes user-specific timing information. While not critical, consider if this exposes any information about other users' activity.

---

4. Accessibility Issue ⚠️ LOW

File: apps/web/src/components/settings/feed-discovery.tsx

The favicon image lacks proper alt text handling:

<img src={feed.favicon} alt="" className="h-6 w-6 rounded" />

While alt="" is used for decorative images, consider if this should have meaningful alt text for screen readers.

---

5. Missing Error Handling for Network Failures ⚠️ MEDIUM

File: apps/web/src/lib/server/feed-discovery-sfn.ts

The ofetch call doesn't have specific handling for network timeouts vs API errors. The generic catch block might expose sensitive information in error messages.

---

Suggestions for Improvement

1. Add Input Sanitization

File: apps/web/src/lib/server/feed-discovery-sfn.ts

The normalizeUrl function should sanitize the input to prevent potential security issues:

function normalizeUrl(query: string): string {
  // Trim whitespace
  const trimmed = query.trim();
  
  // Remove any potentially dangerous characters
  const sanitized = trimmed.replace(/[\r\n\t]/g, '');
  
  if (sanitized.startsWith('http://') || sanitized.startsWith('https://')) {
    return sanitized;
  }
  return `https://${sanitized}`;
}

---

2. Add URL Validation

File: apps/web/src/lib/server/feed-discovery-sfn.ts

Consider adding URL format validation before calling the external API:

const urlSchema = z.string().url();
// Validate normalized URL before API call

---

3. Improve Type Safety

File: apps/web/src/lib/server/feed-discovery-sfn.ts

The FeedsearchResponse interface uses optional fields heavily. Consider making some fields required if the API always returns them:

interface FeedsearchResponse {
  url: string;  // This should always exist
  title?: string;
  // ...
}

---

4. Add JSDoc Comments

File: apps/web/src/lib/rate-limiter.ts

While the code has good inline comments, JSDoc would improve IDE support:

/**
 * Check if a request is allowed based on rate limits
 * @param key - Unique identifier for the rate limit (e.g., userId + operation)
 * @param maxRequests - Maximum requests allowed in the window
 * @param windowMs - Time window in milliseconds
 * @returns true if request is allowed, false if rate limit exceeded
 * @example
 * const allowed = rateLimiter.checkLimit('user123:api-call', 10, 60000);
 */

---

5. Consider Debouncing

File: apps/web/src/components/settings/feed-discovery.tsx

The search form has manual submission which is good, but consider if the input should have debouncing for a better UX if you later add auto-search.

---

6. Memory Leak Potential

File: apps/web/src/lib/rate-limiter.ts

The cleanup interval runs every 5 minutes but only cleans entries older than 1 minute. In high-traffic scenarios, this could still accumulate memory. Consider:

• Reducing cleanup interval
• Adding maximum entries limit
• Using WeakRef for entries

---

Positive Aspects

1. ✅ Good use of React Query - Proper use of query options, stale time, and caching
2. ✅ Sentry Integration - Excellent use of Sentry spans for monitoring
3. ✅ Type Safety - Good use of Zod for input validation and TypeScript interfaces
4. ✅ Error Handling - Graceful handling of rate limit errors with user-friendly messages
5. ✅ Component Structure - Clean separation of concerns between UI and logic
6. ✅ Rate Limiting UX - Good user experience showing remaining wait time
7. ✅ Attribution - Proper attribution to Feedsearch.dev API

---

Final Verdict

Category	Rating
Code Quality	⭐⭐⭐⭐ Good
Security	⭐⭐⭐ Moderate (in-memory rate limiting)
Performance	⭐⭐⭐⭐ Good
Maintainability	⭐⭐⭐⭐⭐ Excellent
Testing	⭐⭐⭐ Needs tests

Overall: Approve with minor suggestions

The code follows project conventions well and integrates cleanly with the existing architecture. The main concern is the in-memory rate limiter which should be documented or replaced with a persistent solution for production deployments.

opencode session  |  github run

[github-actions]

Code Review: Feature/feed-discovery

I've reviewed all the changes in this PR. Here's my comprehensive analysis:

---

Issues Found

1. Bug: In-Memory Rate Limiter Won't Work in Production ⚠️ HIGH

File: apps/web/src/lib/rate-limiter.ts

The RateLimiter class uses an in-memory Map, which will not work in a serverless or multi-process environment. Each server request may hit a different process, making rate limiting ineffective.

// Current implementation - problematic for production
private requests: Map<string, RateLimitEntry> = new Map();

Recommendation: Consider using Redis or database-backed rate limiting for production, or add a warning comment documenting this limitation. The current implementation includes a maxEntries limit which helps, but doesn't solve the multi-process issue.

---

2. Bug: Cache Also In-Memory ⚠️ MEDIUM

File: apps/web/src/lib/server/feed-discovery-sfn.ts

Same issue as above - the SimpleCache is in-memory and won't persist across requests in a distributed environment.

const feedDiscoveryCache = new SimpleCache<DiscoveredFeed[]>(5 * 60 * 1000);

Recommendation: Document this limitation or consider using Redis for distributed caching.

---

3. Potential Memory Leak in Rate Limiter ⚠️ MEDIUM

File: apps/web/src/lib/rate-limiter.ts

The cleanupExpired method uses a hardcoded maxWindowMs = 60 * 1000 (1 minute) for cleanup, but different operations might have different window sizes. If a rate limit uses a larger window (e.g., 5 minutes), entries could be cleaned up prematurely or accumulate:

private cleanupExpired(): void {
    const maxWindowMs = 60 * 1000; // Hardcoded 1 minute
    // ...
}

Recommendation: Track the actual window sizes used or accept a configurable max window.

---

4. Regex Error Message Parsing is Fragile ⚠️ LOW

File: apps/web/src/components/settings/feed-discovery.tsx

The error message parsing uses a regex that could break if the error message format changes:

const getRateLimitSeconds = (errorMessage: string): number | null => {
    const match = errorMessage.match(/wait (\d+) seconds/);
    return match ? parseInt(match[1]) : null;
};

Recommendation: Consider returning structured error data from the server function instead of parsing error messages.

---

5. Missing favicon handling could cause issues ⚠️ LOW

File: apps/web/src/components/settings/feed-discovery.tsx

The favicon is directly used in an img tag without any validation or fallback handling for loading failures:

<img src={feed.favicon} alt={`${feed.title} favicon`} className="h-6 w-6 rounded" />

Recommendation: Consider adding an onError handler to fall back to the GlobeIcon if the favicon fails to load.

---

6. Type Import/Export Inconsistency ⚠️ LOW

File: apps/web/src/lib/queries/feed-discovery-query.ts

Re-exporting the type from another file:

export type { DiscoveredFeed };

This is fine, but it's better to import directly from the source to avoid indirection.

---

Suggestions for Improvement

1. Add Redis/Distributed Rate Limiting Documentation

Document that the current rate limiter is suitable for single-instance deployments but needs Redis for multi-instance setups:

/**
 * Simple in-memory rate limiter using sliding window algorithm
 * NOTE: This is suitable for single-instance deployments only.
 * For multi-instance/production deployments, use Redis-backed rate limiting.
 */

---

2. Improve Error Handling for Network Failures

File: apps/web/src/lib/server/feed-discovery-sfn.ts

The error handling checks for specific error message substrings, which is fragile. Consider using error codes or custom error classes:

// Instead of:
if (error instanceof Error && error.message.includes('timeout')) {

// Consider:
if (error instanceof TimeoutError) {

---

3. Add URL Protocol Validation

File: apps/web/src/lib/server/feed-discovery-sfn.ts

The normalizeUrl function adds https:// prefix but doesn't validate that the resulting URL is valid. The Zod validation happens after normalization, which is good, but consider validating the domain part before making the API call.

---

4. Consider Adding Rate Limit Headers

File: apps/web/src/lib/server/feed-discovery-sfn.ts

Consider adding rate limit information to response headers so the client can show better UI feedback (e.g., remaining requests).

---

5. Add Tests

The PR description mentions manual testing but there are no automated tests for:

• Rate limiter functionality
• Feed discovery server function
• Feed discovery component

---

Positive Aspects

1. Good use of existing patterns - Follows the project's conventions for server functions, queries, and components
2. Proper Sentry integration - Good use of spans for monitoring and selective error capture
3. Type safety - Uses Zod for validation and proper TypeScript interfaces
4. Error handling - Graceful handling of rate limits with user-friendly messages
5. Memory leak prevention - Rate limiter has max entries limit and cleanup mechanisms
6. Clean component structure - Good separation between UI and logic
7. Attribution - Proper attribution to Feedsearch.dev API
8. Caching strategy - 5-minute TTL for discovered feeds is reasonable

---

Final Verdict

Category	Rating
Code Quality	⭐⭐⭐⭐ Good
Security	⭐⭐⭐⭐ Good (with caveats about in-memory rate limiting)
Performance	⭐⭐⭐⭐ Good
Maintainability	⭐⭐⭐⭐⭐ Excellent
Testing	⭐⭐ Needs automated tests

Overall: Approve with minor suggestions

The code follows project conventions well and integrates cleanly with the existing architecture. The main concern is the in-memory rate limiter which should be documented for production deployments. The implementation is otherwise solid with good error handling, caching, and user experience considerations.

Recommended Action Items

1. Add a comment documenting the in-memory rate limiter limitation for production
2. Consider adding onError handler for favicon images
3. Add automated tests for the rate limiter and feed discovery functionality
4. Consider using structured error responses instead of message parsing

opencode session  |  github run