diff --git a/AGENTS.md b/AGENTS.md index 752eb913..97d31775 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -79,6 +79,21 @@ test/ └── mocks/ # Shared test mocks ``` +## Webviews + +When adding or modifying a panel, follow `packages/webview-shared/README.md`. +It is the single source of truth for the IPC contract, exhaustive handler +maps, and the visibility/theme re-send guarantee. + +Non-negotiables: + +- Never hand-roll `window.addEventListener("message", ...)` or + `postMessage({ method, params })`. Use `onNotification` / `sendCommand` + (vanilla) or `useIpc` (React) from `@repo/webview-shared`. +- Extension panels must call **both** `buildCommandHandlers` and + `buildRequestHandlers` (empty `{}` is fine). This gives a compile error + when anyone adds an action to the API without a matching handler. + ## Code Style - TypeScript with strict typing diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f81a2f06..32b8b5c0 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -74,28 +74,18 @@ that are close to shutting down. ## Webviews -The extension uses React-based webviews for rich UI panels, built with Vite and -organized as a pnpm workspace in `packages/`. +The extension ships rich UI panels as webviews built with Vite, organized as a +pnpm workspace in `packages/`. The canonical guide for building one covers +the IPC contract, exhaustiveness rules, the "no dropped events" guarantee, +and a new-panel checklist. It lives next to the code: -### Project Structure +**[`packages/webview-shared/README.md`](packages/webview-shared/README.md)** -```text -packages/ -├── webview-shared/ # Shared types, React hooks, and Vite config -│ └── extension.d.ts # Types exposed to extension (excludes React) -└── tasks/ # Example webview (copy this for new webviews) - -src/webviews/ -├── util.ts # getWebviewHtml() helper -└── tasks/ # Extension-side provider for tasks panel -``` - -Key patterns: +Existing webviews as references: -- **Type sharing**: Extension imports types from `@repo/webview-shared` via path mapping - to `extension.d.ts`. Webviews import directly from `@repo/webview-shared/react`. -- **Message passing**: Use `postMessage()`/`useMessage()` hooks for communication. -- **Lifecycle**: Dispose event listeners properly (see `TasksPanel.ts` for example). +- `packages/tasks` + `src/webviews/tasks/`: React (uses `useIpc`). +- `packages/speedtest` + `src/webviews/speedtest/`: vanilla TS (uses + `onNotification` / `sendCommand`). ### Development @@ -103,15 +93,8 @@ Key patterns: pnpm watch # Rebuild extension and webviews on changes ``` -Press F5 to launch the Extension Development Host. Use "Developer: Reload Webviews" -to see webview changes. - -### Adding a New Webview - -1. Copy `packages/tasks` to `packages/` and update the package name -2. Create a provider in `src/webviews//` (see `TasksPanel.ts` for reference) -3. Register the view in `package.json` under `contributes.views` -4. Register the provider in `src/extension.ts` +Press F5 to launch the Extension Development Host. Use "Developer: Reload +Webviews" to see webview changes. ## Testing diff --git a/packages/shared/src/chat/api.ts b/packages/shared/src/chat/api.ts new file mode 100644 index 00000000..47e2b74e --- /dev/null +++ b/packages/shared/src/chat/api.ts @@ -0,0 +1,20 @@ +import { defineCommand, defineNotification } from "../ipc/protocol"; + +/** The chat webview embeds an iframe that speaks to the Coder server. */ +export const ChatApi = { + /** Iframe reports it needs the session token. */ + vscodeReady: defineCommand("coder:vscode-ready"), + /** Iframe reports the chat UI has rendered. */ + chatReady: defineCommand("coder:chat-ready"), + /** Iframe requests an external navigation; same-origin only. */ + navigate: defineCommand<{ url: string }>("coder:navigate"), + + /** Push the current theme into the iframe. */ + setTheme: defineNotification<{ theme: "light" | "dark" }>("coder:set-theme"), + /** Push the session token to bootstrap iframe auth. */ + authBootstrapToken: defineNotification<{ token: string }>( + "coder:auth-bootstrap-token", + ), + /** Signal that auth could not be obtained. */ + authError: defineNotification<{ error: string }>("coder:auth-error"), +} as const; diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 8f9ccef3..b403822e 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -16,3 +16,6 @@ export { type SpeedtestInterval, type SpeedtestResult, } from "./speedtest/api"; + +// Chat API +export { ChatApi } from "./chat/api"; diff --git a/packages/speedtest/src/index.ts b/packages/speedtest/src/index.ts index bcc71364..65a54584 100644 --- a/packages/speedtest/src/index.ts +++ b/packages/speedtest/src/index.ts @@ -1,5 +1,5 @@ import { SpeedtestApi, type SpeedtestResult, toError } from "@repo/shared"; -import { postMessage, subscribeNotification } from "@repo/webview-shared"; +import { onNotification, sendCommand } from "@repo/webview-shared"; import { renderLineChart } from "./chart"; import { @@ -18,11 +18,11 @@ const TOOLTIP_GAP_PX = 32; let cleanup: (() => void) | undefined; function main(): void { - subscribeNotification(SpeedtestApi.data, ({ workspaceName, result }) => { + onNotification(SpeedtestApi.data, ({ workspaceName, result }) => { try { cleanup?.(); cleanup = renderPage(result, workspaceName, () => - postMessage({ method: SpeedtestApi.viewJson.method }), + sendCommand(SpeedtestApi.viewJson), ); } catch (err) { showError(`Failed to render speedtest: ${toError(err).message}`); diff --git a/packages/webview-shared/README.md b/packages/webview-shared/README.md new file mode 100644 index 00000000..2f20e1ce --- /dev/null +++ b/packages/webview-shared/README.md @@ -0,0 +1,137 @@ +# Webview IPC + +Shared primitives for typed, reliable messaging between the extension host +and VS Code webviews. Both sides use the **same `Api` definition object** so +wire formats can't drift and method-name typos are caught at compile time. + +## Message kinds + +```ts +// packages/shared/src/ipc/protocol.ts +defineNotification(method); // extension to webview, fire-and-forget +defineCommand

(method); // webview to extension, fire-and-forget +defineRequest(method); // webview to extension, awaits response +``` + +Define all three together in one `Api` const so both sides import the exact +same method strings: + +```ts +// packages/shared/src/myfeature/api.ts +export const MyFeatureApi = { + data: defineNotification("myfeature/data"), + doThing: defineCommand<{ id: string }>("myfeature/doThing"), + getThings: defineRequest("myfeature/getThings"), +} as const; +``` + +## Extension side (`src/webviews/...`) + +### Sending notifications + +```ts +import { notifyWebview } from "../util"; + +notifyWebview(panel.webview, MyFeatureApi.data, payload); +``` + +### Handling incoming messages (exhaustiveness) + +Every panel **must** build handler maps with `buildCommandHandlers` and +`buildRequestHandlers`, even if it has zero requests today. Both builders +are mapped over the `Api` definition, so adding a new `defineCommand` or +`defineRequest` entry **produces a compile error** at the panel that forgot +to wire a handler. This makes it impossible to ship an API surface that +the extension silently drops. + +```ts +import { buildCommandHandlers, buildRequestHandlers } from "@repo/shared"; +import { + dispatchCommand, + dispatchRequest, + isIpcCommand, + isIpcRequest, +} from "../util"; + +const commandHandlers = buildCommandHandlers(MyFeatureApi, { + doThing: async (p) => { ... }, +}); +// Empty is fine; it still enforces future additions. +const requestHandlers = buildRequestHandlers(MyFeatureApi, { + getThings: async () => this.fetchThings(), +}); + +panel.webview.onDidReceiveMessage((message: unknown) => { + if (isIpcRequest(message)) { + void dispatchRequest(message, requestHandlers, panel.webview, { + logger, + showErrorToUser: (m) => USER_ACTION_METHODS.has(m), + }); + } else if (isIpcCommand(message)) { + void dispatchCommand(message, commandHandlers, { logger }); + } +}); +``` + +**Error semantics** (built into `dispatchCommand` / `dispatchRequest`): + +| | Logs | Response sent? | `showErrorMessage` default | +| ------------- | ------------- | ----------------------------- | -------------------------- | +| Command fails | `logger.warn` | n/a | **yes** (user action) | +| Request fails | `logger.warn` | `success: false` with `error` | **no** (often background) | + +Pass `showErrorToUser: (method) => …` to override per-method. + +## Webview side, vanilla (`@repo/webview-shared`) + +```ts +import { MyFeatureApi } from "@repo/shared"; +import { onNotification, sendCommand } from "@repo/webview-shared"; + +// Subscribe to pushes. +const unsubscribe = onNotification(MyFeatureApi.data, (payload) => { + render(payload); +}); + +// Fire a command. +sendCommand(MyFeatureApi.doThing, { id: "42" }); +``` + +`onNotification` returns an unsubscribe function; call it on cleanup. + +## Webview side, React + +Use `useIpc` (`packages/webview-shared/src/react/useIpc`). Same semantics +plus request/response correlation with timeout and UUID bookkeeping. + +## The "no dropped events" guarantee + +Webview contexts are **destroyed when hidden** unless +`retainContextWhenHidden` is set (costly; avoid). This means the webview's +in-memory state, event listeners, and canvas pixels are lost whenever the +user switches tabs. + +Every webview that pushes state from the extension must therefore **re-send +on these signals**, so the revived webview isn't left empty or stale: + +1. **Visibility change**: `panel.onDidChangeViewState(() => panel.visible && resend())`. + The webview was just re-created from HTML, so no in-memory state remains. +2. **Color theme change**: `vscode.window.onDidChangeActiveColorTheme(() => panel.visible && resend())`. + DOM elements update via CSS vars, but canvas and SVG drawn imperatively + do not: pixels are baked in. The webview must redraw against the new + theme. + +The `onWhileVisible(panel, event, handler)` helper in `src/webviews/util.ts` +wraps both cases. Both disposables must be collected and disposed in +`panel.onDidDispose` so they don't leak when the user closes the tab. + +See `src/webviews/speedtest/speedtestPanel.ts` for a minimal reference. + +## Checklist for a new webview + +1. Define the API in `packages/shared/src//api.ts` and export from `packages/shared/src/index.ts`. +2. Extension side: `buildCommandHandlers` **and** `buildRequestHandlers` (empty `{}` is fine; both are needed for exhaustiveness). +3. Extension side: dispatch through `isIpcRequest` to `dispatchRequest` and `isIpcCommand` to `dispatchCommand`, both with a logger. +4. Extension side: use `onWhileVisible` for `onDidChangeViewState` and `onDidChangeActiveColorTheme`, dispose in `onDidDispose`. +5. Webview side: use `onNotification` / `sendCommand` (vanilla) or `useIpc` (React). Never hand-roll `window.addEventListener("message", ...)`. +6. Tests: verify the panel posts the expected payload shape, re-sends on visibility and theme, and handles incoming commands. diff --git a/packages/webview-shared/src/index.ts b/packages/webview-shared/src/index.ts index 334b27df..01d83d22 100644 --- a/packages/webview-shared/src/index.ts +++ b/packages/webview-shared/src/index.ts @@ -8,5 +8,5 @@ export interface WebviewMessage { // VS Code state API export { getState, setState, postMessage } from "./api"; -// Notification subscription for non-React webviews -export { subscribeNotification } from "./notifications"; +// Typed IPC helpers for vanilla webviews +export { sendCommand, onNotification } from "./ipc"; diff --git a/packages/webview-shared/src/ipc.ts b/packages/webview-shared/src/ipc.ts new file mode 100644 index 00000000..34f171ee --- /dev/null +++ b/packages/webview-shared/src/ipc.ts @@ -0,0 +1,43 @@ +/** + * Typed IPC helpers for vanilla-TS webviews. React webviews should use + * `useIpc` (./react/useIpc), which adds request/response correlation. + */ + +import { postMessage } from "./api"; + +import type { CommandDef, NotificationDef } from "@repo/shared"; + +/** Send a fire-and-forget command to the extension. */ +export function sendCommand

( + def: CommandDef

, + ...args: P extends void ? [] : [params: P] +): void { + postMessage({ + method: def.method, + params: args[0], + }); +} + +/** + * Subscribe to a typed notification from the extension. Returns an + * unsubscribe function; call it on cleanup. Multiple subscribers are + * invoked in registration order. + */ +export function onNotification( + def: NotificationDef, + callback: (data: D) => void, +): () => void { + const handler = (event: MessageEvent) => { + const msg = event.data; + if ( + typeof msg !== "object" || + msg === null || + (msg as { type?: unknown }).type !== def.method + ) { + return; + } + callback((msg as { data: D }).data); + }; + window.addEventListener("message", handler); + return () => window.removeEventListener("message", handler); +} diff --git a/packages/webview-shared/src/notifications.ts b/packages/webview-shared/src/notifications.ts deleted file mode 100644 index e71877b6..00000000 --- a/packages/webview-shared/src/notifications.ts +++ /dev/null @@ -1,20 +0,0 @@ -import type { NotificationDef } from "@repo/shared"; - -/** Subscribe to an extension notification. Returns an unsubscribe function. */ -export function subscribeNotification( - def: NotificationDef, - callback: (data: D) => void, -): () => void { - const handler = (event: MessageEvent) => { - const msg = event.data as { type?: string; data?: D } | undefined; - if (!msg || typeof msg !== "object") { - return; - } - if (msg.type !== def.method || msg.data === undefined) { - return; - } - callback(msg.data); - }; - window.addEventListener("message", handler); - return () => window.removeEventListener("message", handler); -} diff --git a/packages/webview-shared/src/react/useIpc.ts b/packages/webview-shared/src/react/useIpc.ts index 9fe994b3..c6c0d0ed 100644 --- a/packages/webview-shared/src/react/useIpc.ts +++ b/packages/webview-shared/src/react/useIpc.ts @@ -6,7 +6,7 @@ import { useEffect, useRef } from "react"; import { postMessage } from "../api"; -import { subscribeNotification } from "../notifications"; +import { onNotification as subscribe } from "../ipc"; import type { CommandDef, @@ -61,7 +61,7 @@ export function useIpc(options: UseIpcOptions = {}) { }, []); // Request/response correlation lives here. Notifications are routed via - // the shared subscribeNotification helper (see onNotification below). + // the shared onNotification helper (see the method below). useEffect(() => { const handler = (event: MessageEvent) => { const msg = event.data as IpcResponse | undefined; @@ -143,7 +143,7 @@ export function useIpc(options: UseIpcOptions = {}) { definition: NotificationDef, callback: (data: D) => void, ): () => void { - return subscribeNotification(definition, callback); + return subscribe(definition, callback); } return { request, command, onNotification }; diff --git a/src/webviews/chat/chatPanelProvider.ts b/src/webviews/chat/chatPanelProvider.ts index 3b7f48bd..252291c4 100644 --- a/src/webviews/chat/chatPanelProvider.ts +++ b/src/webviews/chat/chatPanelProvider.ts @@ -1,22 +1,36 @@ import * as vscode from "vscode"; +import { + buildCommandHandlers, + buildRequestHandlers, + ChatApi, + type NotificationDef, +} from "@repo/shared"; + import { type CoderApi } from "../../api/coderApi"; import { type Logger } from "../../logging/logger"; -import { getNonce } from "../util"; +import { + dispatchCommand, + dispatchRequest, + getNonce, + isIpcCommand, + isIpcRequest, + notifyWebview, +} from "../util"; /** * Provides a webview that embeds the Coder agent chat UI. * Authentication flows through postMessage: * * 1. The iframe loads /agents/{id}/embed on the Coder server. - * 2. The embed page detects the user is signed out and sends + * 2. The embed page detects the user is signed out and posts * { type: "coder:vscode-ready" } to window.parent. - * 3. Our webview relays this to the extension host. - * 4. The extension host replies with the session token. - * 5. The webview forwards { type: "coder:vscode-auth-bootstrap" } - * with the token back into the iframe. - * 6. The embed page calls API.setSessionToken(token), re-fetches - * the authenticated user, and renders the chat UI. + * 3. Our shim relays that to the extension as a ChatApi.vscodeReady + * command. + * 4. The extension pushes the session token back with + * ChatApi.authBootstrapToken. + * 5. The shim forwards it into the iframe, which calls + * API.setSessionToken and re-fetches the user. */ export class ChatPanelProvider implements vscode.WebviewViewProvider, vscode.Disposable @@ -28,6 +42,13 @@ export class ChatPanelProvider private chatId: string | undefined; private authRetryTimer: ReturnType | undefined; + private readonly commandHandlers = buildCommandHandlers(ChatApi, { + vscodeReady: () => this.sendAuthToken(), + chatReady: () => this.sendTheme(), + navigate: ({ url }) => this.handleNavigate(url), + }); + private readonly requestHandlers = buildRequestHandlers(ChatApi, {}); + constructor( private readonly client: Pick, private readonly logger: Logger, @@ -41,11 +62,17 @@ export class ChatPanelProvider : "dark"; } + private notify( + def: NotificationDef, + ...args: D extends void ? [] : [data: D] + ): void { + if (this.view) { + notifyWebview(this.view.webview, def, ...args); + } + } + private sendTheme(): void { - this.view?.webview.postMessage({ - type: "coder:set-theme", - theme: this.getTheme(), - }); + this.notify(ChatApi.setTheme, { theme: this.getTheme() }); } /** @@ -75,8 +102,20 @@ export class ChatPanelProvider this.view = webviewView; webviewView.webview.options = { enableScripts: true }; this.disposables.push( - webviewView.webview.onDidReceiveMessage((msg: unknown) => { - this.handleMessage(msg); + webviewView.webview.onDidReceiveMessage((message: unknown) => { + if (isIpcRequest(message)) { + void dispatchRequest( + message, + this.requestHandlers, + webviewView.webview, + { logger: this.logger }, + ); + } else if (isIpcCommand(message)) { + void dispatchCommand(message, this.commandHandlers, { + logger: this.logger, + showErrorToUser: () => false, + }); + } }), vscode.window.onDidChangeActiveColorTheme(() => { this.sendTheme(); @@ -114,38 +153,19 @@ export class ChatPanelProvider webview.html = this.getIframeHtml(embedUrl, coderUrl); } - private handleMessage(message: unknown): void { - if (typeof message !== "object" || message === null) { + private handleNavigate(url: string): void { + const coderUrl = this.client.getHost(); + if (!url || !coderUrl) { return; } - const msg = message as { type?: string; payload?: { url?: string } }; - switch (msg.type) { - case "coder:vscode-ready": - this.sendAuthToken(); - break; - case "coder:chat-ready": - this.sendTheme(); - break; - case "coder:navigate": { - const url = msg.payload?.url; - const coderUrl = this.client.getHost(); - if (url && coderUrl) { - try { - const resolved = new URL(url, coderUrl); - const expected = new URL(coderUrl); - if (resolved.origin === expected.origin) { - void vscode.env.openExternal( - vscode.Uri.parse(resolved.toString()), - ); - } - } catch { - this.logger.warn(`Chat: invalid navigate URL: ${url}`); - } - } - break; + try { + const resolved = new URL(url, coderUrl); + const expected = new URL(coderUrl); + if (resolved.origin === expected.origin) { + void vscode.env.openExternal(vscode.Uri.parse(resolved.toString())); } - default: - break; + } catch { + this.logger.warn(`Chat: invalid navigate URL: ${url}`); } } @@ -178,17 +198,13 @@ export class ChatPanelProvider "Chat iframe requested auth but no session token available " + "after all retries", ); - this.view?.webview.postMessage({ - type: "coder:auth-error", + this.notify(ChatApi.authError, { error: "No session token available. Please sign in and retry.", }); return; } this.logger.info("Chat: forwarding token to iframe"); - this.view?.webview.postMessage({ - type: "coder:auth-bootstrap-token", - token, - }); + this.notify(ChatApi.authBootstrapToken, { token }); } private getIframeHtml(embedUrl: string, allowedOrigin: string): string { @@ -246,53 +262,72 @@ export class ChatPanelProvider status.style.display = 'none'; }); - window.addEventListener('message', (event) => { - const data = event.data; - if (!data || typeof data !== 'object') return; + // Shim sits between two wire formats. The iframe speaks + // { type, payload } (a contract owned by the Coder server). + // The extension speaks the IPC protocol: commands are + // { method, params } and notifications are { type, data }. + // See packages/webview-shared/README.md. + const toIframe = (type, payload) => { + iframe.contentWindow.postMessage({ type, payload }, '${allowedOrigin}'); + }; - if (event.source === iframe.contentWindow) { - if (data.type === 'coder:vscode-ready') { - status.textContent = 'Authenticating…'; - vscode.postMessage({ type: 'coder:vscode-ready' }); - } - if (data.type === 'coder:chat-ready') { - vscode.postMessage({ type: 'coder:chat-ready' }); - } - if (data.type === 'coder:navigate') { - vscode.postMessage(data); - } - return; - } + const showRetry = (error) => { + status.textContent = ''; + status.appendChild(document.createTextNode(error || 'Authentication failed.')); + const btn = document.createElement('button'); + btn.id = 'retry-btn'; + btn.textContent = 'Retry'; + btn.addEventListener('click', () => { + status.textContent = 'Authenticating…'; + vscode.postMessage({ method: 'coder:vscode-ready' }); + }); + status.appendChild(document.createElement('br')); + status.appendChild(btn); + status.style.display = 'block'; + iframe.style.display = 'none'; + }; - if (data.type === 'coder:auth-bootstrap-token') { - status.textContent = 'Signing in…'; - iframe.contentWindow.postMessage({ - type: 'coder:vscode-auth-bootstrap', - payload: { token: data.token }, - }, '${allowedOrigin}'); + const handleFromIframe = (msg) => { + switch (msg.type) { + case 'coder:vscode-ready': + status.textContent = 'Authenticating…'; + vscode.postMessage({ method: 'coder:vscode-ready' }); + return; + case 'coder:chat-ready': + vscode.postMessage({ method: 'coder:chat-ready' }); + return; + case 'coder:navigate': + vscode.postMessage({ + method: 'coder:navigate', + params: { url: msg.payload?.url }, + }); + return; } + }; - if (data.type === 'coder:set-theme') { - iframe.contentWindow.postMessage({ - type: 'coder:set-theme', - payload: { theme: data.theme }, - }, '${allowedOrigin}'); + const handleFromExtension = (msg) => { + const data = msg.data || {}; + switch (msg.type) { + case 'coder:auth-bootstrap-token': + status.textContent = 'Signing in…'; + toIframe('coder:vscode-auth-bootstrap', { token: data.token }); + return; + case 'coder:set-theme': + toIframe('coder:set-theme', { theme: data.theme }); + return; + case 'coder:auth-error': + showRetry(data.error); + return; } + }; - if (data.type === 'coder:auth-error') { - status.textContent = ''; - status.appendChild(document.createTextNode(data.error || 'Authentication failed.')); - const btn = document.createElement('button'); - btn.id = 'retry-btn'; - btn.textContent = 'Retry'; - btn.addEventListener('click', () => { - status.textContent = 'Authenticating…'; - vscode.postMessage({ type: 'coder:vscode-ready' }); - }); - status.appendChild(document.createElement('br')); - status.appendChild(btn); - status.style.display = 'block'; - iframe.style.display = 'none'; + window.addEventListener('message', (event) => { + const msg = event.data; + if (!msg || typeof msg !== 'object') return; + if (event.source === iframe.contentWindow) { + handleFromIframe(msg); + } else { + handleFromExtension(msg); } }); })(); diff --git a/src/webviews/speedtest/speedtestPanelFactory.ts b/src/webviews/speedtest/speedtestPanelFactory.ts index be1e87c9..26503ac9 100644 --- a/src/webviews/speedtest/speedtestPanelFactory.ts +++ b/src/webviews/speedtest/speedtestPanelFactory.ts @@ -2,12 +2,20 @@ import * as vscode from "vscode"; import { buildCommandHandlers, - type SpeedtestData, + buildRequestHandlers, SpeedtestApi, type SpeedtestResult, } from "@repo/shared"; -import { getWebviewHtml } from "../util"; +import { + dispatchCommand, + dispatchRequest, + getWebviewHtml, + isIpcCommand, + isIpcRequest, + notifyWebview, + onWhileVisible, +} from "../util"; import type { Logger } from "../../logging/logger"; @@ -56,21 +64,17 @@ export class SpeedtestPanelFactory { ); // Webview JS is discarded when hidden (no retainContextWhenHidden), and - // the canvas caches theme colors into pixels, so we re-send on visibility - // or theme change to rehydrate and redraw. - const payload: SpeedtestData = { workspaceName, result }; + // canvas pixels bake in theme colors. Re-send on visibility and theme + // changes so the chart rehydrates. const sendData = () => - panel.webview.postMessage({ - type: SpeedtestApi.data.method, - data: payload, + notifyWebview(panel.webview, SpeedtestApi.data, { + workspaceName, + result, }); - const sendIfVisible = () => { - if (panel.visible) { - sendData(); - } - }; sendData(); + // Both builders emit a compile error if any command or request in the + // API lacks a handler here; the empty `{}` below is still load-bearing. const commandHandlers = buildCommandHandlers(SpeedtestApi, { async viewJson() { const doc = await vscode.workspace.openTextDocument({ @@ -80,23 +84,25 @@ export class SpeedtestPanelFactory { await vscode.window.showTextDocument(doc, vscode.ViewColumn.Beside); }, }); + const requestHandlers = buildRequestHandlers(SpeedtestApi, {}); + const logger = this.logger; const disposables: vscode.Disposable[] = [ - panel.onDidChangeViewState(sendIfVisible), - vscode.window.onDidChangeActiveColorTheme(sendIfVisible), - panel.webview.onDidReceiveMessage( - (message: { method: string; params?: unknown }) => { - const handler = commandHandlers[message.method]; - if (handler) { - Promise.resolve(handler(message.params)).catch((err: unknown) => { - this.logger.error( - "Unhandled error in speedtest message handler", - err, - ); - }); - } - }, + onWhileVisible(panel, panel.onDidChangeViewState, sendData), + onWhileVisible( + panel, + vscode.window.onDidChangeActiveColorTheme, + sendData, ), + panel.webview.onDidReceiveMessage((message: unknown) => { + if (isIpcRequest(message)) { + void dispatchRequest(message, requestHandlers, panel.webview, { + logger, + }); + } else if (isIpcCommand(message)) { + void dispatchCommand(message, commandHandlers, { logger }); + } + }), ]; panel.onDidDispose(() => { for (const d of disposables) { diff --git a/src/webviews/tasks/tasksPanelProvider.ts b/src/webviews/tasks/tasksPanelProvider.ts index 6680e857..e7ce3679 100644 --- a/src/webviews/tasks/tasksPanelProvider.ts +++ b/src/webviews/tasks/tasksPanelProvider.ts @@ -12,8 +12,6 @@ import { isStableTask, TasksApi, type CreateTaskParams, - type IpcRequest, - type IpcResponse, type NotificationDef, type TaskDetails, type TaskLogs, @@ -27,10 +25,16 @@ import { streamAgentLogs, streamBuildLogs, } from "../../api/workspace"; -import { toError } from "../../error/errorUtils"; import { type Logger } from "../../logging/logger"; import { vscodeProposed } from "../../vscodeProposed"; -import { getWebviewHtml } from "../util"; +import { + dispatchCommand, + dispatchRequest, + getWebviewHtml, + isIpcCommand, + isIpcRequest, + notifyWebview, +} from "../util"; import type { Preset, @@ -48,31 +52,6 @@ function getTaskBuildUrl(baseUrl: string, task: Task): string { return `${baseUrl}/tasks/${task.owner_name}/${task.id}`; } -/** Check if message is a request (has requestId) */ -function isIpcRequest(msg: unknown): msg is IpcRequest { - return ( - typeof msg === "object" && - msg !== null && - "requestId" in msg && - typeof (msg as IpcRequest).requestId === "string" && - "method" in msg && - typeof (msg as IpcRequest).method === "string" - ); -} - -/** Check if message is a command (has method but no requestId) */ -function isIpcCommand( - msg: unknown, -): msg is { method: string; params?: unknown } { - return ( - typeof msg === "object" && - msg !== null && - !("requestId" in msg) && - "method" in msg && - typeof (msg as { method: string }).method === "string" - ); -} - export class TasksPanelProvider implements vscode.WebviewViewProvider, vscode.Disposable { @@ -183,53 +162,15 @@ export class TasksPanelProvider private async handleMessage(message: unknown): Promise { if (isIpcRequest(message)) { - await this.handleRequest(message); + await dispatchRequest(message, this.requestHandlers, this.view?.webview, { + logger: this.logger, + showErrorToUser: (method) => + TasksPanelProvider.USER_ACTION_METHODS.has(method), + }); } else if (isIpcCommand(message)) { - await this.handleCommand(message); - } - } - - private async handleRequest(message: IpcRequest): Promise { - const { requestId, method, params } = message; - - try { - const handler = this.requestHandlers[method]; - if (!handler) { - throw new Error(`Unknown method: ${method}`); - } - const data = await handler(params); - this.sendResponse({ requestId, method, success: true, data }); - } catch (err) { - const errorMessage = toError(err).message; - this.logger.warn(`Request ${method} failed`, err); - this.sendResponse({ - requestId, - method, - success: false, - error: errorMessage, + await dispatchCommand(message, this.commandHandlers, { + logger: this.logger, }); - if (TasksPanelProvider.USER_ACTION_METHODS.has(method)) { - vscode.window.showErrorMessage(errorMessage); - } - } - } - - private async handleCommand(message: { - method: string; - params?: unknown; - }): Promise { - const { method, params } = message; - - try { - const handler = this.commandHandlers[method]; - if (!handler) { - throw new Error(`Unknown command: ${method}`); - } - await handler(params); - } catch (err) { - const errorMessage = toError(err).message; - this.logger.warn(`Command ${method} failed`, err); - vscode.window.showErrorMessage(errorMessage); } } @@ -581,18 +522,13 @@ export class TasksPanelProvider } } - private sendResponse(response: IpcResponse): void { - this.view?.webview.postMessage(response); - } - private notify( def: NotificationDef, ...args: D extends void ? [] : [data: D] ): void { - this.view?.webview.postMessage({ - type: def.method, - ...(args.length > 0 ? { data: args[0] } : {}), - }); + if (this.view) { + notifyWebview(this.view.webview, def, ...args); + } } dispose(): void { diff --git a/src/webviews/util.ts b/src/webviews/util.ts index edb73fb9..dff8598a 100644 --- a/src/webviews/util.ts +++ b/src/webviews/util.ts @@ -1,6 +1,11 @@ import { randomBytes } from "node:crypto"; import * as vscode from "vscode"; +import { toError } from "../error/errorUtils"; +import { type Logger } from "../logging/logger"; + +import type { IpcRequest, IpcResponse, NotificationDef } from "@repo/shared"; + /** * Get the HTML content for a webview. */ @@ -45,3 +50,128 @@ export function getWebviewHtml( export function getNonce(): string { return randomBytes(16).toString("base64"); } + +export function isIpcRequest(msg: unknown): msg is IpcRequest { + return ( + typeof msg === "object" && + msg !== null && + "requestId" in msg && + typeof (msg as IpcRequest).requestId === "string" && + "method" in msg && + typeof (msg as IpcRequest).method === "string" + ); +} + +export function isIpcCommand( + msg: unknown, +): msg is { method: string; params?: unknown } { + return ( + typeof msg === "object" && + msg !== null && + !("requestId" in msg) && + "method" in msg && + typeof (msg as { method: string }).method === "string" + ); +} + +/** Push a typed notification to a webview. */ +export function notifyWebview( + webview: vscode.Webview, + def: NotificationDef, + ...args: D extends void ? [] : [data: D] +): void { + webview.postMessage({ + type: def.method, + ...(args.length > 0 ? { data: args[0] } : {}), + }); +} + +export interface DispatchOptions { + logger: Logger; + /** + * Predicate run after a handler throws. If it returns true the error is + * also shown via `showErrorMessage`. Defaults: commands show, requests + * don't (see `dispatchCommand` / `dispatchRequest`). + */ + showErrorToUser?: (method: string) => boolean; +} + +/** + * Dispatch a fire-and-forget command. On failure, logs a warning and (by + * default) shows the error to the user. + */ +export async function dispatchCommand( + message: { method: string; params?: unknown }, + handlers: Record void | Promise>, + options: DispatchOptions, +): Promise { + const { method, params } = message; + const showToUser = options.showErrorToUser ?? (() => true); + try { + const handler = handlers[method]; + if (!handler) { + throw new Error(`Unknown command: ${method}`); + } + await handler(params); + } catch (err) { + const errorMessage = toError(err).message; + options.logger.warn(`Command ${method} failed`, err); + if (showToUser(method)) { + vscode.window.showErrorMessage(errorMessage); + } + } +} + +/** + * Dispatch a request and post a typed response back to the webview. On + * failure, logs a warning, posts an error response, and (if + * `showErrorToUser(method)` returns true) shows the error to the user; + * default is not to show. + * + * If `webview` is undefined (e.g. the view was disposed mid-flight) the + * response is dropped silently. + */ +export async function dispatchRequest( + message: IpcRequest, + handlers: Record Promise>, + webview: vscode.Webview | undefined, + options: DispatchOptions, +): Promise { + const { requestId, method, params } = message; + const showToUser = options.showErrorToUser ?? (() => false); + const respond = (response: IpcResponse) => { + void webview?.postMessage(response); + }; + try { + const handler = handlers[method]; + if (!handler) { + throw new Error(`Unknown method: ${method}`); + } + const data = await handler(params); + respond({ requestId, method, success: true, data }); + } catch (err) { + const errorMessage = toError(err).message; + options.logger.warn(`Request ${method} failed`, err); + respond({ requestId, method, success: false, error: errorMessage }); + if (showToUser(method)) { + vscode.window.showErrorMessage(errorMessage); + } + } +} + +/** + * Fire `handler` on `event` only while `panel.visible` is true. Use for + * re-send-on-wake-up signals (visibility, theme). See + * `packages/webview-shared/README.md`. + */ +export function onWhileVisible( + panel: { readonly visible: boolean }, + event: vscode.Event, + handler: () => void, +): vscode.Disposable { + return event(() => { + if (panel.visible) { + handler(); + } + }); +} diff --git a/test/unit/webviews/chat/chatPanelProvider.test.ts b/test/unit/webviews/chat/chatPanelProvider.test.ts index 97fa5a88..d93ea01c 100644 --- a/test/unit/webviews/chat/chatPanelProvider.test.ts +++ b/test/unit/webviews/chat/chatPanelProvider.test.ts @@ -92,11 +92,11 @@ describe("ChatPanelProvider", () => { windowMock.__setActiveColorThemeKind(kind); const { sendFromWebview, postMessage } = createHarness(); - sendFromWebview({ type: "coder:chat-ready" }); + sendFromWebview({ method: "coder:chat-ready" }); expect(findPostedMessage(postMessage, "coder:set-theme")).toEqual({ type: "coder:set-theme", - theme: expected, + data: { theme: expected }, }); }); @@ -108,7 +108,7 @@ describe("ChatPanelProvider", () => { expect(postMessage).toHaveBeenCalledWith({ type: "coder:set-theme", - theme: "light", + data: { theme: "light" }, }); }); }); @@ -117,13 +117,13 @@ describe("ChatPanelProvider", () => { it("sends auth token on coder:vscode-ready", () => { const { sendFromWebview, postMessage } = createHarness(); - sendFromWebview({ type: "coder:vscode-ready" }); + sendFromWebview({ method: "coder:vscode-ready" }); expect( findPostedMessage(postMessage, "coder:auth-bootstrap-token"), ).toEqual({ type: "coder:auth-bootstrap-token", - token: "test-token", + data: { token: "test-token" }, }); }); }); @@ -133,8 +133,8 @@ describe("ChatPanelProvider", () => { const { sendFromWebview } = createHarness(); sendFromWebview({ - type: "coder:navigate", - payload: { url: "/templates" }, + method: "coder:navigate", + params: { url: "/templates" }, }); expect(vscode.env.openExternal).toHaveBeenCalledWith( @@ -145,7 +145,7 @@ describe("ChatPanelProvider", () => { it("ignores navigate without url payload", () => { const { sendFromWebview } = createHarness(); - sendFromWebview({ type: "coder:navigate" }); + sendFromWebview({ method: "coder:navigate", params: {} }); expect(vscode.env.openExternal).not.toHaveBeenCalled(); }); @@ -154,8 +154,8 @@ describe("ChatPanelProvider", () => { const { sendFromWebview } = createHarness(); sendFromWebview({ - type: "coder:navigate", - payload: { url: "https://evil.com/steal" }, + method: "coder:navigate", + params: { url: "https://evil.com/steal" }, }); expect(vscode.env.openExternal).not.toHaveBeenCalled(); diff --git a/test/webview/shared/ipc.test.ts b/test/webview/shared/ipc.test.ts new file mode 100644 index 00000000..32130b06 --- /dev/null +++ b/test/webview/shared/ipc.test.ts @@ -0,0 +1,120 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { defineCommand, defineNotification } from "@repo/shared"; +import { onNotification, sendCommand } from "@repo/webview-shared/ipc"; + +interface Sent { + method: string; + params?: unknown; +} + +const sent: Sent[] = []; + +vi.stubGlobal( + "acquireVsCodeApi", + vi.fn(() => ({ + postMessage: (msg: Sent) => sent.push(msg), + getState: () => undefined, + setState: () => undefined, + })), +); + +beforeEach(() => { + sent.length = 0; +}); + +describe("sendCommand", () => { + it("posts {method, params}", () => { + const cmd = defineCommand<{ id: string }>("ns/doThing"); + sendCommand(cmd, { id: "42" }); + + expect(sent).toEqual([{ method: "ns/doThing", params: { id: "42" } }]); + }); + + it("posts without params for void-payload commands", () => { + const cmd = defineCommand("ns/noop"); + sendCommand(cmd); + + expect(sent).toEqual([{ method: "ns/noop", params: undefined }]); + }); +}); + +describe("onNotification", () => { + it("invokes callback only for matching method", () => { + const def = defineNotification<{ count: number }>("ns/updated"); + const cb = vi.fn(); + + const unsubscribe = onNotification(def, cb); + + window.dispatchEvent( + new MessageEvent("message", { + data: { type: "ns/other", data: { count: 1 } }, + }), + ); + expect(cb).not.toHaveBeenCalled(); + + window.dispatchEvent( + new MessageEvent("message", { + data: { type: "ns/updated", data: { count: 7 } }, + }), + ); + expect(cb).toHaveBeenCalledWith({ count: 7 }); + + unsubscribe(); + }); + + it("stops receiving events after unsubscribe", () => { + const def = defineNotification("ns/ping"); + const cb = vi.fn(); + + const unsubscribe = onNotification(def, cb); + unsubscribe(); + + window.dispatchEvent( + new MessageEvent("message", { + data: { type: "ns/ping", data: "hello" }, + }), + ); + + expect(cb).not.toHaveBeenCalled(); + }); + + it("ignores non-object messages", () => { + const def = defineNotification("ns/evt"); + const cb = vi.fn(); + const unsubscribe = onNotification(def, cb); + + window.dispatchEvent(new MessageEvent("message", { data: null })); + window.dispatchEvent(new MessageEvent("message", { data: "string" })); + window.dispatchEvent(new MessageEvent("message", { data: 42 })); + + expect(cb).not.toHaveBeenCalled(); + unsubscribe(); + }); + + it("supports multiple independent subscribers", () => { + const def = defineNotification("ns/count"); + const a = vi.fn(); + const b = vi.fn(); + + const unsubA = onNotification(def, a); + const unsubB = onNotification(def, b); + + window.dispatchEvent( + new MessageEvent("message", { data: { type: "ns/count", data: 1 } }), + ); + + expect(a).toHaveBeenCalledWith(1); + expect(b).toHaveBeenCalledWith(1); + + unsubA(); + window.dispatchEvent( + new MessageEvent("message", { data: { type: "ns/count", data: 2 } }), + ); + + expect(a).toHaveBeenCalledTimes(1); + expect(b).toHaveBeenCalledTimes(2); + + unsubB(); + }); +});