fix(preview): report navigation timeout accurately#24356

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: 2c592a1
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: 682d0cee-c2da-4a49-9ba2-3b8314306303

Reviewing files that changed from the base of the PR and between a135e59d9a9c59e4ee53c619b2c9ca5ed50eb4d9 and 76d14e2e69209c4aadb6050928e277e66e61642f.

• apps/server/src/preview/ServerBrowser.test.ts

Explanation

The title "fix(preview): report navigation timeout accurately" matches the main change. The diff makes the surfaced timeout for initial and reopened tab navigation reflect the host navigation timeout (15s) instead of the broker's larger request budget (660000ms). The title is a conventional commit with a clear scope and a specific description. It is 50 characters, within a normal length.

Explanation

Issue #24278 is the only directly linked issue. Its coding requirement is that preview_open report the host navigation timeout, or include the host message, instead of the broker's 660000ms request budget. The whole-PR summary shows that ServerBrowserPage.toOperationError now attaches { timeoutMs } to initial and explicit navigation timeouts, and that the broker classifies remote PreviewAutomationTimeoutError details with the remote timeoutMs. The reviewed head file apps/server/src/mcp/PreviewAutomationBroker.ts confirms readRemoteTimeoutMs accepts only positive integers and that the timeout error uses Math.min(context.timeoutMs, remoteTimeoutMs). The regression test in apps/server/src/preview/ServerBrowser.test.ts covers the new-tab page.goto timeout and expects timeoutMs 15000 with the 15000ms message. The issue's alternative suggestions (a caller timeoutMs or a longer wait) are optional directions, not required criteria. The PR description states that navigation timing is unchanged. Limits: the text diff tool was unavailable for the changed blobs, so the ServerBrowser and test changes rely on the supplied whole-PR summary. The test run was not repeated.

Explanation

The whole-PR summary lists changes in four files: PreviewAutomationBroker.ts, ServerBrowser.ts, ServerBrowserPage.ts, and ServerBrowser.test.ts. All changes concern timeout detail propagation for navigation failures, broker timeout classification, and the matching regression test. These changes support issue #24278. The PR description's branch-status and model notes are not code changes. No unrelated implementation or test changes are identified. The incremental change from the previous reviewed head a135e59 to 76d14e2 was not separately diffed because the diff tool was unavailable. The whole-PR evidence and the head broker file show no unrelated change.

Explanation

The description follows the required template sections (Problem, Change, Scope and approval, Verification). It states the bug clearly: preview_open reports 660000ms when Playwright stops initial navigation after 15000ms. It explains the change: the operation-error detail carries the navigation timeout, and the broker caps it at its request budget with a fallback. It links the issue (#24278) and says it came through the triaged automated flow. Verification is specific: a red-then-green regression test, a focused vp test run with 52 passing tests, lint and tsc results, and a before/after runtime table on Linux with timings. It also notes what was not checked: macOS, Windows, mobile, cold Vite runs, and ServerBrowserPage.test.ts. The Scope section cites the template's exemption for a very small, focused fix of an obvious bug. The template asks for an explicit maintainer approval link, but the author says no maintainer comment exists in the preceding 24 hours, so approval is not established by a link. That gap is minor and does not block the description, so the check passes.

• Create a new PR