From e186f87079d52e1e8a7f0d96b9dfe2566d16d5f5 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Wed, 9 Sep 2026 11:56:26 +0200 Subject: [PATCH 1/4] docs(preview-test): document embedded viewer verification harness Adds a reference for testing code in src/webui/quarto-preview/src/frame/ that only runs inside an embedded viewer (RStudio Viewer, VS Code Simple Browser, Posit Workbench) - a plain top-level preview never exercises it. Covers the iframe + postMessage host page pattern, a throwaway reverse proxy for reproducing a proxied deployment's browser/server origin mismatch, the server's sticky first-request client-injection detection, and the precondition assertion needed before trusting any click result. --- .claude/skills/quarto-preview-test/SKILL.md | 7 + .../references/embedded-viewer.md | 120 ++++++++++++++++++ 2 files changed, 127 insertions(+) create mode 100644 .claude/skills/quarto-preview-test/references/embedded-viewer.md diff --git a/.claude/skills/quarto-preview-test/SKILL.md b/.claude/skills/quarto-preview-test/SKILL.md index 1267cd99f33..1e9b4a62f06 100644 --- a/.claude/skills/quarto-preview-test/SKILL.md +++ b/.claude/skills/quarto-preview-test/SKILL.md @@ -169,6 +169,13 @@ Invoked with test IDs (e.g. `/quarto-preview-test T17 T18`) or a topic descripti matching tests from `tests/docs/manual/preview/README.md`. Invoked without IDs or a topic, use the Edit-Verify Cycle above instead — the test matrix is for targeted regression testing. +## Embedded Viewer (iframe / postMessage) + +When testing link classification, viewer `postMessage` events, or code in +`src/webui/quarto-preview/src/frame/`, use the iframe and proxy harness in +`references/embedded-viewer.md`. It covers the first-request constraint and the required +precondition; a top-level preview does not exercise this code. + ## Baseline Comparison Compare dev build against installed release to distinguish regressions: diff --git a/.claude/skills/quarto-preview-test/references/embedded-viewer.md b/.claude/skills/quarto-preview-test/references/embedded-viewer.md new file mode 100644 index 00000000000..7a4a6bd14be --- /dev/null +++ b/.claude/skills/quarto-preview-test/references/embedded-viewer.md @@ -0,0 +1,120 @@ +# Embedded Viewer Verification (iframe / postMessage) + +Use this harness for link classification, viewer `postMessage` events, and other code in +`src/webui/quarto-preview/src/frame/` that runs only in an embedded viewer (RStudio Viewer, +VS Code Simple Browser, or Posit Workbench). A top-level browser preview does not exercise it. + +## Why a harness is required + +- `handleExternalLinks` (and similar embedded-viewer logic) early-returns when + `window.self === window.top`. It only runs inside an iframe. +- The client script injects embedded-viewer options only if the server's *first* HTML request + includes `quartoPreviewReqId=`, `capabilities=`, or `vscodeBrowserReqId=` in its URL or + referrer (`viewerIFrameURL` in `src/core/http-devserver.ts`). `injectClientInitialized` + memoizes the result for the process lifetime; later requests do not trigger detection again. + +## The host page pattern + +Use a parent page that collects the viewer script's `postMessage` events and embeds the preview +URL with a marker query string: + +```html + + +
[]
+ + + +``` + +Read results with `agent-browser eval`, e.g.: + +```js +(() => { document.getElementById('f').contentDocument.getElementById('lnk-hash').click(); return 'clicked'; })() +(() => JSON.stringify(window.__messages))() +``` + +### First-request constraint + +The iframe's marked request must be the server's first HTML request: + +- Do not use a rendered `.qmd` parent. Its unmarked load reaches the render/inject pipeline first + and disables option injection for later requests. +- Do not send a preflight `curl` or readiness request to an unmarked path. +- Use a genuinely static resource for the parent page instead: add it to + `project.resources:` in `_quarto.yml`, then run a **one-time `quarto render`** before starting + `quarto preview`. Preview alone does not perform the initial static-resource copy. Quarto + excludes resource filenames beginning with `_`, so do not give the parent a leading underscore. +- If `QuartoPreview.getOptions()` returns `origin: ""` and `search: ""`, restart the preview + process. The client cannot reset the memoized state. + +## The proxy pattern (simulating Workbench / RStudio Server) + +To make the browser origin differ from the origin inferred by the preview server, run a temporary +reverse proxy on a second port. `127.0.0.1` and `localhost` resolve to the same host but are +distinct browser origins, so they reproduce this mismatch on one machine: + +```typescript +// Browser origin is http://127.0.0.1:4445; deleting the Host header makes the preview +// server infer its own backend origin (http://localhost:4444) instead of the proxy's. +const kBackend = "http://localhost:4444"; + +Deno.serve({ port: 4445, hostname: "127.0.0.1" }, async (req) => { + const url = new URL(req.url); + const headers = new Headers(req.headers); + headers.delete("host"); + headers.delete("accept-encoding"); + const res = await fetch(kBackend + url.pathname + url.search, { + method: req.method, headers, body: req.body, redirect: "manual", + }); + // fetch() decompresses the body but preserves the original encoding and length headers. + // Remove them or the client truncates the decompressed body, potentially dropping the + // injected client script near the end of the page. + const resHeaders = new Headers(res.headers); + resHeaders.delete("content-encoding"); + resHeaders.delete("content-length"); + return new Response(res.body, { status: res.status, headers: resHeaders }); +}); +``` + +Serve the host page from a path the proxy handles locally (for example, `/__host`) so it does +not reach the backend before the iframe. + +## Required precondition + +Before clicking anything, confirm the intended origin relationship and injected options. Because +this snippet reads the iframe, the parent and iframe must share a browser origin (see Known limits): + +```js +(() => { + const w = document.getElementById('f').contentWindow; + return JSON.stringify({ + parent: location.origin, + iframe: w.location.origin, + injected: w.QuartoPreview && w.QuartoPreview.getOptions(), + }); +})() +``` + +In the proxy case, `injected.origin` must differ from `location.origin`; in the direct localhost +control case, they must match. In both cases, `injected.search` must contain the marker query +string. Do not use subsequent click results if `injected` is `undefined` or `null`, either field +is empty, or the origins do not have the expected relationship. + +## Known limits + +- Websocket upgrades (live reload) are not proxied by the script above; a failed + `new WebSocket(...)` does not throw synchronously, so it doesn't affect the code under test. +- A cross-origin parent cannot read the iframe's DOM. `contentDocument` returns `null` (no + throw). Reading a property such as `contentWindow.QuartoPreview` or `contentWindow.location.origin` + throws a `SecurityError`; on the cross-origin `Location` object specifically, only setting + `href` and calling `replace()` are permitted. `postMessage` still works across origins, which + is why it's the escape hatch: keep both frames on one browser origin, as in the proxy pattern + above, or add iframe-side code that sends its location and injected options to the parent with + `postMessage`. From a87a77ffd0e19f7233a2bfc21793a138baf378ed Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Wed, 9 Sep 2026 12:40:34 +0200 Subject: [PATCH 2/4] docs(preview-test): correct static-resource setup for the viewer harness The harness reference told testers to pre-render the project and to avoid a leading underscore on the parent page. Both were wrong: verified on a website project that `quarto preview` copies files declared under `project.resources:` to the output directory on its own, including `_host.html`, and serves them without consuming the first-request client-injection detection. The underscore exclusion applies to input-file and YAML discovery, not to explicitly declared resources. Also records why a static parent is safe at all (the server only injects into output files that map back to an input) and adds --no-browser, without which the auto-opened browser makes the unmarked first request itself. --- .../references/embedded-viewer.md | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/.claude/skills/quarto-preview-test/references/embedded-viewer.md b/.claude/skills/quarto-preview-test/references/embedded-viewer.md index 7a4a6bd14be..002c59c5da2 100644 --- a/.claude/skills/quarto-preview-test/references/embedded-viewer.md +++ b/.claude/skills/quarto-preview-test/references/embedded-viewer.md @@ -42,15 +42,18 @@ Read results with `agent-browser eval`, e.g.: ### First-request constraint -The iframe's marked request must be the server's first HTML request: +The iframe's marked request must be the first request the server injects into: - Do not use a rendered `.qmd` parent. Its unmarked load reaches the render/inject pipeline first and disables option injection for later requests. -- Do not send a preflight `curl` or readiness request to an unmarked path. -- Use a genuinely static resource for the parent page instead: add it to - `project.resources:` in `_quarto.yml`, then run a **one-time `quarto render`** before starting - `quarto preview`. Preview alone does not perform the initial static-resource copy. Quarto - excludes resource filenames beginning with `_`, so do not give the parent a leading underscore. +- Do not send a preflight `curl` or readiness request to an unmarked rendered path. Wait for the + `Listening on` line in the preview output instead. +- Start preview with `--no-browser`, otherwise the auto-opened browser requests `/` unmarked. +- Use a genuinely static resource for the parent page: declare it under `project.resources:` in + `_quarto.yml`. Preview copies project resources to the output directory itself, so no separate + `quarto render` is needed. A static resource has no corresponding input file, and the server only + injects the client script into output files that map back to an input, so serving the parent page + does not consume the detection. - If `QuartoPreview.getOptions()` returns `origin: ""` and `search: ""`, restart the preview process. The client cannot reset the memoized state. From e9e10098597fb1626c41f23b3fb15fe6ecd1861c Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Wed, 9 Sep 2026 12:51:21 +0200 Subject: [PATCH 3/4] docs(preview-test): forbid all unmarked preflight requests in the harness The previous wording only warned against unmarked requests to rendered paths, but the 404 handler in src/project/serve/serve.ts also calls injectClient. Verified: a single unmarked request to a missing path, sent before the marked iframe request, leaves the injected options empty (origin: "", search: "") - the same symptom the section already documents a restart for. The control case, requesting the already-copied static parent page first, keeps them populated. Readiness now waits on the preview output rather than an HTTP poll, since the poll is itself the unmarked request and the parent page's path 404s until the first render finishes. --- .../references/embedded-viewer.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/.claude/skills/quarto-preview-test/references/embedded-viewer.md b/.claude/skills/quarto-preview-test/references/embedded-viewer.md index 002c59c5da2..e06f80efcfc 100644 --- a/.claude/skills/quarto-preview-test/references/embedded-viewer.md +++ b/.claude/skills/quarto-preview-test/references/embedded-viewer.md @@ -46,14 +46,18 @@ The iframe's marked request must be the first request the server injects into: - Do not use a rendered `.qmd` parent. Its unmarked load reaches the render/inject pipeline first and disables option injection for later requests. -- Do not send a preflight `curl` or readiness request to an unmarked rendered path. Wait for the - `Listening on` line in the preview output instead. +- Send no unmarked request of any kind before the iframe's, with one exception: a path that is + already on disk as a declared static resource. A 404 is not safe either — the `on404` handler + calls `injectClient`, so a single unmarked request to a missing path consumes the detection. +- For readiness, wait for the `Browse at` / `Listening on` lines in the preview output (both go to + stderr) rather than polling an HTTP path. Polling is what produces the unmarked request above, + and before the first render finishes even the parent page's own path still 404s. - Start preview with `--no-browser`, otherwise the auto-opened browser requests `/` unmarked. - Use a genuinely static resource for the parent page: declare it under `project.resources:` in `_quarto.yml`. Preview copies project resources to the output directory itself, so no separate - `quarto render` is needed. A static resource has no corresponding input file, and the server only - injects the client script into output files that map back to an input, so serving the parent page - does not consume the detection. + `quarto render` is needed. Such a file has no corresponding input file, and the server injects the + client script only into output files that map back to an input, so serving the parent page does + not consume the detection. - If `QuartoPreview.getOptions()` returns `origin: ""` and `search: ""`, restart the preview process. The client cannot reset the memoized state. From 06a2ae275e22b672e6e83e76eb47b06c8f2c353b Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Wed, 9 Sep 2026 13:00:11 +0200 Subject: [PATCH 4/4] ci: skip test workflows on .claude-only changes Nothing under .claude/ (skills, rules, commands, agent memory files) can affect build or test outcomes, so running smoke, bundle, feature-format, latexmk and performance jobs for those PRs burns runner time for no signal. test-install.yml keeps its paths allowlist and already never matched. --- .github/workflows/performance-check.yml | 4 ++++ .github/workflows/test-bundle.yml | 4 ++++ .github/workflows/test-ff-matrix.yml | 2 ++ .github/workflows/test-quarto-latexmk.yml | 4 ++++ .github/workflows/test-smokes-parallel.yml | 2 ++ 5 files changed, 16 insertions(+) diff --git a/.github/workflows/performance-check.yml b/.github/workflows/performance-check.yml index 73702dd0fe0..4c70e0764b2 100644 --- a/.github/workflows/performance-check.yml +++ b/.github/workflows/performance-check.yml @@ -4,9 +4,13 @@ on: workflow_dispatch: pull_request: branches: [main] + paths-ignore: + - ".claude/**" push: # only trigger on branches, not on tags branches: [main] + paths-ignore: + - ".claude/**" schedule: - cron: 0 * * * * diff --git a/.github/workflows/test-bundle.yml b/.github/workflows/test-bundle.yml index ca43b88890c..5d3929210c7 100644 --- a/.github/workflows/test-bundle.yml +++ b/.github/workflows/test-bundle.yml @@ -6,9 +6,13 @@ on: workflow_dispatch: pull_request: branches: [main] + paths-ignore: + - ".claude/**" push: # only trigger on branches, not on tags branches: [main] + paths-ignore: + - ".claude/**" jobs: test-bundle: diff --git a/.github/workflows/test-ff-matrix.yml b/.github/workflows/test-ff-matrix.yml index 29645fcd5bd..a1195c4a59f 100644 --- a/.github/workflows/test-ff-matrix.yml +++ b/.github/workflows/test-ff-matrix.yml @@ -15,6 +15,7 @@ on: paths-ignore: - "news/**" - "src/resources/language/**" + - ".claude/**" - ".github/workflows/create-release.yml" - ".github/workflows/performance-check.yml" - ".github/workflows/stale-needs-repro.yml" @@ -28,6 +29,7 @@ on: paths-ignore: - "news/**" - "src/resources/language/**" + - ".claude/**" - ".github/workflows/create-release.yml" - ".github/workflows/performance-check.yml" - ".github/workflows/stale-needs-repro.yml" diff --git a/.github/workflows/test-quarto-latexmk.yml b/.github/workflows/test-quarto-latexmk.yml index 266d3d407e8..35c859301ab 100644 --- a/.github/workflows/test-quarto-latexmk.yml +++ b/.github/workflows/test-quarto-latexmk.yml @@ -3,8 +3,12 @@ on: workflow_dispatch: push: branches: main + paths-ignore: + - ".claude/**" pull_request: branches: main + paths-ignore: + - ".claude/**" name: Test quarto-latexmk jobs: diff --git a/.github/workflows/test-smokes-parallel.yml b/.github/workflows/test-smokes-parallel.yml index 1f4185b4944..39b46b2295a 100644 --- a/.github/workflows/test-smokes-parallel.yml +++ b/.github/workflows/test-smokes-parallel.yml @@ -23,6 +23,7 @@ on: # are not relevant to the tested features - "src/resources/language/**" - "dev-docs/**" + - ".claude/**" # don't run on PR working on other workflows - ".github/workflows/create-release.yml" - ".github/workflows/performance-check.yml" @@ -39,6 +40,7 @@ on: - "v[1-9].[0-9]+" # run also on released version branch (for patch releases) paths-ignore: - "news/**" + - ".claude/**" concurrency: # Use github.run_id on main branch