Skip to content

fix: server crash on startup from import.meta.url in CJS bundle#70

Merged
jcanizalez merged 1 commit intomainfrom
fix/server-cjs-import-meta-crash
Mar 20, 2026
Merged

fix: server crash on startup from import.meta.url in CJS bundle#70
jcanizalez merged 1 commit intomainfrom
fix/server-cjs-import-meta-crash

Conversation

@jcanizalez
Copy link
Copy Markdown
Owner

Summary

  • Root cause: import.meta.url is undefined in CJS bundles (tsup output), causing fileURLToPath(undefined) to throw ERR_INVALID_ARG_TYPE and crash the server on startup in the packaged Electron app
  • Removed the unused _serverDir variable and fileURLToPath import — the CJS global __dirname already works correctly at the only usage site (line 94)
  • Added a CI step that builds the server bundle and verifies it: greps for import_meta references, then smoke-tests require() to catch module-level crashes

Test plan

  • yarn workspace @vibegrid/server build — no import.meta warnings
  • grep import_meta packages/server/dist/index.cjs — zero matches
  • yarn build + yarn test — all pass
  • Built unpacked app with electron-builder --mac --dir
  • Launched packaged app — server starts, bridge connects, no crash
  • CI smoke-test catches the old bug (verified with simulated broken code)

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.
Copilot AI review requested due to automatic review settings March 19, 2026 23:50
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 __dirname shim 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-test require().

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.

Comment on lines 1 to 17
@@ -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(
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines +60 to +63
if (e.code === 'ERR_INVALID_ARG_TYPE' || e.code === 'ERR_INVALID_URL') {
console.error('Server bundle crash:', e.message);
process.exit(1);
}
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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);

Copilot uses AI. Check for mistakes.
Comment on lines +44 to +49
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
Copy link

Copilot AI Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
@jcanizalez jcanizalez merged commit 60acc65 into main Mar 20, 2026
5 checks passed
@jcanizalez jcanizalez deleted the fix/server-cjs-import-meta-crash branch March 20, 2026 00:38
jcanizalez added a commit that referenced this pull request Mar 23, 2026
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.
jcanizalez added a commit that referenced this pull request Mar 23, 2026
* 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.
@jcanizalez jcanizalez mentioned this pull request Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants