Hosting

[hunterphillips]

No description provided.

[vercel]

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

Project	Deployment	Review	Updated (UTC)
feather-chat	Ready	Preview, Comment	Jan 8, 2026 2:47am

[claude]

Pull Request Review - PR #19: Hosting

Overview

This PR changes VITE_API_URL from http://localhost:3001 to /api/chat in client/.env.demo to prevent browser permission prompts for local network access in demo mode.

---

✅ Code Quality & Best Practices

Positive aspects:

• Clean, minimal change that solves a specific problem
• Properly scoped to demo environment only (.env.demo)
• Commit message is descriptive and explains the "why"

---

⚠️ Critical Issue Found

PROBLEM: Incorrect relative path will break multiple API endpoints

The change sets VITE_API_URL=/api/chat, but this is incomplete and will cause failures. Here's why:

Current Usage of VITE_API_URL in the codebase:

1. config-store.ts:4 - Used as base URL for multiple endpoints:

   • ${API_URL}/api/models → Would become /api/chat/api/models ❌
   • ${API_URL}/api/config → Would become /api/chat/api/config ❌
   • ${API_URL}/api/chats → Would become /api/chat/api/chats ❌
   • ${API_URL}/api/chats/${id} → Would become /api/chat/api/chats/${id} ❌
   • ${API_URL}/api/uploads → Would become /api/chat/api/uploads ❌

2. useChatSession.ts:27-38 - Constructs chat endpoint:

   const apiEndpoint = `${API_URL}/api/chat`;

   This would become /api/chat/api/chat ❌

The Fix

Change the value to an empty string or just /:

VITE_DEMO_MODE=true
VITE_API_URL=

This way:

• ${API_URL}/api/models → /api/models ✅
• ${API_URL}/api/chat → /api/chat ✅
• All other endpoints work correctly ✅

OR if you need an explicit value:

VITE_API_URL=/

Both approaches will make all API calls use relative paths from the root, which is what you want for same-origin deployment.

---

🔍 Additional Observations

Demo Mode Behavior

The code correctly checks for VITE_DEMO_MODE=true (useChatSession.ts:118-121) and bypasses real API calls entirely, using simulated responses from demo-stream.ts. However:

Question: If demo mode uses fake responses and doesn't actually call the API, why does VITE_API_URL matter at all in demo mode?

Looking at the code:

• Demo mode still calls fetchModels(), loadConfig(), loadChats(), etc. on mount
• These will fail silently in demo mode without a working API
• Consider adding demo mode checks to these initialization calls as well

Vite Configuration

client/vite.config.ts has no proxy configuration. For proper deployment:

• If deploying to Vercel as a static site, you'll need proper rewrites/redirects
• Consider adding a vercel.json rewrite rule to handle API routes if deploying backend separately
• Current vercel.json only specifies build config, not routing

Build Script

✅ The build:demo script correctly uses --mode demo which will load .env.demo

---

🔒 Security Considerations

✅ No security concerns - this is just environment configuration
✅ Moving to relative paths is actually better for security (no CORS needed)

---

🧪 Test Coverage

Recommendation: Test the demo deployment with these scenarios:

1. Verify chat interface loads without errors
2. Check browser console for failed API requests
3. Ensure demo responses work correctly
4. Test that no browser permission dialogs appear

---

📝 Recommendations

High Priority

1. Fix VITE_API_URL value - Change from /api/chat to empty string or /
2. Add demo mode guards - Prevent unnecessary API calls in demo mode:

   fetchModels: async () => {
     if (import.meta.env.VITE_DEMO_MODE === 'true') return;
     // ... existing code
   }

Medium Priority

3. Add Vercel routing config if deploying with backend - Add rewrites to vercel.json
4. Document deployment setup - Add section to README about demo vs production deployment

---

Summary

Status: ⚠️ Changes Required

The concept is correct, but the implementation has a critical bug. The path /api/chat should be changed to an empty string or / to serve as a proper base URL for all API endpoints, not just the chat endpoint.

Once fixed, this will properly resolve the localhost permission issue while maintaining all API functionality.