feat(runner): the authed share round-trip, against the real backend (DEV-2203)

.claude/skills/runner-playwright-e2e/SKILL.md

@@ -0,0 +1,86 @@
+---
+name: runner-playwright-e2e
+description: Use when writing or modifying Playwright E2E specs for the demo runner (runner/e2e/*.spec.ts) - the deterministic-by-default suite, the env-gate taxonomy (E2E_LIVE, E2E_BASE_URL, E2E_BROKER_TOKEN, E2E_AI, E2E_STARTER_MATRIX), the shared helpers, data-* test contracts, CodeMirror and Sandpack gotchas, and container-pool hygiene. NOT for pipeline unit tests (node --test in runner/pipeline/).
+---
+
+# Runner Playwright E2E authoring
+
+Specs live in `runner/e2e/*.spec.ts`, run by `pnpm e2e` against a `vite
+preview` of the built app (or `E2E_BASE_URL` for a deployment). Reference:
+`e2e/helpers.ts` + any recent DEV-2203 spec. The full ruleset, idioms with
+examples, and the anti-pattern history: **`runner/docs/TESTING.md`**. The
+meaningfulness bar: the `runner-test-discipline` skill.
+
+## Deterministic by default
+
+The PR suite must never touch the network beyond the preview server. Start
+every ungated spec with `stubShell(page)` (from `e2e/helpers.ts`): it stubs
+`/api/versions`, aborts both Sandpack hosts (the shell renders fine without a
+grid), and neuters the login redirect. Fake sign-in at the token layer with
+`signIn(page)` — never via `VITE_DEV_USER`, which bypasses the production auth
+path.
+
+Anything that needs the real world takes the **narrowest gate** that covers the
+dependency — `E2E_LIVE` (real preview mount), `E2E_BASE_URL` (worker routes),
+`E2E_BROKER_TOKEN` (authed round-trip), `E2E_AI` (LLM spend),
+`E2E_STARTER_MATRIX` (container matrix). Two hard rules: the spec self-skips
+with instructions (`test.skip(cond, "set X=1 to …")`), and every gated spec is
+named in a workflow that actually runs it — in the same PR. Full taxonomy:
+`runner/docs/TESTING.md`.
+
+## Five rules (non-negotiable)
+
+1. **Import the shared helpers.** `stubShell`, `signIn`, `activeEditor`,
+   `workspaceFiles`, `pickFromMenu`, `previewReady`, `expectGridRendered`,
+   `trackSessions`, `isKnownNoise` live in `e2e/helpers.ts`. Never re-declare
+   them locally — that is how nine copies of `stubShell` happened.
+2. **Hook by the documented `data-*` test contracts, never visible text.**
+   Preview readiness = `data-preview-status` on the Preview section; the active
+   editor pane = `[data-pane-active="true"]`; workspace contents =
+   `window.__HOT_FILES__`. Need a new hook? Add the `data-*` attribute to the
+   component, comment it as a test contract, and use it — don't parse the UI.
+3. **Web-first waits only.** `await expect(locator)...`, `expect.poll()`, or
+   `expect(async () => {...}).toPass()` around debounces (Style panel writes
+   ride 250 ms). Never `waitForTimeout`. Await every assertion.
+4. **CodeMirror is virtualized.** A bare `.cm-content` trips strict mode once a
+   second tab is open, and it only holds the lines on screen — never read file
+   contents from it. Read via `workspaceFiles()`; type via the CodeMirror view
+   dispatch (see `editor-download.spec.ts`). The version/framework pickers are
+   custom listboxes — `pickFromMenu()`, not `selectOption`.
+5. **Container-pool hygiene.** Tier-2 sessions share five global slots with
+   real traffic. Any spec that can boot one wraps in
+   `trackSessions(page)` + `finally { tracked.cleanup(request) }`. For
+   containers, `previewReady` only means the dev server answered — follow with
+   `expectGridRendered` before asserting on the demo.
+
+## Prove it, don't observe it
+
+- **Interaction states**: real pointer + `getComputedStyle` (ADR-0026). A
+  synthetic `mouseover` does not fire `:hover`; a screenshot cannot tell a
+  live hover from a dead one.
+- **Caching/fetch discipline**: collect request paths in `page.route()` and
+  assert the exact list (`docs-examples.spec.ts` — one manifest fetch per
+  bucket, in order).
+- **Persistence**: reload and re-measure. Never `addInitScript` a storage
+  clear — it re-runs on `page.reload()` and defeats the assertion. Each test
+  already gets a fresh context.
+- **Console noise**: filter through `isKnownNoise` and extend `NOISE` with a
+  comment; never blanket-ignore console errors.
+
+## Where tests run
+
+Locally, run **only the spec you created or changed**:
+`cd runner && pnpm e2e e2e/<your-spec>.spec.ts`. The full deterministic suite
+belongs to PR CI (`ci.yml`); the gated suites belong to `e2e-live.yml` and
+`e2e-starter-matrix.yml`. `share-view.spec.ts` depends on the permanent fixture
+demo (`FIXTURE_ID`) — never revoke it.
+
+## Which test — decide, then route
+
+- **A user can see or do it** → E2E here.
+- **Pure logic or a worker route** → `node --test` in `runner/pipeline/`
+  (routes via the `mcp-routes.test.mjs` harness pattern).
+- **Pure refactor** → no new test; `Refactor-only: <reason>` trailer.
+
+The presence gate enforces the choice on every PR; decision table in
+`runner/docs/TESTING.md`.

.claude/skills/runner-test-discipline/SKILL.md

@@ -0,0 +1,83 @@
+---
+name: runner-test-discipline
+description: Use when writing, fixing, or reviewing tests for any runner change (pipeline unit tests, worker route tests, or Playwright E2E), and whenever a test is red during feature work. Enforces that tests prove intended behavior — not just execute code, and never "green for the sake of green". Covers intent-first, deciding whether the code or the test is wrong when red (default - the code), the banned ways of faking green, write-the-failing-test-first, verify-with-a-real-run, no hollow assertions, and not mocking the unit under test.
+---
+
+# Runner test-writing discipline
+
+**Green is not the goal — correct behavior is.** A test that passes but asserts
+nothing, or asserts the *buggy* output, is worse than no test: it certifies the
+bug and reads as coverage. Never make a red test pass by weakening it. Full
+rules, the which-test-where table, the env-gate taxonomy, and the house
+assertion idioms: **`runner/docs/TESTING.md`**. E2E mechanics: the
+`runner-playwright-e2e` skill.
+
+## The test encodes intent, not the implementation
+
+Write the test from the **requirement** — what the user or the API is supposed
+to do — not from what the code currently does. Where feasible, write it first,
+so it is an oracle you cannot accidentally fit to a bug. For E2E, state the
+user-observable expectation *before* you wire a single locator.
+
+## When a test is red, decide what is actually wrong
+
+At the feature stage the **code is the prime suspect, not the test.**
+
+- **Expectation correct, code wrong → fix the code.** The common case. Leave
+  the test alone.
+- **Expectation mis-encoded the intent → tighten the test toward the real
+  behavior** — never loosen it to match the current (possibly wrong) output.
+
+If you cannot tell which is wrong, re-read the requirement — that is not a
+signal to relax the test.
+
+### Banned ways of faking green
+
+- Deleting or loosening an assertion, or widening a timeout/tolerance, to match
+  what the code emits.
+- `test.skip` / `test.fixme`, or focusing with `test.only` / `it.only`.
+- try/catch around the body to swallow a failure.
+- Asserting whatever the code happened to produce (a "snapshot of the bug").
+- Leaning on CI retries to paper over a real intermittent failure.
+- Adding an env gate (`E2E_LIVE` and friends) so PR CI stops running the spec.
+  Gates are for external dependencies and spend, never for red tests.
+
+## Bug fixes: write the failing test first
+
+1. Reproduce the bug as a test and **watch it fail — for the right reason**
+   (the missing behavior, not a typo or a dead selector).
+2. Apply the fix; watch the same test pass.
+3. A regression test that was never red proves nothing. On a bugfix PR, name
+   the spec that fails without the fix.
+
+## Verify before you say "done"
+
+Run the exact command fresh, read the full output and the exit code, and state
+the result with that evidence. Banned phrasings: "should work", "this fixes it"
+without a run, "tested manually, looks fine". After editing source, run the
+impacted test — `pnpm test` for pipeline units, `pnpm e2e e2e/<spec>.spec.ts`
+for the spec you touched — not the whole suite.
+
+## No hollow assertions
+
+Assert the **behavior**, not that the code ran. This repo's cautionary tale
+(fixed in #201): the MCP containment guard was tested against a **local copy of
+its own predicate** — deleting the guard from the route left the test green.
+Import the real control, drive the real route, and check that the test *can*
+fail: neuter the control and watch it go red.
+
+## Don't mock the unit under test
+
+Mock at the real boundary: `page.route()` for the network, in-memory fakes for
+worker bindings (D1, KV, R2), the structural `@cloudflare/sandbox` stub
+(`pipeline/fixtures/`). Route-level worker promises are tested by driving the
+**real default export** of `workers/api/src/index.ts` under `node --test` — the
+`mcp-routes.test.mjs` harness pattern. A fake must model the complete real data
+shape; an incomplete fake gives a false pass.
+
+## The presence gate
+
+Source changed ⇒ a test changed, machine-enforced on every PR
+(`runner/scripts/check-test-presence.mjs`). Pure refactor → `Refactor-only:
+<reason>` trailer; test landing in a named follow-up → `Test-plan: <reason>`.
+Reviewers hold you to the reason.

.github/workflows/ci.yml

@@ -6,11 +6,18 @@ name: CI
 # container so the browser always matches the test package.
 #
 #   unit ──► build ──► authoring ──► e2e
+#   presence (PR-only, needs nothing — runs in parallel)
 #
 # unit gates build: it is the cheapest job (pnpm test builds the runtime
 # itself, by design), so a broken helper fails the run before any minutes go
 # into the app build or a browser container.
 #
+# presence is the test-presence gate (runner/docs/TESTING.md): a PR that
+# changes runner source must also change a test, or declare a Refactor-only:/
+# Test-plan: trailer. It only fires on pull_request — a master push arrives
+# here via workflow_call after its PR already passed it, and has no base to
+# diff against anyway.
+#
 # The live-render checks that need the external Sandpack bundler stay gated
 # behind E2E_LIVE (e2e-live.yml). Also callable (workflow_call) so the deploy
 # workflows can gate on the whole DAG.
@@ -31,6 +38,23 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
+  presence:
+    if: github.event_name == 'pull_request'
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          # The gate diffs the PR range against its base — it needs history,
+          # not just the merge commit.
+          fetch-depth: 0
+
+      - uses: actions/setup-node@v4
+        with:
+          node-version: 22
+
+      - name: Test-presence gate (source change ⇒ test change)
+        run: node runner/scripts/check-test-presence.mjs "${{ github.base_ref }}"
+
   unit:
     runs-on: ubuntu-latest
     defaults:

.gitignore

@@ -5,10 +5,11 @@ dist
 .DS_Store
 .angular
 
-# editors / tooling
+# editors / tooling — local state stays out, committed skills stay in
 .vscode
 .idea/
-.claude/
+.claude/*
+!.claude/skills/
 
 # local environment
 .env

runner/AGENTS.md

@@ -99,7 +99,15 @@ pnpm --filter @handsontable/demo-authoring typecheck
 The `demo-runtime build` is first on purpose: `apps/authoring` typechecks against
 `packages/runtime/dist`, not its source, so a stale `dist` fails on symbols the source has.
 
-What a green E2E run does and does not prove:
+**Testing rules live in [`docs/TESTING.md`](docs/TESTING.md)** — the discipline
+(intent-first, never fake green), the which-test-where table, the env-gate
+taxonomy, and the house assertion idioms — plus the two skills
+(`.claude/skills/runner-test-discipline/`, `.claude/skills/runner-playwright-e2e/`).
+A PR that changes `runner/{apps,packages,workers}/**` source must also change a
+test — machine-enforced by the presence gate (`scripts/check-test-presence.mjs`,
+the `presence` job in `ci.yml`; escape via a `Refactor-only:`/`Test-plan:` trailer).
+
+The quick list — what a green E2E run does and does not prove:
 
 - **The specs that actually mount Sandpack are gated behind `E2E_LIVE=1`.** A default
   `playwright test` skips every one, so a green default run proves nothing about preview
@@ -117,6 +125,13 @@ What a green E2E run does and does not prove:
   (`FIXTURE_ID` in the spec — currently `r-react-18-0-0`). Never revoke it; if it is lost,
   mint a replacement titled "E2E fixture — do not revoke" from any signed-in session and
   update the constant.
+- **The authed write round-trip needs `E2E_BROKER_TOKEN`** (`share-create-live.spec.ts`):
+  a fresh `sessionStorage.hot_token` from a signed-in session on the deployed app. Broker
+  tokens expire and cannot be minted programmatically, so the spec self-skips without one
+  and the workflow treats an expired token as a warning, not a failure. It creates one
+  real demo and revokes it in `finally` (the 410 doubles as the revocation assertion).
+- **`E2E_AI=1` gates the live LLM answer checks** (`ai-live.spec.ts`): two API-level calls
+  per run, real budget, shared 8/min-per-IP rate bucket — a 429 skips rather than fails.
 
 ## Build & deploy