refactor: shared webview IPC helpers with enforced exhaustiveness

[EhabY]

• Shared extension-side dispatch helpers (isIpcCommand, isIpcRequest, notifyWebview, dispatchCommand, dispatchRequest, onWhileVisible) live in src/webviews/dispatch.ts; HTML scaffolding (getWebviewHtml, getWebviewAssetUris, buildWebviewCsp) in src/webviews/html.ts. All three panels dispatch through them.
• Shared vanilla-webview helpers sendCommand and subscribeNotifications in @repo/webview-shared. subscribeNotifications is exhaustive over the API's notifications: a new defineNotification without a handler is a build error. Speedtest and chat use it; React webviews keep useIpc.
• Every panel calls both buildCommandHandlers and buildRequestHandlers (empty {} is fine). New defineCommand / defineRequest entries fail the build at any panel missing a handler, so the extension can't silently drop a future action.
• Chat panel migrated off its ad-hoc {type, ...} protocol onto ChatApi. The iframe shim was extracted into a typed bundle at packages/chat/. A single source-gated window.message listener bridges the iframe's {type, payload} contract and ChatApi; iframe messages can never reach typed notification handlers.
• packages/webview-shared/README.md rewritten as a navigation map with a "Where each handler lives" table and pointers to the canonical reference packages. CONTRIBUTING.md and AGENTS.md point at it as the single source of truth.

[EhabY]

/coder-agents-review

[coder-agents-review]

Solid infrastructure PR. The buildCommandHandlers / buildRequestHandlers pattern is a strong approach to eliminating silently-dropped messages at compile time. The README is well-written reference documentation with the error-semantics table, concrete code examples, and the "no dropped events" explanation. The chat shim decomposition into named functions (toIframe / handleFromIframe / handleFromExtension / showRetry) is a clear improvement over the monolithic handler. The migration is systematic: all three panels moved to the shared helpers, tests updated to match the new wire format.

1 P0, 1 P2, 11 P3, 1 P4, 4 Nit.

The P0 is a regression: the speedtest ready command calls window.postMessage (DOM global) instead of the VS Code IPC because the postMessage import was removed but one call site was not migrated. The chart is blank on initial open. Fix: sendCommand(SpeedtestApi.ready).

The P2 is architectural: the chat shim's switch statements are raw JS inside a template literal, outside the exhaustiveness guarantee. Adding a ChatApi entry compiles cleanly, but the shim silently drops the new message. Seven reviewers flagged this independently.

Most P3s are documentation accuracy (dead file path, missing scaffolding steps, WebviewPanel vs WebviewView), one behavioral default change (speedtest error toasts), one code duplication (useIpc.command vs sendCommand), and test coverage gaps.

PS. Mafu-san's process audit found zero unverified claims in the PR description, all six structural claims hold against the diff. Pariston tried four framings and could not construct a simpler alternative. Takumi traced every concurrency path and found no issues. Ging-TS had no TypeScript modernization findings.

"This also violates the PR's own AGENTS.md rule: 'Never hand-roll ... postMessage({ method, params }). Use onNotification / sendCommand." (Melody)

---

packages/speedtest/src/index.ts:32

P0 [DEREM-4] The postMessage import was removed (line 2), but this call site was not migrated. postMessage now resolves to the DOM global window.postMessage(message, options?), which TypeScript compiles without error (the DOM lib provides the overload). At runtime, window.postMessage({method:"speedtest/ready"}) dispatches a MessageEvent on the webview's own window, not to the extension host. The extension's ready handler (the only path that calls sendData() on initial open) never fires. The chart is blank until the user switches tabs and back.

"Confirmed in the built artifact: sendCommand is inlined as o(...) / s(...), but line 32 emits bare postMessage(...), a global call." (Mafuuu)

Fix: sendCommand(SpeedtestApi.ready);

🤖

packages/webview-shared/src/react/useIpc.ts:119

P3 [DEREM-11] useIpc.command is a line-for-line copy of sendCommand from ipc.ts. Both take a CommandDef<P>, spread variadic args, and call postMessage({ method: def.method, params: args[0] }). The onNotification path was already migrated to delegate to the shared helper (line 146: return subscribe(definition, callback)). The symmetric command path was not. One-line fix: import { sendCommand } from "../ipc" and delegate. (Robin)

🤖

test/unit/webviews/speedtest/speedtestPanelFactory.test.ts:128

P4 [DEREM-16] "ignores unknown message methods" no longer matches actual behavior. The old code silently dropped unknown methods. The new code routes through dispatchCommand, which throws, logs a warning, and pops showErrorMessage (default showErrorToUser: () => true). The test asserts .not.toThrow() synchronously on an async dispatch, so the error toast is invisible. The test name is misleading and the assertion is vacuous for this path. (Bisky)

🤖

🤖 This review was automatically generated with Coder Agents.

[EhabY]

/coder-agents-review

[coder-agents-review]

17 of 20 R1 findings addressed. The author fixed the P0, the P2, all P3s with inline threads, and all Nits. Thorough response.

Two findings remain unaddressed. Both were auto-folded into the review body (their lines fell outside the diff context), so the author had no threaded comment to reply to. Listing them here so they have a reply target.

DEREM-11 (P3) packages/webview-shared/src/react/useIpc.ts:119: useIpc.command is a line-for-line copy of sendCommand from ipc.ts. The onNotification path was already migrated to delegate to the shared helper (line 146: return subscribe(definition, callback)). The symmetric command path was not. One-line fix: import { sendCommand } from "../ipc" and delegate.

DEREM-16 (P4) test/unit/webviews/speedtest/speedtestPanelFactory.test.ts:128: "ignores unknown message methods" test name is misleading. After the dispatch migration, unknown commands throw internally, get caught, and log a warning (no longer silently ignored). With the default flip from DEREM-8 (55ad7ce), the user-facing behavior is closer to the old silent behavior (no error toast), but the internal path changed. The test assertion (.not.toThrow()) is vacuous because it checks synchronously against an async dispatch.

Further review is blocked until these are responded to (fix, acknowledge, or contest). The panel has not yet reviewed the new commits.

🤖 This review was automatically generated with Coder Agents.

[EhabY]

Pushed 699e733 addressing the two remaining R1 findings:

• DEREM-11: useIpc.command now delegates to sendCommand. As a side benefit, the React side picks up the void-command params-omission fix from DEREM-20.
• DEREM-16: renamed ignores unknown message methods to does not surface an error dialog for unknown command methods and replaced the vacuous .not.toThrow() (sync against an async dispatch) with an awaited assertion that vscode.window.showErrorMessage was not called.

[EhabY]

/coder-agents-review

[coder-agents-review]

All 20 R1 findings verified fixed. The author's response was thorough: the P0 speedtest bug, the P2 shim exhaustiveness gap, all P3s, and all Nits were addressed across three commits. Melody walked every API pairing end-to-end and confirmed all chains agree. Mafuuu verified the four highest-severity fixes against the code and ran adversarial scenarios. The dispatch.test.ts and auth-retry tests are genuine and well-structured.

The fix commits introduced significant refactoring beyond the R1 fixes: util.ts split into dispatch.ts + html.ts, chat shim extracted into packages/chat/, subscribeNotifications added for compile-checked notification handling, dispatchCommand default flipped. The panel reviewed these changes.

1 P2, 3 P3, 2 Nit (1 from Netero, 1 new).

The P2 is a fix-chain regression from DEREM-5: subscribeNotifications in the chat shim is source-blind. The old inline script had a single message listener with event.source gating that ensured notification handlers only processed extension messages. The extraction into two independent listeners removed that invariant. If the Coder embed ever posts a message with a notification type string, the handler crashes with TypeError.

"If the Coder server embed ever posts a message with type: 'coder:set-theme', listener 1 fires, extracts (msg as { data }).data (which is undefined), and the handler ({ theme }) => toIframe(...) throws TypeError." (Hisoka)

🤖 This review was automatically generated with Coder Agents.

[jakehwll]

LGTM 🙂