fix(preview): keep typed desktop automation errors across IPC

[PixPMusic]

Problem

Desktop Browser automation failures with a public tag reach agents as Preview automation <op> failed on client preview-…. That includes waitFor timeouts, invalid selectors, and non-editable targets. The desktop main process raises typed errors, but ipcMain.handle rejections keep only the message through ipcRenderer.invoke (electron#24427). The renderer then wraps them as PreviewAutomationOperationError, which is reported as PreviewAutomationExecutionError. The broker's classifyResponseError already handles these three tags, but desktop hosts never sent them.

This becomes more visible once #12899 lands. A real desktop's waitFor timeout will then reach the broker and read "failed on client" instead of "timed out". Today the broker's own deadline hides that.

Fixes #15336.

Change

This follows the pattern from #7301 and the direction suggested in #15336's triage. The desktop returns a typed result over IPC instead of rejecting, and the renderer turns it into a host error with the response tag the broker already classifies.

• packages/contracts: DesktopPreviewAutomationFailureSchema defines the three failures, with timeoutMs, selectorKind, and selectorLength. The type, scroll, and waitFor bridge methods now return DesktopPreviewAutomationFailure | undefined.
• apps/desktop: those three IPC methods resolve the three Manager error tags as that payload. Success still returns undefined, and every other failure still rejects.
• apps/web: the host throws a returned failure, and PreviewAutomationOperationError.fromCause maps it to a timeout, invalid-selector, or not-editable host error. That mapping replaces the hand-written not-editable shape check, which never matched an Electron rejection. Unknown failures stay PreviewAutomationExecutionError.
• The server is unchanged. For timeouts, classifyResponseError already reports the caller's timeoutMs, which is what the desktop waited.

Not covered:

• Click: its result type is what fix(preview): name hidden, disabled, and ambiguous click failures #7301 changes, so I left it alone to keep the two PRs independent.
• Press: its only typed failure is an internal 5 s key-delivery timeout, which the broker would misreport against the request's 15 s.
• Target-not-found and out-of-viewport: these have no public tags (noted in [Bug]: Desktop Browser automation errors lose their type over IPC and report as 'failed on client' #15336's triage).

I simulated a 3-way merge with #7301's diff. It produces three small conflicts, each resolved by keeping both sides.

Scope and approval

This is a focused bug fix for #15336, which maintainer triage confirmed with this fix shape suggested. It only changes how already-public error tags cross Electron IPC. It adds no new tags, settings, or behavior beyond reporting the error that already occurred.

Verification

• New tests that fail on main and pass with the fix:
  • apps/desktop/src/ipc/methods/preview.test.ts, "resolves failures that have a public automation tag instead of rejecting". It also checks that success returns undefined and that target-not-found still rejects.
  • previewAutomationRequestConsumer.test.ts, "maps typed failures the desktop resolves over IPC to the public response tags". It covers timeout, invalid selector, and not-editable, and checks that a plain Electron error and a malformed payload stay ExecutionError.
• PreviewAutomationBroker.test.ts, "reports a host waitFor timeout against the caller deadline", pins the agent-visible timeout message. It passes both before and after, because the server is unchanged.
• vp test run for the desktop IPC, web preview, broker, and contracts IPC tests: 60 passed. Typecheck passes for contracts, desktop, web, and server. Lint and format are clean for the changed directories.
• Real desktop (macOS, dev build, single machine): an agent ran preview_open on a local page, then preview_wait_for with a malformed locator (role=button[name=), then preview_type into the page's h1.
  • Before (main @ 8d84666): both calls returned Preview automation waitFor failed on client preview-… and Preview automation type failed on client preview-….
  • After: Preview automation waitFor received an invalid locator (17 characters). and Preview automation type requires an editable locator (2 characters).
  • Screenshots are in the first comment below.
• The timeout path is covered by tests only. On main the broker's own deadline still answers first; that's [Bug]: preview_wait_for still evicts a live host when its miss reply lands at the broker deadline #12898 / fix(server): keep preview hosts that report a waitFor miss at the deadline #12899. With a broker reply grace applied, I observed the real desktop's timeout reply arriving as "failed on client" on two Macs (details on fix(server): keep preview hosts that report a waitFor miss at the deadline #12899).

Model: Claude Opus 5.5 | Harness: Claude Code in T3 Code

[PixPMusic]

Real desktop before/after (macOS dev build, single machine). Same prompt both runs: preview_open a local page, preview_wait_for with a malformed locator (role=button[name=), preview_type into the page's h1.

Before (main @ 8d84666): both calls report failed on client preview-….

After (this PR): received an invalid locator (17 characters) and requires an editable locator (2 characters).

[macroscopeapp]

Approvability

Verdict: Approved at e2a34f2

Macroscope's review found this PR approvable — This is a focused, backward-compatible fix that preserves existing desktop automation error tags across IPC and improves messages for three already-supported failure cases. Success behavior and unrelated failures remain unchanged, with targeted coverage for the new transport and classification paths.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used

📚 Code guidelines (2)

docs/internals/effect-services.md — configured
AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 5729891f-29f4-4952-83f3-3838bcc1ffdb

📥 Commits

Reviewing files that changed from the base of the PR and between 00eb8f6 and e2a34f2.

📒 Files selected for processing (7)

• apps/desktop/src/ipc/methods/preview.test.ts
• apps/desktop/src/ipc/methods/preview.ts
• apps/server/src/mcp/PreviewAutomationBroker.test.ts
• apps/web/src/components/preview/PreviewAutomationHosts.tsx
• apps/web/src/components/preview/previewAutomationErrors.ts
• apps/web/src/components/preview/previewAutomationRequestConsumer.test.ts
• packages/contracts/src/ipc.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

Desktop automation handlers now return typed values for selected failures through IPC. The web layer maps those values to typed host errors for serialization. Other automation errors remain rejected or use the generic error path.

Changes

Automation failure transport

Layer / File(s)	Summary
IPC failure contract and desktop mapping packages/contracts/src/ipc.ts, apps/desktop/src/ipc/methods/preview.ts, apps/desktop/src/ipc/methods/preview.test.ts	The bridge methods for type, scroll, and waitFor now return an optional typed failure. Desktop handlers map timeout, invalid-selector, and target-not-editable errors to tagged values. Tests cover those values and confirm that target-not-found still rejects.
Web error mapping and downstream checks apps/web/src/components/preview/PreviewAutomationHosts.tsx, apps/web/src/components/preview/previewAutomationErrors.ts, apps/web/src/components/preview/previewAutomationRequestConsumer.test.ts, apps/server/src/mcp/PreviewAutomationBroker.test.ts	Web handlers throw returned failures. Error conversion maps recognized desktop failures to typed host errors, and serialization tests verify that tags and details are retained. A broker test checks the caller-deadline message for a host timeout.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Low

Sequence Diagram(s)

sequenceDiagram
  participant DesktopIPC
  participant PreviewAutomationHosts
  participant PreviewAutomationOperationError
  participant serializePreviewAutomationError
  DesktopIPC->>PreviewAutomationHosts: Return tagged automation failure
  PreviewAutomationHosts->>PreviewAutomationOperationError: Throw returned failure
  PreviewAutomationOperationError->>serializePreviewAutomationError: Provide classified host error
  serializePreviewAutomationError-->>PreviewAutomationHosts: Preserve response tag and details

Loading

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to e2a34

No actionable issue was identified in the selected desktop automation failure changes; the PR is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to e2a34

The updated path preserves error categories without adding browser-control privileges. A conditional compatibility risk remains: an older web client could report a returned failure as success when paired with an updated desktop. Supported version combinations have not been established.

Retained concerns

• Low · reliability · inferred: If an older renderer can run against the updated desktop, timeout, invalid-selector, and non-editable failures resolve normally and enter the consumer's ok:true response branch. This weakens automation failure containment: callers may continue despite an unmet operation or wait condition. The current renderer handles these values correctly; whether the affected mixed-version pairing is supported remains unresolved.

Security review details

Security Blast Radius

• inferred — The changed exposure is failure reporting for existing desktop preview automation callers. Effective operation authority continues to follow the existing manager target resolution; the inspected delta adds no operation channel, tenant permission, credential access, or infrastructure authority. This does not establish complete deployment or tenant-isolation coverage.

Trust Boundaries and Controls

• observed — The IPC factory still decodes incoming payloads before manager dispatch and encodes outgoing results against the registered schema. Web error conversion checks the shared failure schema; malformed values and ordinary Electron rejections remain generic execution failures rather than trusted typed diagnostics.

Resilience and Maintainability Implications

• observed — The inspected consumer independently attributes concurrent responses and filters obsolete connection generations. It does not itself deduplicate request IDs or cancel already-started handlers on disposal. That consumer is unchanged in the actual PR comparison, so these limitations are preexisting rather than introduced by typed failure transport; external replay and cancellation guarantees remain unresolved.

Hardening Proposals

• proposed — Make renderer-desktop version coupling explicit. If mixed versions are supported, negotiate the typed-result capability or gate its use so older consumers continue receiving rejected failures instead of resolved failure values.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	[#15336] The PR transports timeout, invalid-selector, and not-editable failures for type, scroll, and waitFor. The new desktop and web tests cover those results. The issue also names click as …	Extend IPC transport and renderer mapping for the public-tagged failures supported by click, and add tests that verify the tags reach the broker. Keep failures without public response tags as rejections.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 7 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: preserving typed desktop automation errors across IPC.
Description check	✅ Passed	The description covers the required Problem, Change, Scope and approval, and Verification sections. It explains the bug, fix, scope limits, tests, and observed results.
Out of Scope Changes check	✅ Passed	The contracts schema, desktop IPC changes, renderer mapping, and tests support [#15336]. The broker test verifies the user-visible timeout classification for the same fix. No unrelated changes appear …

Full details: Linked Issues check

Explanation

[#15336] The PR transports timeout, invalid-selector, and not-editable failures for type, scroll, and waitFor. The new desktop and web tests cover those results. The issue also names click as an affected automation operation. automationClick remains unchanged with a Schema.Void result, so its typed failures still reject across IPC and lose their tags.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[juliusmarminge]

Note

Grok responding on behalf of Julius.

Thanks for the careful work here, @PixPMusic. I'm closing this as superseded: #15336 was closed as obsolete because, since #15328, preview automation no longer runs in the desktop app. Errors don't cross Electron IPC anymore, and the server host returns the typed errors (PreviewAutomationTimeoutError, PreviewAutomationInvalidSelectorError, PreviewAutomationTargetNotEditableError) directly. If you still see failed on client instead of a typed error on current main, please open a fresh issue with a trace and we'll take another look.