fix: server crash on startup from import.meta.url in CJS bundle#70
fix: server crash on startup from import.meta.url in CJS bundle#70jcanizalez merged 1 commit intomainfrom
Conversation
The server CJS bundle (tsup output) crashed on launch because import.meta.url is undefined in CJS format, causing fileURLToPath() to throw ERR_INVALID_ARG_TYPE. Removed the unused _serverDir variable and fileURLToPath import — the CJS global __dirname already works. Added CI step to catch this class of bug: greps for import_meta in the built bundle and smoke-tests require() to detect module-level crashes before they reach production.
There was a problem hiding this comment.
Pull request overview
Fixes a packaged-Electron server startup crash caused by import.meta.url being evaluated in a CJS bundle, and adds CI validation to prevent regressions in the server bundle output.
Changes:
- Remove
fileURLToPath(import.meta.url)-based__dirnameshim from the server entrypoint to avoid CJS runtime crashes. - Add a CI job step to build the server CJS bundle, scan it for
import_meta, and smoke-testrequire().
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/server/src/index.ts | Removes import.meta.url-derived path logic from server startup code. |
| .github/workflows/ci.yml | Adds a server bundle verification step (grep + require smoke test). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -15,8 +14,6 @@ import { setDataDir, getTaskImagePath as resolveTaskImagePath } from './task-ima | |||
| import { getTailscaleStatus } from './tailscale' | |||
| import log from './logger' | |||
|
|
|||
| const __dirname = path.dirname(fileURLToPath(import.meta.url)) | |||
|
|
|||
| export async function startServer( | |||
There was a problem hiding this comment.
Removing the ESM-friendly __dirname shim makes startServer() rely on the CJS global __dirname, but this package is type: "module" and dev mode runs tsx src/index.ts (ESM). In that environment __dirname is not defined, so path.resolve(__dirname, '../../web/dist') will throw at runtime and break dev startup. Consider computing a separate serverDir without import.meta (to keep the CJS bundle clean), e.g. derive from process.argv[1] when run as the entrypoint, and use that for the webDistDir resolution.
| if (e.code === 'ERR_INVALID_ARG_TYPE' || e.code === 'ERR_INVALID_URL') { | ||
| console.error('Server bundle crash:', e.message); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
The CI smoke-test currently swallows most synchronous require() failures: in the catch, it only exits non-zero for ERR_INVALID_ARG_TYPE/ERR_INVALID_URL, and other errors will be ignored (step exits 0 after 2s). This makes the step ineffective at catching other module-level crashes (e.g. missing native modules, syntax/runtime errors). If the intent is to ensure the bundle can be required safely, the catch should fail the step for any error from require(), while keeping the selective filtering (or a shorter timeout) only for truly async noise.
| if (e.code === 'ERR_INVALID_ARG_TYPE' || e.code === 'ERR_INVALID_URL') { | |
| console.error('Server bundle crash:', e.message); | |
| process.exit(1); | |
| } | |
| console.error('Server bundle failed to require:', e); | |
| process.exit(1); |
| yarn workspace @vibegrid/server build | ||
| # Fail if import.meta snuck into CJS bundle (causes runtime crash in Electron) | ||
| if grep -q 'import_meta' packages/server/dist/index.cjs; then | ||
| echo "ERROR: import_meta found in CJS bundle — will crash at runtime" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
grep -q 'import_meta' packages/server/dist/index.cjs will quietly succeed if the output file path changes or the file isn't produced (grep exit code 2 inside an if won't fail the step under set -e). Add an explicit existence check for packages/server/dist/index.cjs (or glob the produced CJS file) before grepping so this verification can't be skipped accidentally.
In ESM (dev/tsx), __dirname is not a global. The previous fix (#70) removed the import.meta.url fallback because tsup emits it as import_meta.url in the CJS bundle, crashing at runtime. This left __dirname undefined in dev mode. Fix: use typeof guard with process.argv[1] as the ESM fallback. Avoids import.meta.url entirely so the CJS bundle CI check passes.
* fix: session restore — persist sessions continuously and fix Recent Sessions resume (#74) Sessions were only saved during shutdown, but a race condition between bridge.close() and the server's deferred shutdown() meant sessions were often never written to the DB. On macOS, closing the window without quitting made this worse since terminals could exit before the eventual quit triggered a save. Fix by saving sessions on every state change (debounced) and on a 30s periodic interval, so shutdown is just a safety net. Also fix Recent Sessions resume: normalize paths for project lookup so the terminal gets the correct project name (was being filtered out by the visibility hook), and add error handling to prevent silent failures. - Add auto-save to SessionManager (debounced + periodic with dirty flag) - Wire scheduleSave() on session-created, session-exit, SessionStart hook - Add normalizePath() for symlink/trailing-slash resolution - Use normalized comparison in agent history and hook session linking - Extract resolveResumeSessionId() with multi-tier fallback - Extract resolveProjectName() for robust project matching - Add try/catch to all resume handlers * fix: address PR review — cache normalizePath, scoped fetch, cross-platform paths - resolveResumeSessionId: try scoped fetch first (avoids global limit=20 missing project sessions), then fall through to unscoped fallback - resolveProjectName: use normalized path for basename fallback so trailing-slash paths don't produce 'untitled' - Basename matching is now case-insensitive (comment was inaccurate) - startAutoSave: call stopAutoSave first to prevent timer leaks on re-init - persistNow: clear pending debounce timer to avoid redundant DB writes - normalizePath: use path.normalize for cross-platform separator handling - agent-history: cache normalizePath results per unique project path in the per-line loop to avoid repeated realpathSync syscalls - pty-manager: cache normalized session paths at creation time so findUnlinkedSessionByCwd avoids per-iteration realpathSync - Fix comment accuracy ("every state change" → actual trigger list) - Update tests for 2-call pattern in resolveResumeSessionId * fix: show project filter context in Recent Sessions and restore banner - RecentSessionsPopover: header shows "· ProjectName" or "· All Projects" - RecentSessionsCard: same treatment - SessionRestoredBanner: shows project name when single project, or "N projects" when sessions span multiple projects * fix: server crash in dev mode — __dirname undefined in ESM In ESM (dev/tsx), __dirname is not a global. The previous fix (#70) removed the import.meta.url fallback because tsup emits it as import_meta.url in the CJS bundle, crashing at runtime. This left __dirname undefined in dev mode. Fix: use typeof guard with process.argv[1] as the ESM fallback. Avoids import.meta.url entirely so the CJS bundle CI check passes. * fix: prevent multiple terminals from resuming the same session When restoring N sessions, resolveResumeSessionId used .find() which always returned the most-recent match — all terminals got the same session ID. Now accepts a `claimed` set that callers populate as each ID is assigned, so subsequent lookups skip already-taken sessions.
Summary
import.meta.urlisundefinedin CJS bundles (tsup output), causingfileURLToPath(undefined)to throwERR_INVALID_ARG_TYPEand crash the server on startup in the packaged Electron app_serverDirvariable andfileURLToPathimport — the CJS global__dirnamealready works correctly at the only usage site (line 94)import_metareferences, then smoke-testsrequire()to catch module-level crashesTest plan
yarn workspace @vibegrid/server build— noimport.metawarningsgrep import_meta packages/server/dist/index.cjs— zero matchesyarn build+yarn test— all passelectron-builder --mac --dir