feat(desktop): Electron shell, build pipeline, and renderer skeleton#1012
feat(desktop): Electron shell, build pipeline, and renderer skeleton#1012emsanakhchivan wants to merge 65 commits intoGitlawb:mainfrom
Conversation
Adds standalone SDK building blocks with no SDK source dependencies: - sdk.d.ts: ambient type declarations for SDK bundle - coreSchemas.ts + coreTypes.generated.ts: Zod schemas and generated types - errors.ts: SDK-specific error classes - validation.ts: input validation utilities - messageFilters.ts: extracted message filter logic - handlePromptSubmit.ts: imports from messageFilters - 16 generated-types tests
…signature Code review finding: assertFunction used `asserts value is Function` which accepts any function-like value without narrowing. Changed to `(...args: any[]) => any` for better type safety.
Reviewer noted the header said "Generated from index.ts" but no generator produces this file. Updated to "Manually maintained — keep in sync with index.ts". Drift detection added in validate-externals.ts (PR 3).
Tighten SDK public type contract to resolve reviewer blockers: - PermissionResult: unknown[] → precise 6-shape discriminated union (addRules/replaceRules/removeRules/setMode/addDirectories/removeDirectories) - SDKSessionInfo: snake_case → camelCase (sessionId, lastModified, etc.) - ForkSessionResult: session_id → sessionId - SDKPermissionRequestMessage: uuid + session_id now required - SDKPermissionTimeoutMessage: added uuid + session_id - SessionMessage: parent_uuid → parentUuid - SDKMessage/SDKUserMessage/SDKResultMessage: replaced loose inline definitions with re-exports from coreTypes.generated.ts
Modifies core modules for SDK integration: - QueryEngine, tools, state, commands: SDK type hooks - SDK shared utilities (shared.ts, permissions.ts) - 21 SDK tests (shared-utils, permissions) Stack: main ← pr1-foundation ← pr2-sdk-core
casing.ts provides recursive key transformation for the SDK boundary layer. Internal runtime uses snake_case; public API exposes camelCase. Will be used by shared.ts, sessions.ts, query.ts at export boundaries.
Covers snakeToCamel, camelToSnake, mapKeysToCamel, mapKeysToSnake including nested objects, arrays, null/undefined, and round-trips.
Completes the SDK implementation: - SDK build target (dist/sdk.mjs) with TUI dependency stubbing - External dependency lists (scripts/externals.ts) - SDK type generation from Zod schemas (scripts/generate-sdk-types.ts) - External validation (scripts/validate-externals.ts) - SDK source: index, query, v2, sessions modules - agentSdkTypes: re-exports SDK functions (query, createSession, etc.) - 136 SDK tests + 7 build scanner tests Stack: main ← pr1-foundation ← pr2-sdk-core ← pr3-sdk-runtime
shared.ts: SDKSessionInfo, ForkSessionResult, SessionMessage fields now use camelCase matching sdk.d.ts. SDKPermissionRequestMessage and SDKPermissionTimeoutMessage gain required uuid + session_id fields. permissions.ts: onPermissionRequest/onTimeout callbacks now include uuid and session_id in emitted messages.
sessions.ts: toSDKSessionInfo outputs camelCase keys, entryToSessionMessage uses parentUuid, forkSession returns sessionId. query.ts: reads sessionId from listSessions/forkSession results instead of snake_case session_id.
session_id → sessionId in forkSession result assertions and getSessionMessages calls.
…solve wrapper Add createOnceOnlyResolve utility to prevent double-resolution of promises when timeout and host response happen simultaneously. This ensures deterministic behavior in the permission handling flow.
Changes: - Use _+([a-z]) regex to match multiple consecutive underscores before letters - Add lookahead (?=. ) to preserve underscore-letter pairs at string end - Handle dunder names (__proto__, __typename) by stripping wrapper and capitalizing - Add tests for consecutive underscores and trailing underscore preservation
When a canUseTool callback throws an error, the catch block now includes the original error message in the denial message, making debugging easier for SDK consumers.
Add timeout parameter to acquireEnvMutex() to prevent infinite waits in deadlock scenarios. The timeout is optional and defaults to no timeout (wait forever) for backward compatibility. Returns a MutexAcquireResult object with acquired status and optional timeout reason for failed acquisitions.
Add tests for timeout scenarios when host doesn't respond to permission requests, fallback behavior when no onPermissionRequest callback, and MCP connection edge cases for undefined/empty config.
…rror handling - Add createPermissionTarget() factory that applies onceOnlyResolve at registration time, fixing race condition where timeout and host response could both try to resolve the same promise - Add try-catch to releaseEnvMutex() to prevent permanent lock if callback throws - Extract DEFAULT_PERMISSION_TIMEOUT_MS constant (30 seconds) - Add MCP config validation rejecting null, non-objects, and arrays - Preserve error stack traces in MCP connection failures - Add runtime validation to mapMessageToSDK for null/non-object/invalid type - Update tests to use createPermissionTarget and add validation tests
…nter, ripgrep) Resolve conflicts: - package.json: accept main's v0.7.0 + updated description - build.ts: accept main's expanded feature flags; keep CLI_EXTERNALS import - externals.ts: add @vscode/ripgrep from main to external list - agentSdkTypes.ts: keep PR3's real type definitions over main's any stubs
Resolve 5 conflict files — all taking main's side: - src/QueryEngine.ts: selective tool schema cache invalidation - src/bootstrap/state.ts: parentSessionId field, AsyncLocalStorage JSDoc - src/entrypoints/sdk/permissions.ts: SDKLogger injection, NO_SESSION_PLACEHOLDER, race condition fix (register before emit), try-catch host callback - src/entrypoints/sdk/shared.ts: snake_case naming, enriched JSDoc, timeout cleanup - tests/sdk/permissions.test.ts: regression tests for PR2 bug fixes
- Remove extra closing parenthesis in permissions.ts - Remove extra closing braces in shared.ts type definitions - Wrap MCP connection in try/catch to continue without MCP tools on failure
- Remove extra closing parenthesis in permissions.ts - Remove extra closing braces in shared.ts type definitions - Wrap MCP connection in try/catch to continue without MCP tools on failure - Clarify thinkingConfig logic: use ?? true instead of !== false - Add explanatory comment about thinkingEnabled default behavior - Apply createOnceOnlyResolve wrapper in QueryImpl.registerPendingPermission
- Add try-catch around injectAgents() to gracefully handle plugin agent tool validation failures (prevents test crashes from unknown 'LS' tool) - Add console.warn logging to agent loading/injection catch blocks for debugging visibility (matches v2.ts pattern) - Add pendingPermissionPrompts.clear() to close() and interrupt() methods in both query.ts and v2.ts to prevent memory accumulation - Add close() method to SDKSession interface and SDKSessionImpl - Wrap MCP connection in query.ts with try-catch (matches v2.ts behavior) - Add timeoutQueue cleanup in finally blocks (query.ts + v2.ts) - Remove error.stack from MCP error messages to prevent internal path leak All 208 SDK tests pass. TypeScript errors are pre-existing.
- Add SDKAgentLoadFailureMessage type for agent load failure events - Emit agent definition/injection failures to SDK message stream - Add tool name to permission timeout denial message - Replace 'as any' casts with proper typed state access - Fix supportedCommands to use correct mcp.commands/plugins.commands paths - Update test for correct AppState structure
Blocking Issues Fixed: - MCP cleanup missing on session/query close - now disconnects MCP clients to prevent resource leaks in long-running processes with multiple sessions - Engine reference not cleared on close - now sets _engine = null to prevent memory leaks - Added MCP cleanup tests (9 new tests covering cleanup scenarios) Non-Blocking Issues Fixed: - Removed redundant catch block that just rethrew errors (query.ts) - Fixed inconsistent timeout denial message format (permissions.ts) - Fixed hardcoded tool name 'Bash' in test (permissions.test.ts) - Exported PermissionResolveDecision type for SDK consumers (index.ts) All 217 SDK tests pass.
- Add close() method to SDKSession interface (documented but missing from type) - Fix SDKSessionInfo, ForkSessionResult, SessionMessage field naming: snake_case → camelCase to match sdk.d.ts public contract and implementation - Add uuid and session_id to SDKPermissionTimeoutMessage for correlation - Fix JSDoc comment in forkSession to use sessionId (not session_id) These changes align internal types (shared.ts) with the public SDK contract (sdk.d.ts) and actual implementation output. The merge from origin/main introduced snake_case types that mismatched camelCase implementation and tests.
Merge 0f3aa7a incorrectly took main's side for this comment, reverting PR2 fix c725c48. Project has migrated to ~/.openclaude.json, not ~/.claude.json. This is the only PR2 fix lost during merge - all other PR2 fixes (permissions.ts race conditions, state.ts parentSessionId, etc.) are preserved in PR3 via subsequent fix commits.
package.json exports (Breaking Change Mitigation): - Add "./package.json": "./package.json" for tool compatibility - Add "./dist/cli.mjs": "./dist/cli.mjs" for CLI bundle access - Keep ./sdk as sole library entrypoint - Root import intentionally blocked (CLI-first package, no main field) build.ts (Bug Fix): - Add | undefined to result/sdkResult type declarations - Add optional chaining: result?.success, sdkResult?.success - Prevents TypeError masking actual build errors when Bun.build throws tests/sdk/package-consumer-types.test.ts: - Update simulated exports to match real package.json - Add tests verifying exports map structure and file existence
- Add workspace support to root package.json - Create packages/desktop with electron-vite build system - Main process: window creation, app lifecycle, security settings - Preload: contextBridge with platform info - Renderer: React skeleton with dark theme - Tests: 8 tests (main/preload/renderer) - Build: electron-vite produces main/preload/renderer bundles Part of 18-PR desktop app implementation plan. Base branch: sdk/pr3-sdk-runtime
Production CSP no longer allows unsafe-eval or unsafe-inline scripts. style-src retains unsafe-inline for Vite's injected styles.
electron-vite couldn't resolve trpc-electron/main from workspace root node_modules. Externalize these deps instead of bundling. Also update package.json main field to out/main/index.cjs (electron-vite output).
Merge origin/main into desktop/pr1-electron-shell to include: - feat: SDK Runtime — Query Engine, Sessions, Build Pipeline (Gitlawb#984) - fix(web-search): surface diagnostic for 0 hits (Gitlawb#1006) - feat: Karpathy guidelines skill (Gitlawb#909) - fix: commit attribution configuration (Gitlawb#920) - feat: provider profiles in user config (Gitlawb#969) - feat: self-hosted Firecrawl (Gitlawb#949) - feat: context partitioning and relevance pruning (Gitlawb#849) - fix: user agent loading (Gitlawb#972) - fix(groq): strip unsupported store field (Gitlawb#983) - feat: Hicap gateway provider (Gitlawb#979) No conflicts — clean auto-merge.
- Fix critical preload path mismatch (.js → .cjs) that broke contextBridge at runtime - Rewrite main-process tests to properly exercise whenReady promise chain (old tests used callback pattern, all assertions were silently skipped) - Remove unused production deps (better-sqlite3, electron-log, electron-updater, superjson) - Remove unused @electron-toolkit/preload devDep - Clean external array in vite config - Add .catch() error handler on app.whenReady() - Tighten CSP by removing dev-only ws://localhost:* from connect-src
- Move CSP from HTML meta tag to session headers (dev/prod branching) - Add Window.platform type declaration (env.d.ts) - Add tests for optimizer, window-all-closed (win32 + macOS) - Extract mockApp to test scope for handler inspection - Remove @types/better-sqlite3, add private:true, clean vite config
…rkspace Previous lockfile had drifted significantly, missing @smithy/* and @aws-sdk/* transitive resolutions needed by CLI build/smoke.
Adding "workspaces": ["packages/*"] changes bun's module resolution and stops hoisting transitive deps (@smithy/*, @aws-sdk/*, vscode-jsonrpc, highlight.js) needed by CLI build. Desktop package is self-contained in PR1 with no SDK dependency — workspaces deferred to PR2+ when SDK integration is actually needed.
Root-level `bun test` discovers desktop test files but lacks vitest-specific APIs (vi.resetModules, vi.doMock) and jsdom. Main tests: skip when vi.resetModules unavailable (describe.skip guard) Renderer tests: skip when DOM environment unavailable (document check) Preload tests: pass normally (only uses basic vi.fn/vi.mock)
…covery CI runs `bun test` from repo root which discovers *.test.* files. Desktop tests use vitest-specific APIs (vi.resetModules, vi.doMock, jsdom, @testing-library/react) incompatible with bun's built-in runner. Rename *.test.ts -> *.vitest.ts so bun's test runner ignores them. Vitest discovers them via the updated include pattern in vitest.config.ts. Result: - bun test (root CI): 0 desktop tests discovered — clean - vitest (packages/desktop): 11/11 pass
|
Thanks for this, @emsanakhchivan — and congrats on #984 landing earlier today. The PR1 code itself is well-scoped: additive, tests are thoughtful (the cross-platform path regex, the The harder question is the 18-PR / ~33,500-line commitment outlined in the body. Merging PR1 implicitly endorses the full stack — once
If the answers to 1–3 are "yes there's a doc, yes it's agreed, yes I'll drive it" then PR1 is a clear approve from me. If not, I'd suggest pausing PR1 until alignment lands — not because the code is bad, but because of the path-dependency it creates. One small nit on PR1 itself: the file table in the body lists |
Add comprehensive design doc for the desktop app initiative: - 18-PR wave-based delivery plan - Electron vs Tauri rationale (stability, Node.js native, sidecar trade-offs) - Same-repo rationale (SDK access, shared CI, unified development) - Permission system, SDK integration layer, database schema - Technology stack and constraints Addresses reviewer concern about missing RFC/design doc.
- Add `desktop` job to pr-checks.yml (parallel with `web` job) - Tests run via vitest in packages/desktop/ with jsdom environment - Rename *.vitest.* back to standard *.test.* pattern - Add vitest-only guards to gracefully skip in bun:test runner - Main tests: skip when vi.resetModules unavailable - Renderer tests: skip when jsdom (document) unavailable - Preload tests: pass in both runners (basic vi.mock only) Result: - CI desktop job: 11/11 pass (vitest + jsdom) - CI smoke-and-tests: 2 pass, 9 skip, 0 fail (bun:test)
emsanakhchivan
force-pushed
the
desktop/pr1-electron-shell
branch
from
4248c1e to
53375df
Compare
… error describe.skip callback was being evaluated even when skipped, causing 'Cannot find module @testing-library/react' error. Moved imports into it() callbacks so they only execute when tests actually run (vitest only, not bun:test).
Thanks for the thorough review, @gnanam1990 really appreciate the detailed feedback on the code quality and test patterns. You're right that the 18-PR scope is the harder question here. Let me address each point: Q1: RFC / Design Doc Design doc committed: Covers architecture, permission system, SDK integration, database schema, 18-PR wave plan, and tech stack. Electron vs Tauri — leaning toward Electron for stability and Node.js native runtime (SDK runs directly in main process, no sidecar needed for MCP servers and tool execution). Tauri's smaller binary is attractive but introduces complexity: sidecar process for Node.js work + Rust for any native code. Open to discussion here if there's a preference. Same repo vs separate — direct SDK import without publishing, shared CI. Q2: Co-maintainer Approval Waiting for @kevincodex1 and @anandh8x to review and comment in this thread. Your workspaces concern was fair, it was in PR1 originally and did break the CLI build, so I've pulled it out. That said, the underlying question doesn't go away: desktop will need SDK access at some point, and we should agree on how before merging. I see a few paths forward:
What's your instinct here? Happy to go with whatever the team prefers. Q3: Maintenance Commitment I'm committing to drive the 18-PR stack to completion. If the team needs to redistribute work across waves, each wave is scoped to be independently useful, so handoffs are straightforward. Will coordinate with project leads as needed. Q4: File Table Nit Resolved — files renamed back to standard |
|
One more thing worth raising before this stack picks up momentum: we have no visual sense of what the desktop app actually looks like. No screenshots in PR1, no mockups for the planned waves, no walkthrough GIF, not even a Figma link. For an Electron application PR — where the entire point of the surface is visual — that's the single biggest piece of context missing. Concretely, before merging PR1, would be very helpful to see:
The PR body has 33,500 lines of described intent but zero pixels. Hard to give a confident review on that basis. |
|
Settings Menu Design V: 
|
|
can we align the branding to https://openclaude.gitlawb.com/ ? so we will have a uniformed look and feel |
|
this is great btw. thank you |
Add Section 9 — Branding & Design System (Website Alignment) to the desktop app design doc. Documents the complete OpenClaude website design language that the desktop app must replicate for a unified visual identity: - Typography: Fira Code monospace-first font stack with OpenType features - Color system: Full light/dark theme tokens with warm orange (#ff7a1a) accent replacing the current blue (#0034FF) - Theme toggle: Light default with localStorage persistence - Borders: Sharp corners (no border-radius), dashed dividers - Component specs: Nav, buttons, code blocks, scrollbars, splash screen - PR4 rebranding checklist covering all UI surfaces
@kevincodex1 Done. Here's what was done: PR1 — Updated the design doc (docs/desktop-app-design.md) with a full Section 9 covering the complete PR4 — Rebranded all 31 UI files to match the website:
openclaude-nth-v.mp4 |
|
Hi, @kevincodex1 - What are your thoughts on the topics mentioned above, specifically the Tauri vs. Electron debate and the architecture we should use to integrate the SDK, etc.? |
|
Hi @emsanakhchivan thank you so much for working on this, please give me till tomorrow or Friday to have a deeper review on this |
Bring sidebar architecture spec, branding & design system, hybrid JSONL storage sections from PR3 to keep documentation consistent across branches.
3 participants
PR 1: Electron Shell + Build System
Summary
Introduces the OpenClaude Desktop application as
packages/desktop/— a bootable Electron app with main process, preload, and renderer skeleton. Window opens with "OpenClaude Desktop" placeholder, dev server works with HMR, build produces runnable binary, and all 11 tests pass. This PR is the first in the 18-PR desktop stack, establishing the foundational Electron architecture, build pipeline, and test infrastructure that subsequent PRs build upon.Design Doc
Committed at
docs/desktop-app-design.md— covers architecture, permission system, SDK integration, database schema, 18-PR wave plan, and tech stack.Electron vs Tauri: Leaning toward Electron for stability and Node.js native runtime (SDK runs directly in main process — no sidecar needed for MCP servers/tool execution). Tauri's smaller binary is attractive but requires sidecar for Node.js work + Rust for native code. Open to discussion.
Same repo vs separate: Direct SDK import without npm publish, shared CI infrastructure.
openclaude-nth-v.mp4
Settings Menu Design V:
What changed
docs/desktop-app-design.md.github/workflows/pr-checks.ymldesktopCI job — runs vitest onpackages/desktop/tests/with jsdom, parallel with existingwebandsmokejobspackages/desktop/package.json"private": truepackages/desktop/electron.vite.config.tspackages/desktop/tsconfig.json@/*path aliaspackages/desktop/vitest.config.ts@alias for renderer importspackages/desktop/.gitignoreout/,release/,node_modules/,dist/packages/desktop/build/icon.{ico,icns,png}packages/desktop/src/main/index.tspackages/desktop/src/preload/index.tswindow.platform(os, arch) via contextBridge — no Node.js API leakagepackages/desktop/src/renderer/index.htmlpackages/desktop/src/renderer/main.tsxReactDOM.createRootmountpackages/desktop/src/renderer/App.tsxpackages/desktop/src/renderer/env.d.tswindow.platformpackages/desktop/src/renderer/styles/globals.csspackages/desktop/tests/main/index.test.tspackages/desktop/tests/preload/index.test.tspackages/desktop/tests/renderer/App.test.tsxNotable: Root
package.jsonunchanged — no workspaces added yet. Workspaces deferred to PR2 when SDK dependency is needed.Why it changed
OpenClaude needs a desktop application to provide a native OS experience (system tray, file dialogs, local file access, dock/taskbar integration) that the CLI and web UI cannot offer. The design spec defines a full GUI desktop app with CLI parity, built as a feature-based monolith inside the monorepo. PR1 establishes the three-process Electron architecture (main / preload / renderer) with a working dev loop, production build pipeline, and comprehensive test coverage. This is the foundation upon which all 18 PRs depend.
Beyond CLI parity, the desktop surface unlocks UX patterns that a terminal or browser tab cannot: instant model switching from a persistent toolbar, native keyboard shortcuts, and a workspace that retains context across conversations without page reloads. These capabilities require a dedicated application layer, which this PR lays the groundwork for.
Impact
packages/desktop/—cd packages/desktop && bun install && bun run devbun installresolves workspaces including desktoppackages/desktop/tests/— run withbun run testout/main/index.cjs,out/preload/index.cjs,out/renderer/assetsbun run package:winproduces runnable binary inrelease/session.defaultSession.webRequest.onHeadersReceived— dev mode allowsws://localhost:*for Vite HMR, production is strict@electron-toolkit/utilsfor platform helpers, rawcontextBridge(no@electron-toolkit/preload)Testing
bun run test— 11 pass, 0 fail (6 main + 2 preload + 3 renderer)bun run ts:check— 0 errorsbun run build— electron-vite produces all 3 bundles successfullyvi.doMock+vi.resetModulespattern to properly test promise-basedwhenReady()/preload[/\\]index\.cjs/for Windows/POSIXwindow-all-closedhandler tested for both macOS (no quit) and non-macOS (quit)watchWindowShortcutsregistration verified viabrowser-window-createdevent simulationNotes
.cjsmatching electron-vite CJS output formatprivate: trueprevents accidental npm publishwindow.platformtyped viaenv.d.tsfor renderer TypeScript supportcontextIsolation: true,nodeIntegration: false— renderer has no Node.js accesssandbox: false— required forprocess.platform/process.archin preloadwindow.platform(os, arch strings) — no filesystem, network, or IPCdefault-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; connect-src 'self'; img-src 'self' data: blob: https:setWindowOpenHandlerredirects allwindow.opento system browser