Repository navigation
Add host:window and host:ipc SDK type declarations - #2
Conversation
Reviewer's GuideIntroduce a new ext_window crate and host:window/host:ipc SDK surface while refactoring existing UI/IPC responsibilities: window management is delegated to a new WindowCmd pipeline and tao/wry event handlers; IPC is extracted into ext_ipc with its own state and capabilities; UI extension is slimmed to dialogs/menus/tray; host.d.ts gains typed host:window and host:ipc modules; and TypeScript-based init/preload scripts are transpiled at build time for both ext_ui and ext_window. Sequence diagram for the new host:window window creation flowsequenceDiagram
actor App
participant SDK as HostWindowSDK
participant Core as DenoCoreOps
participant ExtWin as ext_window
participant Host as ForgeHost
participant EL as TaoEventLoop
participant TaoWin as TaoWindow_WebView
App->>SDK: createWindow(options)
SDK->>Core: op_window_create(WindowOptions)
Core->>ExtWin: op_window_create(WindowOpts)
ExtWin->>Host: send WindowCmd Create(opts, respond)
Host->>EL: proxy.send_event(UserEvent WinCreate)
EL->>TaoWin: WindowBuilder.build + WebViewBuilder.build
EL-->>Host: Created window id
Host-->>ExtWin: respond.send(Ok(window_id))
ExtWin-->>Core: window_id
Core-->>SDK: window_id
SDK-->>App: Window handle with id
Note over EL,ExtWin: On success, Host also sends WindowSystemEvent create on CH_win_events
Class diagram for ext_window and ext_ipc core typesclassDiagram
%% ext_window data types
class WindowOpts {
+Option String url
+Option u32 width
+Option u32 height
+Option String title
+Option bool resizable
+Option bool decorations
+Option bool visible
+Option bool transparent
+Option bool always_on_top
+Option i32 x
+Option i32 y
+Option u32 min_width
+Option u32 min_height
+Option u32 max_width
+Option u32 max_height
+Option Vec~String~ channels
}
class Position {
+i32 x
+i32 y
}
class Size {
+u32 width
+u32 height
}
class NativeHandle {
+String platform
+u64 handle
}
class WindowSystemEvent {
+String window_id
+String event_type
+serde_json::Value payload
}
class WindowState {
+bool is_visible
+bool is_focused
+bool is_fullscreen
+bool is_maximized
+bool is_minimized
+bool is_resizable
+bool has_decorations
+bool is_always_on_top
}
class FileFilter {
+String name
+Vec~String~ extensions
}
class FileDialogOpts {
+Option String title
+Option String default_path
+Option Vec~FileFilter~ filters
+Option bool multiple
+Option bool directory
}
class MessageDialogOpts {
+Option String title
+String message
+Option String kind
+Option Vec~String~ buttons
}
class MenuItem {
+Option String id
+String label
+Option String accelerator
+Option bool enabled
+Option bool checked
+Option Vec~MenuItem~ submenu
+Option String item_type
}
class TrayOpts {
+Option String icon
+Option String tooltip
+Option Vec~MenuItem~ menu
}
class MenuEventWin {
+String menu_id
+String item_id
+String label
}
%% WindowCmd command pipeline
class WindowCmd {
}
<<enumeration>> WindowCmd
WindowCmd : Create(opts, respond)
WindowCmd : Close(window_id, respond)
WindowCmd : Minimize(window_id)
WindowCmd : Maximize(window_id)
WindowCmd : Unmaximize(window_id)
WindowCmd : Restore(window_id)
WindowCmd : SetFullscreen(window_id, fullscreen)
WindowCmd : Focus(window_id)
WindowCmd : GetPosition(window_id, respond)
WindowCmd : SetPosition(window_id, x, y)
WindowCmd : GetSize(window_id, respond)
WindowCmd : SetSize(window_id, width, height)
WindowCmd : GetTitle(window_id, respond)
WindowCmd : SetTitle(window_id, title)
WindowCmd : SetResizable(window_id, resizable)
WindowCmd : SetDecorations(window_id, decorations)
WindowCmd : SetAlwaysOnTop(window_id, always_on_top)
WindowCmd : SetVisible(window_id, visible)
WindowCmd : GetState(window_id, respond)
WindowCmd : ShowOpenDialog(opts, respond)
WindowCmd : ShowSaveDialog(opts, respond)
WindowCmd : ShowMessageDialog(opts, respond)
WindowCmd : SetAppMenu(items, respond)
WindowCmd : ShowContextMenu(window_id, items, respond)
WindowCmd : CreateTray(opts, respond)
WindowCmd : UpdateTray(tray_id, opts, respond)
WindowCmd : DestroyTray(tray_id, respond)
WindowCmd : GetNativeHandle(window_id, respond)
class WindowState2 {
+mpsc::Sender~WindowCmd~ cmd_tx
+Rc RefCell Option mpsc::Receiver~WindowSystemEvent~ events_rx
+Rc RefCell Option mpsc::Receiver~MenuEventWin~ menu_events_rx
}
class WindowCapabilityChecker {
+check_windows() Result<(), String>
+check_menus() Result<(), String>
+check_dialogs() Result<(), String>
+check_tray() Result<(), String>
+check_native_handle() Result<(), String>
}
<<interface>> WindowCapabilityChecker
class PermissiveWindowChecker {
}
class WindowCapabilities {
+Arc~dyn WindowCapabilityChecker~ checker
}
PermissiveWindowChecker ..|> WindowCapabilityChecker
WindowCapabilities --> WindowCapabilityChecker
WindowState2 --> WindowCmd
WindowState2 --> WindowSystemEvent
WindowState2 --> MenuEventWin
%% ext_ipc core types
class IpcEvent {
+String window_id
+String channel
+serde_json::Value payload
+Option String event_type
}
class ToRendererCmd {
}
<<enumeration>> ToRendererCmd
ToRendererCmd : Send(window_id, channel, payload)
class IpcState {
+mpsc::Sender~ToRendererCmd~ to_renderer_tx
+Rc RefCell Option mpsc::Receiver~IpcEvent~ to_deno_rx
}
class IpcCapabilityChecker {
+check_channel(channel, window_channels) Result<(), String>
}
<<interface>> IpcCapabilityChecker
class PermissiveIpcChecker {
}
class IpcCapabilities {
+Arc~dyn IpcCapabilityChecker~ checker
}
PermissiveIpcChecker ..|> IpcCapabilityChecker
IpcCapabilities --> IpcCapabilityChecker
IpcState --> ToRendererCmd
IpcState --> IpcEvent
%% Relationships to host capabilities
class CapabilityAdapters {
+Arc~dyn ext_ui::UiCapabilityChecker~ ui
+Arc~dyn ext_window::WindowCapabilityChecker~ window
+Arc~dyn ext_ipc::IpcCapabilityChecker~ ipc
}
CapabilityAdapters --> WindowCapabilityChecker
CapabilityAdapters --> IpcCapabilityChecker
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The host:window and host:ipc logic is currently implemented twice (in
sdk/*.tsandcrates/ext_*/ts/init.tswith near-identical code); consider refactoring to share a single source (e.g., generate the SDK from the ext_* TS or vice versa) to avoid divergence and reduce maintenance overhead. - On Linux,
WinGetNativeHandlereturns a hard-coded handle value of 0 with a generic "linux" platform string; if this is intentional as a placeholder it would be helpful to either gate it behind a capability / feature flag or clearly document that this handle is not usable so callers do not assume it is a real native handle. - The new
WindowState2type inext_windowis a bit confusingly named alongsideWindowState; renaming it to something likeWindowRuntimeState(or similar) would make its purpose clearer and reduce the chance of mixing it up with the user-facing window state struct.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The host:window and host:ipc logic is currently implemented twice (in `sdk/*.ts` and `crates/ext_*/ts/init.ts` with near-identical code); consider refactoring to share a single source (e.g., generate the SDK from the ext_* TS or vice versa) to avoid divergence and reduce maintenance overhead.
- On Linux, `WinGetNativeHandle` returns a hard-coded handle value of 0 with a generic "linux" platform string; if this is intentional as a placeholder it would be helpful to either gate it behind a capability / feature flag or clearly document that this handle is not usable so callers do not assume it is a real native handle.
- The new `WindowState2` type in `ext_window` is a bit confusingly named alongside `WindowState`; renaming it to something like `WindowRuntimeState` (or similar) would make its purpose clearer and reduce the chance of mixing it up with the user-facing window state struct.
## Individual Comments
### Comment 1
<location> `sdk/host.d.ts:519-521` </location>
<code_context>
+ }
+
+ /** Native window handle (platform-specific) */
+ export interface NativeHandle {
+ /** Platform type: "windows", "macos", "linux-x11", "linux-wayland" */
+ platform: string;
+ /** Raw handle value (HWND, NSView*, X11 window, etc.) */
+ handle: bigint;
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** NativeHandle.handle is typed as bigint in TS but implemented as a number in Rust, which can cause precision and interop issues.
Rust serializes `NativeHandle.handle` as a `u64` via Serde, which JS deserializes as a `number`, but the TS type is `bigint` and related code (`host.window.ts`, `ext_window/ts/init.ts`) also treats it as `bigint`. This mismatch means callers think they have a `BigInt` but actually receive a double, so large handles (>2^53) will be imprecise.
Please either:
- change the TS type to `number`, or
- serialize the handle as a string in Rust and type it as `bigint | string` (or `string`) with explicit conversion on the TS side.
That way the representation is consistent and avoids subtle precision bugs with handle values.
</issue_to_address>
### Comment 2
<location> `crates/ext_ipc/src/lib.rs:129-130` </location>
<code_context>
+ }
+}
+
+/// Wrapper to store the capability checker in OpState
+pub struct IpcCapabilities {
+ pub checker: Arc<dyn IpcCapabilityChecker>,
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** IPC capability checker is initialized but never consulted in op_ipc_send/recv, so channel allowlists are effectively ignored.
`IpcCapabilities` is stored in `OpState` but never read by `op_ipc_send`/`op_ipc_recv`, so channel allowlists (e.g. `WindowOpts.channels`) are only enforced at the WebView → host boundary, not Deno → renderer. Either hook `IpcCapabilities` into these ops (e.g. a `check_ipc_channel(state: &OpState, channel, window_channels)` helper called before send/recv) or remove the unused wiring to avoid implying these checks are applied when they are not.
</issue_to_address>
### Comment 3
<location> `crates/forge-host/src/main.rs:730` </location>
<code_context>
+ WindowCmd::ShowSaveDialog { opts, respond } => {
+ let _ = proxy.send_event(UserEvent::WinShowSaveDialog(opts, respond));
+ }
+ WindowCmd::ShowMessageDialog { opts, respond } => {
+ let _ = proxy.send_event(UserEvent::WinShowMessageDialog(opts, respond));
+ }
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Message dialog implementation partially ignores the `buttons` option and always maps to built-in button sets, which may not match SDK expectations.
This diverges from the `host.window.ts` types, which suggest arbitrary button labels with indices corresponding to the provided array. Callers may rely on that contract, leading to confusing behavior (e.g., labels not shown, more than two buttons silently collapsed to a built‑in set).
If you intend to keep using only `rfd::MessageButtons`, consider either documenting that only 1–2 non-customizable buttons are supported (and constraining `opts.buttons` in the TS layer), or, if true custom labels are required, switching to a mechanism that supports fully custom button sets instead of the built-in presets.
Suggested implementation:
```rust
WindowCmd::ShowSaveDialog { opts, respond } => {
let _ = proxy.send_event(UserEvent::WinShowSaveDialog(opts, respond));
}
// NOTE: Message dialogs currently use rfd::MessageButtons presets and do not honor
// arbitrary button labels from `opts.buttons`. Only 1–2 non-customizable buttons
// are effectively supported; callers should not rely on custom labels here.
WindowCmd::ShowMessageDialog { opts, respond } => {
let _ = proxy.send_event(UserEvent::WinShowMessageDialog(opts, respond));
}
```
To fully align behavior with the TypeScript types and your review comment, consider:
1. Updating the `host.window.ts` (or equivalent) to document that only 1–2 non-customizable buttons are supported for message dialogs, or to narrow the `buttons` type accordingly.
2. Optionally adding validation/warnings on the TS side when more than two buttons or custom labels are provided, so callers get early feedback instead of silent degradation.
3. If full custom button support is desired, refactoring the Rust-side message dialog implementation to use a custom dialog mechanism (rather than `rfd::MessageButtons`) that can render arbitrary labels and return the selected index.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| /// Wrapper to store the capability checker in OpState | ||
| pub struct IpcCapabilities { |
There was a problem hiding this comment.
issue (bug_risk): IPC capability checker is initialized but never consulted in op_ipc_send/recv, so channel allowlists are effectively ignored.
IpcCapabilities is stored in OpState but never read by op_ipc_send/op_ipc_recv, so channel allowlists (e.g. WindowOpts.channels) are only enforced at the WebView → host boundary, not Deno → renderer. Either hook IpcCapabilities into these ops (e.g. a check_ipc_channel(state: &OpState, channel, window_channels) helper called before send/recv) or remove the unused wiring to avoid implying these checks are applied when they are not.
- Add sdk/host.window.ts with full TypeScript implementation for window operations - Add sdk/generated/host.window.d.ts for editor intellisense - Add host:window and host:ipc module declarations to sdk/host.d.ts - Include code review fixes: NativeHandle.handle typed as number (not bigint), documented message dialog button limitations, documented Linux native handle placeholder
8de192f to
7c9d9fc
Compare
Summary
This PR adds SDK type declarations for the new
host:windowandhost:ipcmodules:sdk/host.window.ts- Full TypeScript implementation for window operationssdk/generated/host.window.d.ts- Generated declarations for editor intellisensesdk/host.d.ts- Addhost:windowandhost:ipcmodule declarationsCode Review Fixes Applied
NativeHandle.handletyped asnumber(notbigint) - Rust u64 serializes to JS numberNote
The
ext_windowcrate itself was already merged in PR #1. This PR only adds the SDK/type declaration files that provide TypeScript support.