chore: fix replay when self host#1102
Conversation
Wendong-Fan
left a comment
There was a problem hiding this comment.
Nice fix! The root cause analysis is solid and the multi-source resolution approach makes sense. Left a few inline comments on edge cases that could bite us in production.
| if (!fs.existsSync(filePath)) return undefined; | ||
| const content = fs.readFileSync(filePath, 'utf-8'); | ||
| const lines = content.split(/\r?\n/); | ||
| const line = lines.find((l) => l.trim() && !l.trim().startsWith('#') && l.startsWith(`${key}=`)); |
There was a problem hiding this comment.
Small inconsistency here: l.trim() is used for the empty/comment checks, but l.startsWith(...) runs on the raw (untrimmed) line. So if someone has a leading space like SERVER_URL=xxx in their .env, the trim checks pass but the key match fails silently.
Suggestion: store l.trim() in a variable and use it for all checks:
const line = lines.find((l) => {
const trimmed = l.trim();
return trimmed && !trimmed.startsWith('#') && trimmed.startsWith(`${key}=`);
});| const lines = content.split(/\r?\n/); | ||
| const line = lines.find((l) => l.trim() && !l.trim().startsWith('#') && l.startsWith(`${key}=`)); | ||
| if (!line) return undefined; | ||
| return line.slice(key.length + 1).trim(); |
There was a problem hiding this comment.
This will break if the .env value is quoted, e.g. SERVER_URL="https://my-server.com/api". The returned value would include the surrounding quotes, making the URL invalid.
Worth adding a strip for single/double quotes after the trim:
let value = line.trim().slice(key.length + 1).trim();
if ((value.startsWith('"') && value.endsWith('"')) ||
(value.startsWith("'") && value.endsWith("'"))) {
value = value.slice(1, -1);
}
return value;(Also note: since we changed to matching on trimmed, need line.trim().slice(...) here too.)
| if (!proxyUrl) return undefined; | ||
| const trimmed = proxyUrl.trim().replace(/\/+$/, ''); | ||
| if (!trimmed) return undefined; | ||
| return `${trimmed}/api`; |
There was a problem hiding this comment.
If someone already has /api in their proxy URL (e.g. VITE_PROXY_URL=http://localhost:8000/api), this would produce http://localhost:8000/api/api.
Quick guard before appending:
if (trimmed.endsWith('/api')) return trimmed;
return `${trimmed}/api`;|
|
||
| const devServerUrl = process.env.VITE_DEV_SERVER_URL; | ||
| if (!resolvedServerUrl && devServerUrl) { | ||
| const devEnvPath = path.join(process.cwd(), '.env.development'); |
There was a problem hiding this comment.
process.cwd() is unreliable in packaged Electron apps. It varies depending on how the user launched the app (Finder, terminal, Spotlight, etc.). Since this block only runs when VITE_DEV_SERVER_URL is set (i.e. during dev), app.getAppPath() would be a safer choice here and still point to the project root.
|
thanks! @Wendong-Fan |
🐞 Root Cause
The local Electron app always overwrote
SERVER_URLwhen spawning the backend,forcing it to: https://dev.eigent.ai/api
As a result:
✅ Fix
Updated the Electron backend launcher to:
SERVER_URLfrom~/.eigent/.envor existing environment variablesSERVER_URLis not providedAfter restarting the app and running a new task:
What is the purpose of this pull request?