feat(desktop): wire WyStack server+client over Electron IPC (YW-69)#44
feat(desktop): wire WyStack server+client over Electron IPC (YW-69)#44youhaowei wants to merge 4 commits into
Conversation
…e foundation) DashFrame's wystack pointer was 7 commits behind origin/main, missing the T2a/T2b/T3a foundation that downstream v0.2 integration work builds on: - ea4f20f @wystack/transport package + Pipe primitive (T2a / YW-55) - 235183f server Engine: Session + Dispatch over Pipe (T2b / YW-56) - 402bb3c client neutral Engine + WebSocket adapter (T3a / YW-58) This was the actual blocker behind 'waiting on WyStack' — the work had landed on the remote but was never consumed. Unblocks grooming of T2c (routes.ts -> Hono adapter) and T3b (client WS adapter). Verified: bun install clean, @wystack/transport 75/75 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…b WS relocation) Consumes two merged WyStack v0.2 enablement tickets: - T2c (YW-57, wystack#28): routes.ts WS path delegates to attachEngine over a Hono-socket Pipe; REST + invalidation kept inline. Includes the ack-failure -> 4002 regression test from review. - T3b (YW-59, wystack#29): browser WS adapter relocated to packages/client/src/transport/websocket.ts; public surface preserved. Verified: bun install clean, wystack server+client 130/130 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… T5+T6) Consumes the merged WyStack Electron IPC transport: - T5 (YW-67, wystack#33): @wystack/server/electron — attachElectronTransport binds ipcMain + webContents into a Pipe driving attachEngine. Lifecycle is __connect/__close-driven (model A); no did-finish-load. - T6 (YW-68, wystack#32): @wystack/client/electron — createElectronPipe wraps ipcRenderer as the EnginePipe; __connect on construction signals reload. This completes the WyStack transport spine. DashFrame can now mount a WyStack server over IPC (next: T7 integration smoke). Verified: bun install clean, wystack server+client 180/180 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Implements the v0.2 integration milestone: mounts a WyStack server in the Electron
main process, bridges IPC channels through the preload, and fetches projectInfo in
the renderer via a WyStack IPC query instead of the hand-rolled dashframe:project:info
invoke path.
Key changes:
- main.ts: buildWyStackApp() registers a `projectInfo` query backed by
ProjectHandle.meta, then calls attachElectronTransport({ app, ipcMain }) to mount
the WyStack server over the wystack:c2s / wystack:s2c IPC channels. No auth
(trusted in-process transport).
- preload.ts: exposes window.wysIpc — a minimal IpcRendererLike surface scoped to
the wystack:c2s/s2c channels via contextBridge. A WeakMap bridges the original
callback references through the context-isolation boundary so removeListener
correlates correctly.
- apps/renderer/src/wystack.ts (new): custom WyStackClient over IPC. Uses
createElectronPipe from @wystack/client/electron for the pipe, a hand-rolled
call→result correlator (Map<id, {resolve,reject}>), and a no-op WsManager (non-
reactive slice only — YW-62 gates subscriptions/invalidation). Also exports
useProjectInfo() — a typed convenience hook wrapping useQuery.
- apps/renderer/src/main.tsx: wraps app in QueryClientProvider + WyStackProvider.
- apps/renderer/src/routes/index.tsx: calls useProjectInfo() and renders the
project name/version from the WyStack response.
- desktop build:main: changed from --packages=external to explicitly bundle @wystack/*
and its transitive deps (drizzle-orm, zod, hono) inline. postgres is kept external
(dynamic import in @wystack/db that the desktop PGLite path never takes).
- turbo.json: per-package task overrides for @dashframe/desktop and @dashframe/renderer
typecheck/test tasks so turbo does not try to rebuild @wystack/* packages through
the dependency chain (their dists are managed by bun build:wystack, not turbo).
- scripts/build-wystack.mjs + package.json: new build:wystack script that builds
@wystack/transport, @wystack/server, and @wystack/client dists, excluding test
files and the bun-only serve-bun.ts entrypoint.
NOTE: The call→result correlator is hand-rolled in apps/renderer/src/wystack.ts
because @wystack/client has no built-in IPC call transport — the client engine only
owns the reactive tier. A proper @wystack/client IPC call transport should land as
follow-up work.
Runtime evidence: Electron main process starts, opens the project DB at
~/.DashFrame/default-project, prints "[dashframe] WyStack IPC transport mounted",
and creates the window. The only error when launched headless is ERR_CONNECTION_REFUSED
for the Vite URL (expected: no dev server running in the test env).
bun check: 59/59 tasks successful.
Closes YW-69
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR integrates WyStack, a query framework, into DashFrame's Electron app. It adds build tooling for WyStack packages, mounts a WyStack server in the Electron main process that exposes project metadata queries, exposes an IPC bridge in the preload layer, implements a renderer-side IPC-backed client with React Query integration, and updates the home route to fetch and display project info over this new IPC channel. ChangesWyStack IPC Integration
Sequence DiagramssequenceDiagram
participant Main as Electron Main
participant WyStackApp
participant ipcMain
participant Renderer
Main->>WyStackApp: buildWyStackApp(projectHandle)
WyStackApp-->>Main: WyStackApp configured with projectInfo
Main->>ipcMain: attachElectronTransport(wyStackApp)
ipcMain-->>Main: Transport mounted
Renderer->>ipcMain: wystack:c2s message
ipcMain->>WyStackApp: route to handler
WyStackApp-->>ipcMain: result frame
ipcMain-->>Renderer: wystack:s2c response
sequenceDiagram
participant Hook as useProjectInfo
participant Client as ipcClient
participant Correlator as Call/Result Map
participant wysIpc
Hook->>Client: query(projectInfo)
Client->>Correlator: new call id, register handlers
Correlator->>wysIpc: send wystack:c2s with id, path, args
wysIpc-->>Correlator: wystack:s2c result frame with matching id
Correlator-->>Client: resolve promise with result
Client-->>Hook: return typed project data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/desktop/src/main.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. apps/desktop/src/preload.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. apps/renderer/src/dashframe-api.d.tsESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ac1ca067a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| { | ||
| name: '@wystack/server', | ||
| dir: path.join(WYSTACK, 'packages/server'), | ||
| include: ['src'], |
There was a problem hiding this comment.
Build @wystack/db before @wystack/server
On a clean checkout this script builds @wystack/server before any @wystack/db dist exists, but @wystack/server/src/create.ts imports @wystack/db and the package exports resolve types/imports from dist. That makes the new setup prerequisite fail during the server tsc run before the desktop IPC wiring can be built; add/build @wystack/db before @wystack/server or narrow the server build to files that do not import the db package.
Useful? React with 👍 / 👎.
| function ipcCall( | ||
| funcPath: string, | ||
| args: Record<string, unknown>, | ||
| ): Promise<unknown> { | ||
| return new Promise((resolve, reject) => { | ||
| const id = nextCallId(); | ||
| pending.set(id, { resolve, reject }); | ||
| pipe.send({ type: "call", id, path: funcPath, args }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
ipcCall stores a {resolve, reject} in pending before calling pipe.send. If the send throws synchronously (e.g., context-bridge serialization error), the Promise constructor catches it and rejects—but the pending entry for that id is never deleted, so it leaks. More critically, if the IPC response is silently dropped for any reason (renderer reload mid-call, transient IPC error, unexpected frame type), the promise hangs indefinitely and the pending entry accumulates. Adding a timeout (e.g., via AbortSignal or setTimeout+pending.delete) and a try/finally cleanup around pipe.send would make this more robust.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/renderer/src/wystack.ts
Line: 98-107
Comment:
**Pending calls never time out or clean up on send failure**
`ipcCall` stores a `{resolve, reject}` in `pending` before calling `pipe.send`. If the send throws synchronously (e.g., context-bridge serialization error), the Promise constructor catches it and rejects—but the `pending` entry for that `id` is never deleted, so it leaks. More critically, if the IPC response is silently dropped for any reason (renderer reload mid-call, transient IPC error, unexpected frame type), the promise hangs indefinitely and the `pending` entry accumulates. Adding a timeout (e.g., via `AbortSignal` or `setTimeout`+`pending.delete`) and a `try/finally` cleanup around `pipe.send` would make this more robust.
How can I resolve this? If you propose a fix, please make it concise.
| const { detach } = attachElectronTransport({ | ||
| app: wyStackApp, | ||
| ipcMain, | ||
| // No resolveContext — trusted in-process transport, no auth handshake. | ||
| }); | ||
| app.on("before-quit", () => detach()); | ||
|
|
There was a problem hiding this comment.
detach() is called twice during the quit sequence
Both closeProjectBeforeQuit and () => detach() are registered on the same before-quit event. On the first quit, both fire: closeProjectBeforeQuit calls event.preventDefault() then asynchronously closes the project and calls app.quit() again; detach() runs in that same first-event pass. The second app.quit() fires before-quit again—closeProjectBeforeQuit exits early, and () => detach() runs a second time. If attachElectronTransport's detach is not idempotent (e.g., it warns or throws when a handler is removed twice), the quit sequence would be noisy or broken. Consider guarding with a let detached = false flag, or move the detach() call inside closeProjectBeforeQuit so it's subject to the same isClosingProject guard.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main.ts
Line: 150-156
Comment:
**`detach()` is called twice during the quit sequence**
Both `closeProjectBeforeQuit` and `() => detach()` are registered on the same `before-quit` event. On the first quit, both fire: `closeProjectBeforeQuit` calls `event.preventDefault()` then asynchronously closes the project and calls `app.quit()` again; `detach()` runs in that same first-event pass. The second `app.quit()` fires `before-quit` again—`closeProjectBeforeQuit` exits early, and `() => detach()` runs a second time. If `attachElectronTransport`'s `detach` is not idempotent (e.g., it warns or throws when a handler is removed twice), the quit sequence would be noisy or broken. Consider guarding with a `let detached = false` flag, or move the `detach()` call inside `closeProjectBeforeQuit` so it's subject to the same `isClosingProject` guard.
How can I resolve this? If you propose a fix, please make it concise.| function buildWyStackApp(handle: ProjectHandle): WyStackApp { | ||
| return { | ||
| functions: new Map([ | ||
| [ | ||
| "projectInfo", | ||
| { | ||
| type: "query" as const, | ||
| path: "projectInfo", | ||
| args: {}, | ||
| handler: async () => ({ | ||
| projectId: handle.meta.projectId, | ||
| name: handle.meta.name, | ||
| version: handle.meta.version, | ||
| schemaVersion: handle.meta.schemaVersion, | ||
| createdAt: handle.meta.createdAt.toISOString(), | ||
| createdBy: handle.meta.createdBy, | ||
| }), | ||
| }, | ||
| ], | ||
| ]), | ||
| subscriptions: createSubscriptionManager(), | ||
| async call(funcPath, args) { | ||
| const fn = this.functions.get(funcPath); | ||
| if (!fn) throw new Error(`Unknown function: ${funcPath}`); | ||
| // projectInfo has no args and doesn't use ctx.db — pass a minimal context | ||
| const result = await fn.handler({ db: null as never }, args); | ||
| return { | ||
| result, | ||
| tablesRead: new Set<string>(), | ||
| tablesWritten: new Set<string>(), | ||
| }; | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
projectInfo data construction
buildWyStackApp and the existing registerIpc both read the same six fields from handle.meta and construct an identical shape (projectId, name, version, schemaVersion, createdAt.toISOString(), createdBy). If ProjectInfo evolves (a new field added, a field renamed), the two sites will drift. Consider extracting a small helper like projectInfoFromHandle(handle: ProjectHandle): ProjectInfo and calling it from both places.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/main.ts
Line: 38-71
Comment:
**Duplicate `projectInfo` data construction**
`buildWyStackApp` and the existing `registerIpc` both read the same six fields from `handle.meta` and construct an identical shape (`projectId`, `name`, `version`, `schemaVersion`, `createdAt.toISOString()`, `createdBy`). If `ProjectInfo` evolves (a new field added, a field renamed), the two sites will drift. Consider extracting a small helper like `projectInfoFromHandle(handle: ProjectHandle): ProjectInfo` and calling it from both places.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| execSync(`bun x tsc -p ${tmpFile}`, { stdio: 'inherit' }); | ||
| console.log(`[build-wystack] ${pkg.name}: done`); | ||
| } catch (err) { | ||
| console.error(`[build-wystack] ${pkg.name}: build failed`); | ||
| process.exit(1); | ||
| } finally { | ||
| try { execSync(`rm -f ${tmpFile}`); } catch {} | ||
| } |
There was a problem hiding this comment.
execSync template literals will break on any machine whose repo root contains spaces (common on macOS home directories like /Users/John Doe/). The tsc invocation and rm -f both interpolate tmpFile without quoting.
| execSync(`bun x tsc -p ${tmpFile}`, { stdio: 'inherit' }); | |
| console.log(`[build-wystack] ${pkg.name}: done`); | |
| } catch (err) { | |
| console.error(`[build-wystack] ${pkg.name}: build failed`); | |
| process.exit(1); | |
| } finally { | |
| try { execSync(`rm -f ${tmpFile}`); } catch {} | |
| } | |
| execSync(`bun x tsc -p "${tmpFile}"`, { stdio: 'inherit' }); | |
| console.log(`[build-wystack] ${pkg.name}: done`); | |
| } catch (err) { | |
| console.error(`[build-wystack] ${pkg.name}: build failed`); | |
| process.exit(1); | |
| } finally { | |
| try { execSync(`rm -f "${tmpFile}"`); } catch {} | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/build-wystack.mjs
Line: 97-104
Comment:
Unquoted path variables in `execSync` template literals will break on any machine whose repo root contains spaces (common on macOS home directories like `/Users/John Doe/`). The `tsc` invocation and `rm -f` both interpolate `tmpFile` without quoting.
```suggestion
execSync(`bun x tsc -p "${tmpFile}"`, { stdio: 'inherit' });
console.log(`[build-wystack] ${pkg.name}: done`);
} catch (err) {
console.error(`[build-wystack] ${pkg.name}: build failed`);
process.exit(1);
} finally {
try { execSync(`rm -f "${tmpFile}"`); } catch {}
}
```
How can I resolve this? If you propose a fix, please make it concise.| for (const pkg of packages) { | ||
| const distDir = path.join(pkg.dir, 'dist'); | ||
| if (existsSync(distDir)) { | ||
| console.log(`[build-wystack] ${pkg.name}: dist already exists, skipping`); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
dist/ directory skips rebuild silently
The script checks existsSync(distDir) and skips the package entirely if dist/ is present. A partially-built or outdated dist left over from a different commit or a failed prior build will never be replaced unless it is manually deleted. During active development on the wystack submodule this can be confusing: bun setup appears to succeed while the renderer actually loads stale code. Consider adding a --force flag or comparing a checksum/mtimes against the source files.
Prompt To Fix With AI
This is a comment left during a code review.
Path: scripts/build-wystack.mjs
Line: 74-79
Comment:
**Stale `dist/` directory skips rebuild silently**
The script checks `existsSync(distDir)` and skips the package entirely if `dist/` is present. A partially-built or outdated `dist` left over from a different commit or a failed prior build will never be replaced unless it is manually deleted. During active development on the wystack submodule this can be confusing: `bun setup` appears to succeed while the renderer actually loads stale code. Consider adding a `--force` flag or comparing a checksum/mtimes against the source files.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
26-50: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winInvoke the build script with
bun, notnode.Both
setup(Line 26) andbuild:wystack(Line 50) shell out tonode, but the repo standardizes on Bun as the runtime (packageManager: bun@1.3.5). Usingbunkeeps tooling consistent and avoids relying on a separate Node install.♻️ Proposed fix
- "setup": "git submodule update --init --recursive && bun install && node scripts/build-wystack.mjs", + "setup": "git submodule update --init --recursive && bun install && bun scripts/build-wystack.mjs", @@ - "build:wystack": "node scripts/build-wystack.mjs" + "build:wystack": "bun scripts/build-wystack.mjs"As per coding guidelines: "Use
buninstead ofnpm,yarn, orpnpmfor package management and runtime".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` around lines 26 - 50, The package scripts "setup" and "build:wystack" currently invoke node to run scripts (node scripts/build-wystack.mjs); change both to use bun (bun scripts/build-wystack.mjs) so the repo consistently uses Bun as the runtime per packageManager and guidelines; update the "setup" and "build:wystack" script entries to shell out to bun instead of node.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/main.ts`:
- Around line 47-54: The project metadata object returned in the IPC handler
(the async handler that builds { projectId, name, version, schemaVersion,
createdAt, createdBy } from handle.meta) is duplicated in registerIpc; extract a
single helper function (e.g., buildProjectInfo or getProjectInfo) that accepts
the source meta (handle.meta or equivalent) and returns the unified shape
(ensure createdAt is converted with toISOString()); replace the inline object in
both the handler and the registerIpc usage to call that helper so both code
paths share the same implementation.
In `@apps/renderer/src/routes/index.tsx`:
- Around line 16-39: The page currently uses raw HTML tags (main, h1, p,
section, strong, small) in the routes component; replace them with the
stdui/@dashframe/ui equivalents (e.g., Container/Layout, Typography/Heading,
Text/Paragraph, Section/Card, Strong/Emphasis, and Caption/Small) by first
adding any missing components to the stdui or `@dashframe/ui` package and then
refactoring the JSX in this routes module to import and use those components
(look for the component rendering block that references isLoading/isError/data
and replace each raw element with the corresponding stdui/@dashframe/ui
component while preserving props and conditional logic).
- Line 19: Replace the inline style on the error paragraph so it uses the
semantic token class instead of hard-coded color: find the JSX that renders the
error (the expression using isError and error?.message) and remove style={{
color: "red" }} and apply the semantic token class (e.g.
className="text-palette-destructive") to that <p> so the error uses the stdui
semantic color token for destructive text.
In `@apps/renderer/src/wystack.ts`:
- Around line 77-107: ipcCall currently registers pending entries that can leak
and hang forever; update ipcCall to create a timeout (e.g., setTimeout) that
rejects the promise with a descriptive Error after a configurable duration and
removes the pending entry, ensure the timeout is cleared when the promise is
settled in the existing pipe.onMessage handlers (where pending.get(msg.id)
resolves/rejects) and also ensure pending.delete is performed if pipe.send
throws by wrapping pipe.send in try/catch and rejecting+cleaning up there;
reference the ipcCall function, the pending map, nextCallId(), pipe.send, and
the onMessage resolve/reject handling to implement the guaranteed cleanup and
timer lifecycle.
In `@scripts/build-wystack.mjs`:
- Around line 74-79: The current loop in build-wystack.mjs (iterating packages
and using distDir/existing guard) can silently reuse stale dist/, so change the
skip logic to compare file mtimes: for each pkg (pkg.dir), recursively compute
the newest mtime under the source tree (e.g., pkg.dir/src or
pkg.dir/**/*.{ts,js,jsx,tsx}) and compare it to the mtime of distDir (fs.stat).
Only skip when distDir exists AND its mtime is newer than the newest source
mtime; otherwise treat it as stale and rebuild; update any logging to indicate
when dist is skipped vs rebuilt. Ensure you reference the same variables used in
the loop (packages, pkg.dir, distDir) when implementing the check.
- Line 1: This file fails Prettier format checks; run the project's formatter to
fix it (e.g., execute the project's format script: "bun run format" or run
Prettier with --write on scripts/build-wystack.mjs) and commit the reformatted
file so CI's "Lint, Type Check & Format" check passes; ensure the shebang line
(#!/usr/bin/env node) remains intact after formatting.
- Around line 96-104: The finally block currently calls execSync(`rm -f
${tmpFile}`) which is POSIX-only and fails on Windows and the unquoted
${tmpFile} can break on paths with spaces; change that to use the imported fs
API: call fs.unlinkSync(tmpFile) inside the finally try/catch and ignore ENOENT
(or catch and no-op), and also ensure the earlier execSync invocation that runs
tsc includes the tmpFile path quoted (e.g., wrap ${tmpFile} in quotes in the
command string) so paths with spaces are handled safely; reference the
try/catch/finally block, tmpFile variable, and the execSync calls to locate and
update the code.
- Line 21: Replace the platform-unsafe ROOT assignment by using
fileURLToPath(import.meta.url) (import from 'url') to compute ROOT correctly;
replace shell-string execSync calls that build commands with unquoted paths
(e.g., execSync(`bun x tsc -p ${tmpFile}`) and execSync(`rm -f ${tmpFile}`)) by
using child_process.execFileSync or spawnSync with argument arrays so paths with
spaces are handled safely and remove the POSIX-only rm by calling fs.rmSync or
fs.unlinkSync for tmpFile; finally, change the simple “skip if dist/ exists”
logic so it doesn’t silently reuse stale builds (either always rebuild, add a
forced-rebuild flag, or compare timestamps/checksum of inputs before skipping)
and update references to ROOT, tmpFile, and the exec calls accordingly (look for
constants ROOT, variable tmpFile, and the execSync usages).
---
Outside diff comments:
In `@package.json`:
- Around line 26-50: The package scripts "setup" and "build:wystack" currently
invoke node to run scripts (node scripts/build-wystack.mjs); change both to use
bun (bun scripts/build-wystack.mjs) so the repo consistently uses Bun as the
runtime per packageManager and guidelines; update the "setup" and
"build:wystack" script entries to shell out to bun instead of node.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9b1b6796-578f-41eb-84a5-e25a26c6cd78
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
apps/desktop/package.jsonapps/desktop/src/main.tsapps/desktop/src/preload.tsapps/renderer/package.jsonapps/renderer/src/dashframe-api.d.tsapps/renderer/src/main.tsxapps/renderer/src/routes/index.tsxapps/renderer/src/wystack.tslibs/wystackpackage.jsonscripts/build-wystack.mjsturbo.json
| handler: async () => ({ | ||
| projectId: handle.meta.projectId, | ||
| name: handle.meta.name, | ||
| version: handle.meta.version, | ||
| schemaVersion: handle.meta.schemaVersion, | ||
| createdAt: handle.meta.createdAt.toISOString(), | ||
| createdBy: handle.meta.createdBy, | ||
| }), |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Extract the projectInfo shape to avoid divergence.
This object is duplicated verbatim in registerIpc (Lines 103-110). Two code paths now expose the same project metadata and must be kept in sync by hand. Extract a single helper.
♻️ Proposed fix
+function toProjectInfo(handle: ProjectHandle) {
+ return {
+ projectId: handle.meta.projectId,
+ name: handle.meta.name,
+ version: handle.meta.version,
+ schemaVersion: handle.meta.schemaVersion,
+ createdAt: handle.meta.createdAt.toISOString(),
+ createdBy: handle.meta.createdBy,
+ };
+}
@@
- handler: async () => ({
- projectId: handle.meta.projectId,
- name: handle.meta.name,
- version: handle.meta.version,
- schemaVersion: handle.meta.schemaVersion,
- createdAt: handle.meta.createdAt.toISOString(),
- createdBy: handle.meta.createdBy,
- }),
+ handler: async () => toProjectInfo(handle),
@@
- ipcMain.handle("dashframe:project:info", () => ({
- projectId: handle.meta.projectId,
- name: handle.meta.name,
- version: handle.meta.version,
- schemaVersion: handle.meta.schemaVersion,
- createdAt: handle.meta.createdAt.toISOString(),
- createdBy: handle.meta.createdBy,
- }));
+ ipcMain.handle("dashframe:project:info", () => toProjectInfo(handle));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| handler: async () => ({ | |
| projectId: handle.meta.projectId, | |
| name: handle.meta.name, | |
| version: handle.meta.version, | |
| schemaVersion: handle.meta.schemaVersion, | |
| createdAt: handle.meta.createdAt.toISOString(), | |
| createdBy: handle.meta.createdBy, | |
| }), | |
| function toProjectInfo(handle: ProjectHandle) { | |
| return { | |
| projectId: handle.meta.projectId, | |
| name: handle.meta.name, | |
| version: handle.meta.version, | |
| schemaVersion: handle.meta.schemaVersion, | |
| createdAt: handle.meta.createdAt.toISOString(), | |
| createdBy: handle.meta.createdBy, | |
| }; | |
| } | |
| handler: async () => toProjectInfo(handle), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/main.ts` around lines 47 - 54, The project metadata object
returned in the IPC handler (the async handler that builds { projectId, name,
version, schemaVersion, createdAt, createdBy } from handle.meta) is duplicated
in registerIpc; extract a single helper function (e.g., buildProjectInfo or
getProjectInfo) that accepts the source meta (handle.meta or equivalent) and
returns the unified shape (ensure createdAt is converted with toISOString());
replace the inline object in both the handler and the registerIpc usage to call
that helper so both code paths share the same implementation.
| <main className="p-8 font-sans"> | ||
| <h1>DashFrame v0.2</h1> | ||
| <p>hello - desktop shell boot</p> | ||
| {isLoading && <p>Loading project info…</p>} | ||
| {isError && <p style={{ color: "red" }}>Error: {error?.message}</p>} | ||
| {data && ( | ||
| <section> | ||
| <p> | ||
| <strong>Project:</strong> {data.name} | ||
| </p> | ||
| <p> | ||
| <strong>Version:</strong> {data.version} | ||
| </p> | ||
| <p> | ||
| <strong>Schema version:</strong> {data.schemaVersion} | ||
| </p> | ||
| <p> | ||
| <strong>Project ID:</strong> {data.projectId} | ||
| </p> | ||
| <p> | ||
| <small>Fetched via WyStack IPC query over Electron transport</small> | ||
| </p> | ||
| </section> | ||
| )} | ||
| </main> |
There was a problem hiding this comment.
Replace raw HTML elements with stdui components.
The page uses raw HTML elements (<main>, <h1>, <p>, <section>, <strong>, <small>) instead of components from @stdui/react or @dashframe/ui. As per coding guidelines, all UI on pages must use stdui or @dashframe/ui components.
Add the necessary typography and layout components to the UI package first, then refactor this page to use them.
As per coding guidelines: "All UI on pages MUST use stdui or @dashframe/ui components; add missing components to stdui or the UI package first"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/renderer/src/routes/index.tsx` around lines 16 - 39, The page currently
uses raw HTML tags (main, h1, p, section, strong, small) in the routes
component; replace them with the stdui/@dashframe/ui equivalents (e.g.,
Container/Layout, Typography/Heading, Text/Paragraph, Section/Card,
Strong/Emphasis, and Caption/Small) by first adding any missing components to
the stdui or `@dashframe/ui` package and then refactoring the JSX in this routes
module to import and use those components (look for the component rendering
block that references isLoading/isError/data and replace each raw element with
the corresponding stdui/@dashframe/ui component while preserving props and
conditional logic).
| <h1>DashFrame v0.2</h1> | ||
| <p>hello - desktop shell boot</p> | ||
| {isLoading && <p>Loading project info…</p>} | ||
| {isError && <p style={{ color: "red" }}>Error: {error?.message}</p>} |
There was a problem hiding this comment.
Use semantic color token instead of inline style.
The error message uses an inline style={{ color: "red" }} instead of a semantic token. Replace with a semantic token like text-palette-destructive for theme consistency and accessibility.
🎨 Proposed fix
- {isError && <p style={{ color: "red" }}>Error: {error?.message}</p>}
+ {isError && <p className="text-palette-destructive">Error: {error?.message}</p>}As per coding guidelines: "Use semantic tokens from stdui (bg-neutral-bg, text-neutral-fg, bg-palette-primary), not shadcn naming"
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {isError && <p style={{ color: "red" }}>Error: {error?.message}</p>} | |
| {isError && <p className="text-palette-destructive">Error: {error?.message}</p>} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/renderer/src/routes/index.tsx` at line 19, Replace the inline style on
the error paragraph so it uses the semantic token class instead of hard-coded
color: find the JSX that renders the error (the expression using isError and
error?.message) and remove style={{ color: "red" }} and apply the semantic token
class (e.g. className="text-palette-destructive") to that <p> so the error uses
the stdui semantic color token for destructive text.
| pipe.onMessage((msg) => { | ||
| if (msg.type === "result") { | ||
| const p = pending.get(msg.id); | ||
| if (p) { | ||
| pending.delete(msg.id); | ||
| p.resolve(msg.data); | ||
| } | ||
| } else if (msg.type === "error" && msg.id) { | ||
| const p = pending.get(msg.id); | ||
| if (p) { | ||
| pending.delete(msg.id); | ||
| p.reject(new Error(msg.error)); | ||
| } | ||
| } | ||
| // authenticated / subscribed / invalidate frames are no-ops in this slice. | ||
| }); | ||
|
|
||
| // --------------------------------------------------------------------------- | ||
| // Call helper — sends a `call` frame and awaits the `result` response | ||
| // --------------------------------------------------------------------------- | ||
|
|
||
| function ipcCall( | ||
| funcPath: string, | ||
| args: Record<string, unknown>, | ||
| ): Promise<unknown> { | ||
| return new Promise((resolve, reject) => { | ||
| const id = nextCallId(); | ||
| pending.set(id, { resolve, reject }); | ||
| pipe.send({ type: "call", id, path: funcPath, args }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Pending calls can leak and hang forever — add a timeout and failure cleanup.
ipcCall registers an entry in pending and resolves/rejects it only when a matching result/error frame arrives. There is no timeout and no cleanup if a response never comes (main process crash, dropped frame, malformed id). Consequences:
- The
pendingmap grows unbounded across the session — a memory leak. - The returned promise never settles, so React Query stays stuck in
isLoadingwith no error surfaced to the UI.
Additionally, if pipe.send throws, the executor rejects the promise but the pending entry is left dangling.
🛡️ Proposed fix: timeout + guaranteed cleanup
function ipcCall(
funcPath: string,
args: Record<string, unknown>,
): Promise<unknown> {
return new Promise((resolve, reject) => {
const id = nextCallId();
- pending.set(id, { resolve, reject });
- pipe.send({ type: "call", id, path: funcPath, args });
+ const timer = setTimeout(() => {
+ if (pending.delete(id)) {
+ reject(new Error(`WyStack IPC call '${funcPath}' timed out`));
+ }
+ }, 30_000);
+ pending.set(id, {
+ resolve: (data) => {
+ clearTimeout(timer);
+ resolve(data);
+ },
+ reject: (err) => {
+ clearTimeout(timer);
+ reject(err);
+ },
+ });
+ try {
+ pipe.send({ type: "call", id, path: funcPath, args });
+ } catch (err) {
+ pending.delete(id);
+ clearTimeout(timer);
+ reject(err instanceof Error ? err : new Error(String(err)));
+ }
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pipe.onMessage((msg) => { | |
| if (msg.type === "result") { | |
| const p = pending.get(msg.id); | |
| if (p) { | |
| pending.delete(msg.id); | |
| p.resolve(msg.data); | |
| } | |
| } else if (msg.type === "error" && msg.id) { | |
| const p = pending.get(msg.id); | |
| if (p) { | |
| pending.delete(msg.id); | |
| p.reject(new Error(msg.error)); | |
| } | |
| } | |
| // authenticated / subscribed / invalidate frames are no-ops in this slice. | |
| }); | |
| // --------------------------------------------------------------------------- | |
| // Call helper — sends a `call` frame and awaits the `result` response | |
| // --------------------------------------------------------------------------- | |
| function ipcCall( | |
| funcPath: string, | |
| args: Record<string, unknown>, | |
| ): Promise<unknown> { | |
| return new Promise((resolve, reject) => { | |
| const id = nextCallId(); | |
| pending.set(id, { resolve, reject }); | |
| pipe.send({ type: "call", id, path: funcPath, args }); | |
| }); | |
| } | |
| pipe.onMessage((msg) => { | |
| if (msg.type === "result") { | |
| const p = pending.get(msg.id); | |
| if (p) { | |
| pending.delete(msg.id); | |
| p.resolve(msg.data); | |
| } | |
| } else if (msg.type === "error" && msg.id) { | |
| const p = pending.get(msg.id); | |
| if (p) { | |
| pending.delete(msg.id); | |
| p.reject(new Error(msg.error)); | |
| } | |
| } | |
| // authenticated / subscribed / invalidate frames are no-ops in this slice. | |
| }); | |
| // --------------------------------------------------------------------------- | |
| // Call helper — sends a `call` frame and awaits the `result` response | |
| // --------------------------------------------------------------------------- | |
| function ipcCall( | |
| funcPath: string, | |
| args: Record<string, unknown>, | |
| ): Promise<unknown> { | |
| return new Promise((resolve, reject) => { | |
| const id = nextCallId(); | |
| const timer = setTimeout(() => { | |
| if (pending.delete(id)) { | |
| reject(new Error(`WyStack IPC call '${funcPath}' timed out`)); | |
| } | |
| }, 30_000); | |
| pending.set(id, { | |
| resolve: (data) => { | |
| clearTimeout(timer); | |
| resolve(data); | |
| }, | |
| reject: (err) => { | |
| clearTimeout(timer); | |
| reject(err); | |
| }, | |
| }); | |
| try { | |
| pipe.send({ type: "call", id, path: funcPath, args }); | |
| } catch (err) { | |
| pending.delete(id); | |
| clearTimeout(timer); | |
| reject(err instanceof Error ? err : new Error(String(err))); | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/renderer/src/wystack.ts` around lines 77 - 107, ipcCall currently
registers pending entries that can leak and hang forever; update ipcCall to
create a timeout (e.g., setTimeout) that rejects the promise with a descriptive
Error after a configurable duration and removes the pending entry, ensure the
timeout is cleared when the promise is settled in the existing pipe.onMessage
handlers (where pending.get(msg.id) resolves/rejects) and also ensure
pending.delete is performed if pipe.send throws by wrapping pipe.send in
try/catch and rejecting+cleaning up there; reference the ipcCall function, the
pending map, nextCallId(), pipe.send, and the onMessage resolve/reject handling
to implement the guaranteed cleanup and timer lifecycle.
| @@ -0,0 +1,107 @@ | |||
| #!/usr/bin/env node | |||
There was a problem hiding this comment.
Run Prettier to clear the failing format check.
CI (Lint, Type Check & Format) reports Prettier --check issues in this file; run bun run format to fix.
🧰 Tools
🪛 GitHub Actions: CI / 1_Lint, Type Check & Format.txt
[warning] 1-1: Prettier --check reported formatting issues. Code style issues found in the above file. Run Prettier with --write to fix.
🪛 GitHub Actions: CI / Lint, Type Check & Format
[warning] 1-1: Prettier --check reported code style issues in this file. Run Prettier with --write to fix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/build-wystack.mjs` at line 1, This file fails Prettier format checks;
run the project's formatter to fix it (e.g., execute the project's format
script: "bun run format" or run Prettier with --write on
scripts/build-wystack.mjs) and commit the reformatted file so CI's "Lint, Type
Check & Format" check passes; ensure the shebang line (#!/usr/bin/env node)
remains intact after formatting.
| import { writeFileSync, mkdirSync } from 'fs'; | ||
| import path from 'path'; | ||
|
|
||
| const ROOT = new URL('..', import.meta.url).pathname; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
❓ Verification inconclusive
🌐 Web query:
Node.js fileURLToPath vs URL.pathname Windows file path handling
💡 Result:
In Node.js, URL.pathname does not perform Windows-path conversion; it exposes the URL path component in URL form (leading slashes, forward-slash separators, and any percent-encoding). To get a real Windows filesystem path, use url.fileURLToPath (optionally with { windows: true }). Key Windows behaviors 1) fileURLToPath vs URL.pathname - Example from the Node docs: new URL('file:///C:/path/').pathname is incorrect for file usage (it yields /C:/path/), while fileURLToPath('file:///C:/path/') yields the correct Windows path C:\path\ [1]. - Same idea for UNC: new URL('file://nas/foo.txt').pathname is /foo.txt (not a UNC path), while fileURLToPath('file://nas/foo.txt') yields \nas\foo.txt on Windows [1]. 2) fileURLToPath decodes percent-encoded characters and normalizes - Node’s fileURLToPath “decodes percent-encoded characters … and then normalizes the resulting path” [2]. - Node also rejects encoded slashes/backslashes in the file URL path (encoded forward/back slashes are “correctly rejected”) [2]. - Node warns about security: you should not rely on fileURLToPath alone to prevent directory traversal; validate the returned path before using it for filesystem operations [2]. 3) Explicit Windows/Posix selection - fileURLToPath(url[, options]) supports options.windows: true => treat as Windows rules, false => POSIX rules, undefined => system default [2]. 4) Non-UTF-8 percent-encoded sequences - Node introduced fileURLToPathBuffer because the existing fileURLToPath can fail when the input URL contains percent-encoded bytes that are not valid UTF-8 sequences [3][4]. If you may have legacy encodings (or such bytes), convert to a Buffer instead. 5) If your bug is about pathname with spaces/encoding - A common pitfall is expecting URL.pathname to be decoded/converted for filesystem use. URL.pathname will keep things in URL form (e.g., spaces may remain percent-encoded), whereas fileURLToPath decodes percent encodings as part of the conversion [1][2]. Practical guidance - If you have a file: URL and want a path to pass to fs/path APIs, do: fileURLToPath(urlOrString, { windows: true }) on Windows-targeted paths [2]. - Do not use new URL(...).pathname directly for filesystem operations; it can have the “wrong” leading slash form and won’t yield Windows drive/UNC formats [1]. Note on URL-pathname parsing vs Windows path APIs - Node’s node:path behaves according to the runtime OS, and recommends using path.win32 for Windows-style operations on non-Windows systems [5]. That’s separate from fileURLToPath’s job (converting file: URLs to OS-correct paths).
Citations:
- 1: https://nodejs.org/docs/latest-v23.x/api/url.html
- 2: https://nodejs.org/docs/latest-v24.x/api/url.html
- 3: nodejs/node@ef0230abaf
- 4: url: add fileURLToPathBuffer API nodejs/node#58700
- 5: https://nodejs.org/dist/latest/docs/api/path.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the file around the reported lines
ls -la scripts/build-wystack.mjs
nl -ba scripts/build-wystack.mjs | sed -n '1,140p'
# Confirm how ROOT is used downstream
rg -n "ROOT" scripts/build-wystack.mjs
# Check for rm -f / execSync usage and quoting
rg -n "rm -f|execSync\\(" scripts/build-wystack.mjs
# Check package.json for build/run scripts that imply Windows/Electron targeting
if [ -f package.json ]; then
cat -n package.json | sed -n '1,200p'
rg -n "\"(start|build|pack|electron|wystack)\"|electron|bun x|node " package.json || true
fi
# If present, check for Prettier config / pipeline hints
ls -ლა .prettierrc* package.json 2>/dev/null || true
find . -maxdepth 3 -type f \( -iname "*prettier*" -o -iname "*.prettierrc*" -o -iname ".prettierrc*" \) | head -n 50 || trueRepository: youhaowei/DashFrame
Length of output: 199
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== File listing =="
ls -la scripts/build-wystack.mjs
echo "== First 160 lines with numbers (no nl) =="
cat -n scripts/build-wystack.mjs | sed -n '1,160p'
echo "== ROOT usages =="
rg -n "ROOT" scripts/build-wystack.mjs
echo "== rm -f / execSync occurrences =="
rg -n "rm -f|execSync\\(" scripts/build-wystack.mjs
echo "== Check package.json for Electron/Windows hints =="
if [ -f package.json ]; then
cat -n package.json | sed -n '1,220p'
rg -n "\"(start|build|pack|electron|wystack)\"" package.json || true
rg -n "electron|bun x|wystack|tsc" package.json || true
else
echo "package.json not found"
fi
echo "== Prettier config presence (if any) =="
ls -la .prettierrc* 2>/dev/null || true
ls -la prettier* 2>/dev/null || true
find . -maxdepth 3 -type f \( -iname "*prettier*" -o -iname "*prettierrc*" \) | head -n 50 || trueRepository: youhaowei/DashFrame
Length of output: 8257
new URL(...).pathname is not cross-platform; use fileURLToPath.
const ROOT = new URL('..', import.meta.url).pathname; will produce URL-form paths on Windows (e.g. /C:/... and percent-encoded characters), which can corrupt downstream path.join results.
- Replace
ROOTwithfileURLToPath(same intent, correct path semantics). execSync(\bun x tsc -p ${tmpFile}`)andexecSync(`rm -f ${tmpFile}`)build command strings without quoting. If paths contain spaces, they can break; additionallyrm -fis POSIX-only—usefs.rmSync/unlinkSync` instead.- The “skip if
dist/exists” logic can leave staledistwhen dependencies/inputs change (consider a forced rebuild when appropriate).
♻️ Proposed fix
-import path from 'path';
+import path from 'path';
+import { fileURLToPath } from 'url';
-const ROOT = new URL('..', import.meta.url).pathname;
+const ROOT = fileURLToPath(new URL('..', import.meta.url));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/build-wystack.mjs` at line 21, Replace the platform-unsafe ROOT
assignment by using fileURLToPath(import.meta.url) (import from 'url') to
compute ROOT correctly; replace shell-string execSync calls that build commands
with unquoted paths (e.g., execSync(`bun x tsc -p ${tmpFile}`) and execSync(`rm
-f ${tmpFile}`)) by using child_process.execFileSync or spawnSync with argument
arrays so paths with spaces are handled safely and remove the POSIX-only rm by
calling fs.rmSync or fs.unlinkSync for tmpFile; finally, change the simple “skip
if dist/ exists” logic so it doesn’t silently reuse stale builds (either always
rebuild, add a forced-rebuild flag, or compare timestamps/checksum of inputs
before skipping) and update references to ROOT, tmpFile, and the exec calls
accordingly (look for constants ROOT, variable tmpFile, and the execSync
usages).
| for (const pkg of packages) { | ||
| const distDir = path.join(pkg.dir, 'dist'); | ||
| if (existsSync(distDir)) { | ||
| console.log(`[build-wystack] ${pkg.name}: dist already exists, skipping`); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Stale dist/ after a submodule bump will silently be reused.
The skip-if-dist-exists guard never rebuilds. This PR bumps the libs/wystack submodule, so any machine that previously ran setup will keep serving an outdated dist/ and silently mismatch the new sources. Consider keying the skip on source freshness, or documenting that build:wystack requires a manual dist/ clean after submodule updates.
Want me to add a staleness check (compare newest src mtime vs dist) so changed packages rebuild automatically?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/build-wystack.mjs` around lines 74 - 79, The current loop in
build-wystack.mjs (iterating packages and using distDir/existing guard) can
silently reuse stale dist/, so change the skip logic to compare file mtimes: for
each pkg (pkg.dir), recursively compute the newest mtime under the source tree
(e.g., pkg.dir/src or pkg.dir/**/*.{ts,js,jsx,tsx}) and compare it to the mtime
of distDir (fs.stat). Only skip when distDir exists AND its mtime is newer than
the newest source mtime; otherwise treat it as stale and rebuild; update any
logging to indicate when dist is skipped vs rebuilt. Ensure you reference the
same variables used in the loop (packages, pkg.dir, distDir) when implementing
the check.
| try { | ||
| execSync(`bun x tsc -p ${tmpFile}`, { stdio: 'inherit' }); | ||
| console.log(`[build-wystack] ${pkg.name}: done`); | ||
| } catch (err) { | ||
| console.error(`[build-wystack] ${pkg.name}: build failed`); | ||
| process.exit(1); | ||
| } finally { | ||
| try { execSync(`rm -f ${tmpFile}`); } catch {} | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
rm -f is POSIX-only; use the already-imported fs API.
rm -f (Line 103) fails on Windows, and unquoted ${tmpFile} interpolation (Lines 97, 103) breaks if the path contains spaces. Since fs is already imported, delete the temp file in-process.
♻️ Proposed fix
-import { writeFileSync, mkdirSync } from 'fs';
+import { writeFileSync, rmSync } from 'fs';
@@
try {
- execSync(`bun x tsc -p ${tmpFile}`, { stdio: 'inherit' });
+ execSync(`bun x tsc -p "${tmpFile}"`, { stdio: 'inherit' });
console.log(`[build-wystack] ${pkg.name}: done`);
} catch (err) {
console.error(`[build-wystack] ${pkg.name}: build failed`);
process.exit(1);
} finally {
- try { execSync(`rm -f ${tmpFile}`); } catch {}
+ try { rmSync(tmpFile, { force: true }); } catch {}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| execSync(`bun x tsc -p ${tmpFile}`, { stdio: 'inherit' }); | |
| console.log(`[build-wystack] ${pkg.name}: done`); | |
| } catch (err) { | |
| console.error(`[build-wystack] ${pkg.name}: build failed`); | |
| process.exit(1); | |
| } finally { | |
| try { execSync(`rm -f ${tmpFile}`); } catch {} | |
| } | |
| try { | |
| execSync(`bun x tsc -p "${tmpFile}"`, { stdio: 'inherit' }); | |
| console.log(`[build-wystack] ${pkg.name}: done`); | |
| } catch (err) { | |
| console.error(`[build-wystack] ${pkg.name}: build failed`); | |
| process.exit(1); | |
| } finally { | |
| try { rmSync(tmpFile, { force: true }); } catch {} | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/build-wystack.mjs` around lines 96 - 104, The finally block currently
calls execSync(`rm -f ${tmpFile}`) which is POSIX-only and fails on Windows and
the unquoted ${tmpFile} can break on paths with spaces; change that to use the
imported fs API: call fs.unlinkSync(tmpFile) inside the finally try/catch and
ignore ENOENT (or catch and no-op), and also ensure the earlier execSync
invocation that runs tsc includes the tmpFile path quoted (e.g., wrap ${tmpFile}
in quotes in the command string) so paths with spaces are handled safely;
reference the try/catch/finally block, tmpFile variable, and the execSync calls
to locate and update the code.
DashFrame's first integration with WyStack. Replaces the superseded Electron-IPC attempt (PR #44) with the HTTP+WS-loopback model from the Data Path & Transport Deployment spec: the renderer is a localhost web client of a loopback-bound server — the same client + transport the cloud web client uses. - apps/server: the dedicated server app gains its first real content — a `functions` registry (the tRPC-style API surface) with a `projectInfo` query reading project_meta via WyStack's DrizzleTracker, plus a deployment-agnostic `createDashframeServer(db)` factory. Exports `type Functions` for type-only client consumption. - apps/desktop main: starts the server on 127.0.0.1:0 before the window, exposes the loopback URL over a new `getServerInfo` IPC channel, stops it on quit. - apps/renderer: async bootstrap resolves the URL via IPC, mints the WyStack client once at module scope, and a route round-trips `projectInfo` into an assertable DOM node. - Integration test proves the full open-project → serve → HTTP path. No auth: the loopback token mechanism is an open spec decision, out of scope for this smoke (single-user trunk treats auth as a no-op). Two load-bearing seams established here, both called out deliberately: - Bumps the libs/wystack submodule pointer c2850a5 -> 8a1d4d5 (wystack origin/main HEAD; brings client createWyStack + WS engine). Not Codex's in-flight #35. - Adds build:wystack (runs the submodule's own turbo build) wired into setup — DashFrame now builds the wystack dist as a prerequisite, since this is the first DashFrame code to consume it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
DashFrame's first integration with WyStack. Replaces the superseded Electron-IPC attempt (PR #44) with the HTTP+WS-loopback model from the Data Path & Transport Deployment spec: the renderer is a localhost web client of a loopback-bound server — the same client + transport the cloud web client uses. - apps/server: the dedicated server app gains its first real content — a `functions` registry (the tRPC-style API surface) with a `projectInfo` query reading project_meta via WyStack's DrizzleTracker, plus a deployment-agnostic `createDashframeServer(db)` factory. Exports `type Functions` for type-only client consumption. - apps/desktop main: starts the server on 127.0.0.1:0 before the window, exposes the loopback URL over a new `getServerInfo` IPC channel, stops it on quit. - apps/renderer: async bootstrap resolves the URL via IPC, mints the WyStack client once at module scope, and a route round-trips `projectInfo` into an assertable DOM node. - Integration test proves the full open-project → serve → HTTP path. No auth: the loopback token mechanism is an open spec decision, out of scope for this smoke (single-user trunk treats auth as a no-op). Two load-bearing seams established here, both called out deliberately: - Bumps the libs/wystack submodule pointer c2850a5 -> 8a1d4d5 (wystack origin/main HEAD; brings client createWyStack + WS engine). Not Codex's in-flight #35. - Adds build:wystack (runs the submodule's own turbo build) wired into setup — DashFrame now builds the wystack dist as a prerequisite, since this is the first DashFrame code to consume it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
DashFrame's first integration with WyStack. Replaces the superseded Electron-IPC attempt (PR #44) with the HTTP+WS-loopback model from the Data Path & Transport Deployment spec: the renderer is a localhost web client of a loopback-bound server — the same client + transport the cloud web client uses. - apps/server: the dedicated server app gains its first real content — a `functions` registry (the tRPC-style API surface) with a `projectInfo` query reading project_meta via WyStack's DrizzleTracker, plus a deployment-agnostic `createDashframeServer(db)` factory. Exports `type Functions` for type-only client consumption. - apps/desktop main: starts the server on 127.0.0.1:0 before the window, exposes the loopback URL over a new `getServerInfo` IPC channel, stops it on quit. - apps/renderer: async bootstrap resolves the URL via IPC, mints the WyStack client once at module scope, and a route round-trips `projectInfo` into an assertable DOM node. - Integration test proves the full open-project → serve → HTTP path. No auth: the loopback token mechanism is an open spec decision, out of scope for this smoke (single-user trunk treats auth as a no-op). Two load-bearing seams established here, both called out deliberately: - Bumps the libs/wystack submodule pointer c2850a5 -> 8a1d4d5 (wystack origin/main HEAD; brings client createWyStack + WS engine). Not Codex's in-flight #35. - Adds build:wystack (runs the submodule's own turbo build) wired into setup — DashFrame now builds the wystack dist as a prerequisite, since this is the first DashFrame code to consume it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Summary
buildWyStackApp()registers aprojectInfoquery backed byProjectHandle.meta, thenattachElectronTransport({ app, ipcMain })mounts the WyStack server over thewystack:c2s/wystack:s2cIPC channels. No auth — trusted in-process transport.window.wysIpcexposes a minimalIpcRendererLikesurface scoped to the wystack channels viacontextBridge. AWeakMapbridges original callbacks through context isolation soremoveListenercorrelates correctly.apps/renderer/src/wystack.ts): customWyStackClientover IPC usingcreateElectronPipe+ a hand-rolled call→result correlator (Map<id, {resolve,reject}>). No-opWsManager— non-reactive only (YW-62 gates subscriptions).useProjectInfo()convenience hook usinguseQuery.main.tsx): wraps app inQueryClientProvider + WyStackProvider.useProjectInfo()and renders project name/version from the WyStack response.build:mainnow bundles@wystack/*and transitive deps (drizzle-orm, zod, hono) inline via esbuild, keeping onlyelectron,@dashframe/*,@electric-sql/pglite, andpostgresas externals.turbo.jsonper-package task overrides prevent turbo from re-running wystack builds through the dependency chain. Newscripts/build-wystack.mjs+bun build:wystackbuilds the wystack package dists (excluding test files andserve-bun.ts).Implementation note: The call→result correlator over IPC is hand-rolled in the renderer because
@wystack/clienthas no built-in IPC call transport — the client engine only owns the reactive (subscribe/invalidate) tier. A@wystack/client-side IPC call transport should land as follow-up work.Runtime evidence
Electron main process launched headless; full log:
The
ERR_CONNECTION_REFUSEDis expected — no Vite dev server was running in the headless test environment. The WyStack transport mounts and the window opens successfully. Full app-launch verification with the renderer showing project info requiresbun run dev:desktopwith a display.bun checkresultAll 59 tasks pass.
Test plan
bun run dev:desktop— app launches, renderer shows project name/version in the index routewystack:c2ssend visible on connect;wystack:s2ccarries{type:"result",id:"call_1",data:{name:...,version:...}}[dashframe] WyStack IPC transport mountedvisible in main process logsCloses YW-69
🤖 Generated with Claude Code
Greptile Summary
This PR wires WyStack (the project's custom query engine) over Electron IPC, replacing the previous static "hello" screen with a live
projectInfoquery from the main process. The transport is three-layered: main process registers a WyStack server overwystack:c2s/wystack:s2cIPC channels, the preload bridges them throughcontextBridge, and the renderer runs a hand-rolled call/result correlator wrapped in aWyStackClient-compatible object.buildWyStackAppservesprojectInfofromProjectHandle.metain-memory; the preloadwysIpcbridge uses a channel allowlist and aWeakMapto correlateremoveListenercalls correctly across context isolation.wystack.ts): customWyStackClientover IPC with a per-call correlator (Map<id, {resolve,reject}>); no-opWsManagerstubs subscriptions until YW-62;useProjectInfohook surfaced via TanStack Query.build:mainnow bundles@wystack/*inline; a newscripts/build-wystack.mjspre-builds wystack submodule dists excluding test files; Turbo overrides prevent re-running wystack builds through the dependency chain.Confidence Score: 4/5
Safe to merge for the current smoke-test scope; the IPC wiring is correct and the contextBridge allowlist is properly scoped, but two areas around lifecycle cleanup and the build script are worth addressing before the codebase grows.
The IPC protocol is straightforward and the preload bridge is well-guarded. The main concerns are:
detach()being called twice in the quit sequence (bothbefore-quithandlers fire on the first quit attempt, and again on the second), the hand-rolled correlator having no timeout or send-failure cleanup path, and thebuild-wystack.mjsscript silently skipping staledist/directories and using unquoted paths that would break on space-containing repo roots. None of these cause visible breakage in the happy path, but they are latent issues that could surface during development or on certain developer machines.apps/desktop/src/main.ts(quit-sequencedetachcall ordering),apps/renderer/src/wystack.ts(correlator cleanup), andscripts/build-wystack.mjs(stale-dist skip + unquoted paths).Important Files Changed
buildWyStackApp; thedetach()registration onbefore-quitfires twice during the quit sequence, and theprojectInfodata shape is duplicated fromregisterIpc.window.wysIpcbridge viacontextBridge; channel allowlist and WeakMap-based listener correlation are well-implemented. The null-event wrapper (listener(null, ...rest)) is correctly documented.pendingentries that can never settle.QueryClientProvider+WyStackProvider; straightforward provider composition, no issues.useProjectInfo()to render project name/version; loading/error states are handled correctly.distexistence check silently skips stale builds.build:mainfrom--packages=externalto explicit externals so@wystack/*and transitive deps are bundled; adds@wystack/serveras a workspace dependency.Sequence Diagram
sequenceDiagram participant R as Renderer (wystack.ts) participant PB as Preload Bridge (wysIpc) participant IM as ipcMain (Electron) participant WS as WyStack Server participant PH as ProjectHandle.meta R->>PB: "wysIpc.send("wystack:c2s", {type:"call", id:"call_1", path:"projectInfo"})" PB->>IM: ipcRenderer.send("wystack:c2s", msg) IM->>WS: attachElectronTransport routes call frame WS->>PH: "handler({ db: null }, args)" PH-->>WS: "{ projectId, name, version, ... }" WS-->>IM: "{type:"result", id:"call_1", data:{...}}" IM-->>PB: ipcRenderer "wystack:s2c" event PB-->>R: "listener(null, {type:"result", id:"call_1", data:{...}})" R->>R: pending.get("call_1").resolve(data) R->>R: useProjectInfo() renders project name/versionPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "feat(desktop): wire WyStack server+clien..." | Re-trigger Greptile