Fix/windows startup crash#1739
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR aims to prevent a Windows startup crash by guarding a macOS-only Electron window API and adds a regression test around window focus behavior, but it also includes a broad set of additional changes across git utilities, dependency probing, hooks/notifications, account/auth, build/release tooling, and database migration history.
Changes:
- Guard macOS-only window button APIs to avoid Windows startup/focus crashes; add platform regression test.
- Switch production renderer loading to an
app://protocol handler and update preload path / window wiring. - Introduce multiple new core modules (git helpers, dependency manager, agent hooks, Forgejo integration, account service) and significant build/release + Drizzle migration changes (scope expansion vs PR description).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/core/git/impl/status-parser.test.ts | Adds StatusParser test coverage for porcelain parsing and limits |
| src/main/core/git/impl/git-utils.ts | Adds git diff/status parsing helpers and base-ref normalization |
| src/main/core/git/impl/git-repo-utils.ts | Adds repository-level git operations (clone/init/PR fetch) |
| src/main/core/git/impl/detectGitInfo.ts | Adds git repo detection and base ref computation |
| src/main/core/git/impl/cat-file-batch.ts | Adds persistent git cat-file --batch helper for blob reads |
| src/main/core/git/impl/cat-file-batch.test.ts | Adds integration tests for CatFileBatch against temp repos |
| src/main/core/git/git-watcher-service.ts | Adds .git watcher to emit workspace/ref change events |
| src/main/core/forgejo/forgejo-issue-provider.ts | Adds Forgejo issue listing/search provider |
| src/main/core/forgejo/forgejo-connection-service.ts | Adds Forgejo credential storage + client creation + error mapping |
| src/main/core/forgejo/controller.ts | Exposes Forgejo RPC controller endpoints |
| src/main/core/editor/editor-buffer-service.ts | Adds DB-backed editor buffer persistence and pruning |
| src/main/core/editor/controller.ts | Exposes editor buffer RPC controller endpoints |
| src/main/core/dependencies/types.ts | Adds dependency probing domain types |
| src/main/core/dependencies/registry.ts | Adds dependency descriptors for core + agent CLIs |
| src/main/core/dependencies/probe.ts | Adds command path resolution + version probing |
| src/main/core/dependencies/dependency-manager.ts | Adds dependency manager with probing + install support |
| src/main/core/dependencies/controller.ts | Exposes dependency manager RPC controller endpoints |
| src/main/core/conversations/utils.ts | Adds DB row → shared Conversation mapping |
| src/main/core/conversations/types.ts | Adds ConversationProvider interface and config type |
| src/main/core/conversations/renameConversation.ts | Adds conversation rename operation |
| src/main/core/conversations/impl/ssh-conversation.ts | Adds SSH-based conversation/session provider implementation |
| src/main/core/conversations/impl/agent-command.ts | Adds provider CLI command/args builder logic |
| src/main/core/conversations/getConversationsForTask.ts | Adds conversation listing by task operation |
| src/main/core/conversations/getConversations.ts | Adds conversation listing for non-archived tasks |
| src/main/core/conversations/deleteConversation.ts | Adds conversation deletion with session stop + telemetry |
| src/main/core/conversations/createConversation.ts | Adds conversation creation + session start + telemetry |
| src/main/core/conversations/controller.ts | Exposes conversation RPC controller endpoints |
| src/main/core/app/utils.ts | Adds app utilities (fonts, version resolution, install checks) |
| src/main/core/app/controller.ts | Exposes app RPC controller endpoints |
| src/main/core/agent-hooks/notification.ts | Adds OS notification logic + app focus detection |
| src/main/core/agent-hooks/hook-server.ts | Adds localhost hook server with token protection |
| src/main/core/agent-hooks/hook-config.ts | Adds hook config writer for Claude/Codex + gitignore entries |
| src/main/core/agent-hooks/event-enricher.ts | Adds hook payload normalization + event enrichment |
| src/main/core/agent-hooks/claude-trust-service.ts | Adds auto-trust Claude worktree config management |
| src/main/core/agent-hooks/classifiers/rovo.ts | Adds terminal output classifier for rovo |
| src/main/core/agent-hooks/classifiers/qwen.ts | Adds terminal output classifier for qwen |
| src/main/core/agent-hooks/classifiers/pi.ts | Adds terminal output classifier for pi |
| src/main/core/agent-hooks/classifiers/opencode.ts | Adds terminal output classifier for opencode |
| src/main/core/agent-hooks/classifiers/mistral.ts | Adds terminal output classifier for mistral |
| src/main/core/agent-hooks/classifiers/kiro.ts | Adds terminal output classifier for kiro |
| src/main/core/agent-hooks/classifiers/kimi.ts | Adds terminal output classifier for kimi |
| src/main/core/agent-hooks/classifiers/kilocode.ts | Adds terminal output classifier for kilocode |
| src/main/core/agent-hooks/classifiers/index.ts | Registers classifier factory mapping for providers |
| src/main/core/agent-hooks/classifiers/goose.ts | Adds terminal output classifier for goose |
| src/main/core/agent-hooks/classifiers/generic.ts | Adds generic fallback terminal output classifier |
| src/main/core/agent-hooks/classifiers/gemini.ts | Adds terminal output classifier for gemini |
| src/main/core/agent-hooks/classifiers/droid.ts | Adds terminal output classifier for droid |
| src/main/core/agent-hooks/classifiers/cursor.ts | Adds terminal output classifier for cursor |
| src/main/core/agent-hooks/classifiers/copilot.ts | Adds terminal output classifier for copilot |
| src/main/core/agent-hooks/classifiers/continue.ts | Adds terminal output classifier for continue |
| src/main/core/agent-hooks/classifiers/codebuff.ts | Adds terminal output classifier for codebuff |
| src/main/core/agent-hooks/classifiers/cline.ts | Adds terminal output classifier for cline |
| src/main/core/agent-hooks/classifiers/charm.ts | Adds terminal output classifier for charm |
| src/main/core/agent-hooks/classifiers/base.ts | Adds classifier base types + ANSI stripping + buffering |
| src/main/core/agent-hooks/classifiers/autohand.ts | Adds terminal output classifier for autohand |
| src/main/core/agent-hooks/classifiers/auggie.ts | Adds terminal output classifier for auggie |
| src/main/core/agent-hooks/classifiers/amp.ts | Adds terminal output classifier for amp |
| src/main/core/agent-hooks/classifier-wiring.ts | Wires classifier output into AgentEvents + idle detection + notifications |
| src/main/core/agent-hooks/agent-hook-service.ts | Adds top-level hook service to start server and emit events |
| src/main/core/account/services/emdash-account-service.ts | Adds account OAuth flow + session + provider token dispatch |
| src/main/core/account/services/credential-store.ts | Adds encrypted credential storage wrapper |
| src/main/core/account/provider-token-registry.ts | Adds registry for provider token handlers |
| src/main/core/account/provider-token-registry.test.ts | Adds tests for provider token registry behavior |
| src/main/core/account/controller.ts | Exposes account RPC controller endpoints |
| src/main/core/account/config.ts | Adds auth server config |
| src/main/config/github.config.ts | Removes GitHub OAuth device flow config file |
| src/main/app/window.ts | Updates main window creation; adds macOS API guard and protocol-based loading |
| src/main/app/window.test.ts | Adds regression tests for macOS-only API guard on Windows |
| src/main/app/staticServer.ts | Removes embedded static HTTP server for production renderer |
| src/main/app/protocol.ts | Adds app:// protocol handler for renderer asset loading |
| src/main/app/menu.ts | Refactors menu clicks to emit typed main-process events |
| src/main/app/lifecycle.ts | Removes legacy lifecycle module |
| scripts/release/verify-win.ts | Adds Windows signature verification script |
| scripts/release/verify-mac.ts | Adds macOS codesign/arch verification script |
| scripts/release/upload-r2.ts | Adds R2 artifact upload script |
| scripts/release/tsconfig.json | Adds TS config for release scripts |
| scripts/release/rebuild-native.ts | Adds native-module rebuild helper for release builds |
| scripts/release/notarize-mac.ts | Adds notarization + stapling automation |
| scripts/release/lib/log.ts | Adds CI-friendly logging helpers |
| scripts/release/lib/exec.ts | Adds exec helper with better errors |
| scripts/release/lib/config.ts | Adds shared release config sourced from app identity |
| scripts/release/lib/artifacts.ts | Adds artifact discovery helpers |
| scripts/release/inject-telemetry.ts | Adds build-time PostHog config injection script |
| scripts/release/build.ts | Adds scripted electron-builder build wrapper |
| scripts/postinstall.ts | Replaces CJS postinstall with ESM/TS version for electron-rebuild |
| scripts/postinstall.cjs | Removes legacy postinstall script |
| scripts/emdash-run.ts | Removes emdash-run dev CLI script |
| scripts/copy-main-assets.cjs | Removes legacy asset copy script |
| scripts/clean.cjs | Removes legacy clean script |
| postcss.config.js | Removes PostCSS config file |
| pnpm-workspace.yaml | Updates onlyBuiltDependencies list |
| electron.vite.config.ts | Adds electron-vite config with aliases and renderer plugins |
| electron-builder.config.ts | Adds electron-builder packaging configuration |
| drizzle/meta/_journal.json | Rewrites Drizzle migration journal entries |
| drizzle/0009_add_ssh_support.sql | Removes prior migration |
| drizzle/0008_add_archived_at_to_tasks.sql | Removes prior migration |
| drizzle/0007_add_is_main_to_conversations.sql | Removes prior migration |
| drizzle/0006_add_multi_chat_support.sql | Removes prior migration |
| drizzle/0005_add_use_worktree_to_tasks.sql | Removes prior migration |
| drizzle/0004_add_line_comments.sql | Removes prior migration |
| drizzle/0003_add_run_config_status_to_projects.sql | Removes prior migration |
| drizzle/0002_lyrical_impossible_man.sql | Removes prior migration |
| drizzle/0001_skinny_robin_chapel.sql | Adds new migration creating app_secrets |
| drizzle/0001_add_base_ref_to_projects.sql | Removes prior migration |
| drizzle/0000_initial.sql | Removes prior initial schema migration |
| drizzle.config.ts | Refactors drizzle DB URL resolution to app helpers |
| docs/lib/source.ts | Reorders imports |
| docs/lib/layout.shared.tsx | Reorders imports |
| docs/content/docs/tmux-sessions.mdx | Updates tmux docs to apply to all terminal sessions |
| docs/content/docs/contributing.mdx | Updates command names and typecheck script |
| docs/app/api/search/route.ts | Reorders imports |
| docs/app/[[...slug]]/page.tsx | Reorders imports |
| docs/app/[[...slug]]/layout.tsx | Reorders imports |
| docker-ssh/entrypoint.sh | Adds SSH dev container entrypoint env injection |
| docker-ssh/dockerfile | Adds SSH dev container Dockerfile |
| docker-compose.yaml | Adds compose setup for SSH dev container |
| dev-app-update.yml | Adds dev auto-update config |
| components.json | Updates shadcn/ui config and aliases |
| agents/workflows/worktrees.md | Adds agent-facing workflow docs |
| agents/workflows/testing.md | Adds test/validation workflow docs |
| agents/workflows/remote-development.md | Adds remote development workflow docs |
| agents/risky-areas/updater.md | Adds "risky area" notes for updater/packaging |
| agents/risky-areas/ssh.md | Adds "risky area" notes for SSH |
| agents/risky-areas/pty.md | Adds "risky area" notes for PTY |
| agents/risky-areas/database.md | Adds "risky area" notes for database |
| agents/quickstart.md | Adds agent quickstart |
| agents/integrations/providers.md | Adds agent provider integration docs |
| agents/integrations/mcp.md | Adds MCP integration guidance |
| agents/conventions/typescript.md | Adds TypeScript/React conventions |
| agents/conventions/renderer-patterns.md | Adds renderer patterns guidance |
| agents/conventions/main-patterns.md | Adds main process patterns guidance |
| agents/conventions/ipc.md | Adds IPC/RPC conventions guidance |
| agents/conventions/config-files.md | Adds repo config file rules |
| agents/architecture/shared.md | Adds shared modules architecture doc |
| agents/architecture/renderer.md | Adds renderer architecture doc |
| agents/architecture/overview.md | Adds overall architecture doc |
| agents/architecture/main-process.md | Adds main-process architecture doc |
| agents/README.md | Adds index for agent docs |
| README.md | Updates native-module crash FAQ text |
| CONTRIBUTING.md | Updates Node version guidance and typecheck script name |
| .prettierrc | Adds import sorting plugin and Tailwind stylesheet path |
| .prettierignore | Ignores out/ build output |
| .nvmrc | Updates Node version |
| .github/workflows/windows-beta-build.yml | Updates Node version and native rebuild module list |
| .github/workflows/code-consistency-check.yml | Updates Node version and typecheck script name |
| .eslintrc.json | Removes overrides and adds ignorePatterns |
| .codex/config.toml | Adds Codex notify hook config |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mainWindow.on('focus', () => { | ||
| capture('app_window_focused'); | ||
| mainWindow?.setWindowButtonVisibility(true); | ||
| if (process.platform === 'darwin') { | ||
| mainWindow?.setWindowButtonVisibility(true); | ||
| } | ||
| checkAndReportDailyActiveUser(); | ||
| }); |
There was a problem hiding this comment.
The PR description says this change is only a Windows startup-crash guard + regression test, but the diff includes large cross-cutting additions/removals (protocol handler, Drizzle migration history rewrite, new account/dependencies/Forgejo/agent-hooks modules, release tooling, etc.). This makes review and risk assessment difficult; please either split into focused PRs or update the PR title/description to accurately reflect the scope and user impact (especially around DB migrations + packaging).
There was a problem hiding this comment.
This rewrites the Drizzle migration journal (and the PR also removes multiple numbered migrations). Changing drizzle/meta/_journal.json and deleting existing migrations will break upgrades for any existing user DBs, and can make drizzle-kit generate/validate an incompatible schema history. Mandatory: revert the journal + existing numbered migrations to be append-only, and generate new migrations instead of rewriting history (or handle this as an explicit 'DB reset' / breaking change with a coordinated migration path).
There was a problem hiding this comment.
initializeNewProject() is validating and writing relative to '.' rather than localPath. As written, it can (1) succeed even when localPath doesn't exist, and (2) write README.md into the wrong directory, then run git commands with cwd: localPath, causing git add README.md to fail. Fix by checking existence of localPath (not '.') and writing the README into localPath (or using an fs provider API that supports a cwd/base path).
There was a problem hiding this comment.
resolveCommandPath() can return an empty string when stdout is empty (because '' ?? null yields ''). Downstream code treats any truthy path as success, but an empty string is neither a valid path nor null. Change the return to explicitly return null when firstLine is falsy/empty.
| return firstLine ? firstLine : null; |
There was a problem hiding this comment.
This uses crypto.randomUUID() without importing crypto (and TypeScript/node typings won't necessarily have a crypto global in this compilation target). This is likely a runtime ReferenceError and/or a typecheck failure. Import randomUUID from node:crypto (or import the module) and use that instead.
There was a problem hiding this comment.
JSON.parse(row.config) can throw if the DB contains malformed JSON (corruption, older versions, manual edits), which would crash conversation listing. Wrap the parse in a try/catch (or validate shape) and default autoApprove to undefined on failure. Also, these imports appear to be used as types; switching to import type avoids carrying runtime dependencies into the main bundle.
There was a problem hiding this comment.
Returning command: cli! relies on a non-null assertion and will fail later with a less-actionable error if the CLI is not configured. Prefer an explicit guard that throws a clear error (or falls back to the provider registry default CLI) so configuration issues are surfaced deterministically and with context.
There was a problem hiding this comment.
Returning command: cli! relies on a non-null assertion and will fail later with a less-actionable error if the CLI is not configured. Prefer an explicit guard that throws a clear error (or falls back to the provider registry default CLI) so configuration issues are surfaced deterministically and with context.
| if (!cli) { | |
| throw new Error(`CLI is not configured for provider "${providerId}".`); | |
| } | |
| return { command: cli, args }; |
|
Most of the Copilot comments above are because I initially opened against the wrong branch instead of current main. |
Greptile SummaryThis PR wraps the macOS-only Confidence Score: 5/5Safe to merge — one-line guard with a matching regression test, no logic changes elsewhere. The fix is minimal and directly addresses the crash: No files require special attention.
|
| Filename | Overview |
|---|---|
| src/main/app/window.ts | Adds process.platform === 'darwin' guard around setWindowButtonVisibility in the focus handler — minimal, correct fix for the Windows crash. |
| src/main/app/window.test.ts | New test file verifying that setWindowButtonVisibility is skipped on Windows and called on macOS; uses vi.resetModules() + dynamic import pattern to exercise per-platform code paths. |
Sequence Diagram
sequenceDiagram
participant E as Electron Runtime
participant W as window.ts (focus handler)
participant P as process.platform
E->>W: 'focus' event fires
W->>W: capture('app_window_focused')
W->>P: check process.platform
alt platform === 'darwin'
P-->>W: 'darwin'
W->>E: mainWindow.setWindowButtonVisibility(true)
else platform !== 'darwin' (e.g. win32, linux)
P-->>W: 'win32' / 'linux'
W->>W: skip setWindowButtonVisibility (guard prevents crash)
end
W->>W: checkAndReportDailyActiveUser()
Reviews (1): Last reviewed commit: "fix(window): guard macOS-only window but..." | Re-trigger Greptile
|
Thanks for opening this PR! @uexoo This fix was supposed for the Emdash v1 beta we just launched https://github.com/Davidknp/emdash/tree/v1 |
Summary
Guard a macOS-only Electron window API so Emdash no longer crashes on startup on Windows. This keeps the existing macOS behavior and adds a regression test for both platforms. Test may be overkill.
Fixes
Fixes #1738
Snapshot
N/A
Type of change
Mandatory Tasks
Checklist