diff --git a/.github/scripts/e2e-tests-workflow-contract_test.py b/.github/scripts/e2e-tests-workflow-contract_test.py index 9ad16b61e8..23854e6277 100644 --- a/.github/scripts/e2e-tests-workflow-contract_test.py +++ b/.github/scripts/e2e-tests-workflow-contract_test.py @@ -122,6 +122,85 @@ class DesktopE2EWorkflowContractTest(unittest.TestCase): self.assertIn('artifact.name === "e2e-timing-profile"', workflow) self.assertIn("!artifact.expired", workflow) + # @covers AC-PLATFORM-E2E-DURATION-AWARE-SHARDING-002.1 + # @covers AC-PLATFORM-E2E-DURATION-AWARE-SHARDING-002.2 + def test_container_job_reuses_verified_browser_cache_with_fallback(self) -> None: + workflow = E2E_WORKFLOW.read_text(encoding="utf-8") + container_job = job_block(workflow, "e2e-containers", "desktop-e2e") + + self.assertIn( + "uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9", + container_job, + ) + self.assertIn( + "uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9", + container_job, + ) + self.assertIn("id: playwright_image", container_job) + self.assertLess( + container_job.index("- name: Log in to GHCR"), + container_job.index("- name: Resolve immutable Playwright runtime image"), + ) + self.assertIn( + "if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository\n uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f", + container_job, + ) + self.assertIn( + "image=ghcr.io/kdlbs/kandev-ci:runtime-latest", + container_job, + ) + self.assertIn( + 'docker buildx imagetools inspect "$image"', + container_job, + ) + self.assertIn("for attempt in 1 2 3; do", container_job) + self.assertIn( + 'if candidate="$(docker buildx imagetools inspect "$image"', + container_job, + ) + self.assertIn('sleep "$((attempt * 2))"', container_job) + self.assertIn("path: /tmp/ms-playwright", container_job) + self.assertIn( + "key: e2e-playwright-${{ runner.os }}-v1.61.1-noble-${{ steps.playwright_image.outputs.digest }}-${{ github.run_id }}-${{ github.run_attempt }}", + container_job, + ) + self.assertIn( + "restore-keys: e2e-playwright-${{ runner.os }}-v1.61.1-noble-${{ steps.playwright_image.outputs.digest }}-", + container_job, + ) + self.assertIn("id: playwright_cache", container_job) + self.assertIn("id: playwright_cache_verify", container_job) + self.assertIn("PLAYWRIGHT_BROWSERS_PATH=/tmp/ms-playwright", container_job) + self.assertIn( + "if: steps.playwright_cache.outputs.cache-hit != ''", + container_job, + ) + self.assertIn( + "if: steps.playwright_cache.outputs.cache-hit == '' || steps.playwright_cache_verify.outcome != 'success'", + container_job, + ) + self.assertIn( + "github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository", + container_job, + ) + self.assertIn( + "PLAYWRIGHT_RUNTIME_REF: ${{ steps.playwright_image.outputs.ref }}", + container_job, + ) + self.assertIn('docker pull "$PLAYWRIGHT_RUNTIME_REF"', container_job) + self.assertIn('"$PLAYWRIGHT_RUNTIME_REF" \\', container_job) + self.assertNotIn('docker pull "${{ steps.playwright_image.outputs.ref }}"', container_job) + self.assertIn( + "if: success() && (steps.playwright_cache.outputs.cache-hit == '' || steps.playwright_cache_verify.outcome != 'success')", + container_job, + ) + self.assertIn("GITHUB_STEP_SUMMARY", container_job) + self.assertIn("id: browser_setup_timer", container_job) + self.assertIn('echo "started_at=$(date +%s)" >> "$GITHUB_OUTPUT"', container_job) + self.assertIn('setup_mode="cache-hit"', container_job) + self.assertIn('setup_mode="image-fallback"', container_job) + self.assertGreaterEqual(container_job.count("continue-on-error: true"), 2) + if __name__ == "__main__": unittest.main() diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 8bf210ca69..67b2f9690d 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -513,14 +513,20 @@ jobs: working-directory: apps run: pnpm install --frozen-lockfile + - name: Start Playwright browser setup timer + id: browser_setup_timer + run: echo "started_at=$(date +%s)" >> "$GITHUB_OUTPUT" + # The kandev-ci image is public, so fork PRs can pull it anonymously. # Their read-only GITHUB_TOKEN can still be rejected by GHCR during an # explicit login because it belongs to the base repository, not the - # contributor's fork. Keep authenticated pulls for trusted runs. + # contributor's fork. Keep authenticated metadata and image requests for + # trusted runs. # # A merge_group run has no `github.event.pull_request` at all, so the - # first operand decides it. That is the right answer: the queue branch - # lives in this repository and its token is a base-repository token. + # condition decides it is a base-repository run. That is the right + # answer: the queue branch lives in this repository and its token is a + # base-repository token. - name: Log in to GHCR if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0 @@ -529,27 +535,107 @@ jobs: username: ${{ github.actor }} password: ${{ secrets.GITHUB_TOKEN }} - # Skip the Playwright CDN download (~7 min and an outage-prone path) by - # copying the prebaked chromium out of the kandev-ci image, which is - # `FROM mcr.microsoft.com/playwright:...` and already ships the matching - # browser at /ms-playwright. Pulling the `runtime` tag (matches the - # `e2e` / `e2e-report` jobs above) — the `build` tag has the same - # browser layer but is ~300 MB larger because of the Go toolchain. - - name: Extract Playwright browsers from kandev-ci image + # Resolve the mutable convenience tag before restoring the cache. The + # digest binds both the cache and the fallback extraction to one image + # identity, while avoiding a full image pull on a verified cache hit. + - name: Resolve immutable Playwright runtime image + id: playwright_image run: | set -euo pipefail - docker pull ghcr.io/kdlbs/kandev-ci:runtime-latest + image=ghcr.io/kdlbs/kandev-ci:runtime-latest + digest="" + for attempt in 1 2 3; do + candidate="" + if candidate="$(docker buildx imagetools inspect "$image" | awk '$1 == "Digest:" { print $2; exit }')" \ + && [[ "$candidate" =~ ^sha256:[0-9a-f]{64}$ ]]; then + digest="$candidate" + break + fi + if (( attempt < 3 )); then + echo "::warning::Could not resolve $image on attempt $attempt/3; retrying" + sleep "$((attempt * 2))" + fi + done + if [[ ! "$digest" =~ ^sha256:[0-9a-f]{64}$ ]]; then + echo "::error::Could not resolve an immutable digest for $image" + exit 1 + fi + { + echo "digest=$digest" + echo "ref=$image@$digest" + } >> "$GITHUB_OUTPUT" + + # The runtime image remains the browser source of truth. The cache only + # avoids copying its already-installed browsers on warm host runners. + # Each run has a unique primary key so a repaired fallback can replace a + # cache entry whose verification failed. The restore prefix selects the + # newest compatible entry without allowing a rejected exact key to win. + - name: Restore Playwright browser cache + id: playwright_cache + uses: actions/cache/restore@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + continue-on-error: true + with: + path: /tmp/ms-playwright + key: e2e-playwright-${{ runner.os }}-v1.61.1-noble-${{ steps.playwright_image.outputs.digest }}-${{ github.run_id }}-${{ github.run_attempt }} + restore-keys: e2e-playwright-${{ runner.os }}-v1.61.1-noble-${{ steps.playwright_image.outputs.digest }}- + + - name: Configure Playwright browser path + run: echo "PLAYWRIGHT_BROWSERS_PATH=/tmp/ms-playwright" >> "$GITHUB_ENV" + + # A cache hit is usable only after Chromium launches from the restored + # directory. `cache-hit` is non-empty for both an exact hit and a + # restore-key match. A corrupted or incompatible entry takes the normal + # image extraction path below. + - name: Verify cached Playwright browser + id: playwright_cache_verify + if: steps.playwright_cache.outputs.cache-hit != '' + continue-on-error: true + working-directory: apps/web + run: | + set -euo pipefail + node <<'NODE' + const { chromium } = require("@playwright/test"); + + (async () => { + const browser = await chromium.launch({ headless: true }); + const page = await browser.newPage(); + await page.goto("data:text/html,ok"); + await browser.close(); + })().catch((error) => { + console.error(error); + process.exit(1); + }); + NODE + echo "Playwright Chromium cache entry verified." + + # On a cache miss or failed cache verification, skip the Playwright CDN + # download (~7 min and an outage-prone path) by copying the prebaked + # browser out of the kandev-ci image. The runtime image is + # `FROM mcr.microsoft.com/playwright:...` and ships it at /ms-playwright. + # Pulling the resolved digest keeps the fallback tied to the image used + # for the cache key; the build image has the same browser layer but is + # ~300 MB larger. + - name: Extract Playwright browsers from kandev-ci image + id: playwright_extract + if: steps.playwright_cache.outputs.cache-hit == '' || steps.playwright_cache_verify.outcome != 'success' + env: + PLAYWRIGHT_RUNTIME_REF: ${{ steps.playwright_image.outputs.ref }} + run: | + set -euo pipefail + docker pull "$PLAYWRIGHT_RUNTIME_REF" mkdir -p /tmp/ms-playwright docker run --rm \ -v /tmp/ms-playwright:/dest \ - ghcr.io/kdlbs/kandev-ci:runtime-latest \ + "$PLAYWRIGHT_RUNTIME_REF" \ cp -a /ms-playwright/. /dest/ - echo "PLAYWRIGHT_BROWSERS_PATH=/tmp/ms-playwright" >> "$GITHUB_ENV" - # Browser binaries come from the image above. GitHub's Ubuntu runners - # usually already have the shared libs Chromium needs, so smoke-test the - # copied browser first and only hit apt as a bounded fallback. + # Browser binaries come from the verified cache or the image above. + # GitHub's Ubuntu runners usually already have the shared libs Chromium + # needs, so smoke-test the copied browser and only hit apt as a bounded + # fallback. - name: Verify Playwright browser runtime + id: playwright_runtime_verify + if: steps.playwright_cache.outputs.cache-hit == '' || steps.playwright_cache_verify.outcome != 'success' working-directory: apps/web run: | set -euo pipefail @@ -579,6 +665,49 @@ jobs: timeout 180s npx playwright install-deps chromium smoke_test + - name: Save Playwright browser cache + id: playwright_cache_save + if: success() && (steps.playwright_cache.outputs.cache-hit == '' || steps.playwright_cache_verify.outcome != 'success') + uses: actions/cache/save@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + continue-on-error: true + with: + path: /tmp/ms-playwright + key: e2e-playwright-${{ runner.os }}-v1.61.1-noble-${{ steps.playwright_image.outputs.digest }}-${{ github.run_id }}-${{ github.run_attempt }} + + - name: Summarize Playwright browser setup + if: always() + env: + CACHE_HIT: ${{ steps.playwright_cache.outputs.cache-hit }} + CACHE_RESTORE_OUTCOME: ${{ steps.playwright_cache.outcome }} + CACHE_VERIFY_OUTCOME: ${{ steps.playwright_cache_verify.outcome }} + CACHE_EXTRACT_OUTCOME: ${{ steps.playwright_extract.outcome }} + CACHE_SAVE_OUTCOME: ${{ steps.playwright_cache_save.outcome }} + IMAGE_DIGEST: ${{ steps.playwright_image.outputs.digest }} + SETUP_STARTED_AT: ${{ steps.browser_setup_timer.outputs.started_at }} + run: | + set -euo pipefail + started="${SETUP_STARTED_AT:-$(date +%s)}" + elapsed="$(( $(date +%s) - started ))" + cache_state="miss" + setup_mode="image-fallback" + if [[ -n "${CACHE_HIT:-}" ]]; then + cache_state="hit" + fi + if [[ -n "${CACHE_HIT:-}" && "${CACHE_VERIFY_OUTCOME:-}" == "success" ]]; then + setup_mode="cache-hit" + fi + { + echo "### Playwright browser setup" + echo "- image-digest: ${IMAGE_DIGEST:-unresolved}" + echo "- cache: ${cache_state}" + echo "- mode: ${setup_mode}" + echo "- restore: ${CACHE_RESTORE_OUTCOME:-not-run}" + echo "- verification: ${CACHE_VERIFY_OUTCOME:-not-run}" + echo "- extraction: ${CACHE_EXTRACT_OUTCOME:-not-run}" + echo "- save: ${CACHE_SAVE_OUTCOME:-not-run}" + echo "- elapsed: ${elapsed}s" + } >> "$GITHUB_STEP_SUMMARY" + - name: Download backend build uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: diff --git a/apps/web/e2e/README.md b/apps/web/e2e/README.md index 5164738a90..017843b0ff 100644 --- a/apps/web/e2e/README.md +++ b/apps/web/e2e/README.md @@ -182,6 +182,22 @@ Dispatch the workflow with `fail_on_flaky=true` to set `failOnFlakyTests: true` for a diagnostic run. Normal PR runs retain the existing two-retry policy while the summary makes retry groups visible. +Container-backed CI jobs also cache the browser directory used by the host +runner. Each job first resolves the `runtime-latest` convenience tag to a +sha256 image digest. The cache key includes the runner OS, the Playwright +`v1.61.1-noble` browser source, that digest, and a run-specific primary-key +suffix. Its stable restore prefix selects the newest compatible entry without +making a rejected cache entry win forever. A verified exact or prefix match +sets `PLAYWRIGHT_BROWSERS_PATH=/tmp/ms-playwright` and skips the GHCR pull and +browser copy. A miss, stale entry, unavailable cache, or failed Chromium +verification uses the digest-pinned runtime-image extraction path. A fallback +save uses the current run's primary key, so a repaired entry becomes the next +candidate. Cache operations are best-effort and cannot change the manifest or +block a healthy fallback. The container job summary reports the image digest, +cache state, verification, extraction, save status, and total setup time so +cache transfer cost can be compared with the 54-second browser provisioning +baseline. + ### Flake rate and trend CI retries hide flakes: with `retries: 2` and `failOnFlakyTests: false`, a test diff --git a/apps/web/e2e/tests/review/review-file-status.spec.ts b/apps/web/e2e/tests/review/review-file-status.spec.ts index 262b6b7d30..3c482eb93e 100644 --- a/apps/web/e2e/tests/review/review-file-status.spec.ts +++ b/apps/web/e2e/tests/review/review-file-status.spec.ts @@ -17,6 +17,7 @@ const MOVED_PATH = "review-status-a-very-long-new-name-that-must-truncate.ts"; test.describe("Review file status", () => { test.describe.configure({ timeout: 120_000 }); + // @covers AC-PLATFORM-E2E-DURATION-AWARE-SHARDING-002.4 test("shows every status, keeps the marker visible at minimum width, and explains a pure move", async ({ testPage, apiClient, @@ -224,9 +225,13 @@ test.describe("Review file status", () => { .toBeGreaterThan(0); await movedRow.click(); + const movedHeader = dialog.locator( + `[data-testid="review-file-header"][data-file-path="${MOVED_PATH}"]`, + ); + const movedSection = movedHeader.locator(".."); await expect( - dialog.getByText(`Moved from ${MOVED_FROM_PATH}; no textual changes`), + movedSection.getByText(`Moved from ${MOVED_FROM_PATH}; no textual changes`), ).toBeVisible(); - await expect(dialog.getByText("Loading diff...")).toHaveCount(0); + await expect(movedSection.getByText("Loading diff...")).toHaveCount(0); }); }); diff --git a/docs/decisions/2026-08-30-e2e-browser-cache.md b/docs/decisions/2026-08-30-e2e-browser-cache.md new file mode 100644 index 0000000000..9c469d735d --- /dev/null +++ b/docs/decisions/2026-08-30-e2e-browser-cache.md @@ -0,0 +1,74 @@ +# ADR-2026-08-30: Cache host-runner E2E browser provisioning + +**Status:** accepted +**Date:** 2026-08-30 +**Area:** infra, workflow + +## Context + +The container-backed E2E jobs run on host runners because they need the Docker +daemon. Each job currently pulls the runtime image and copies its baked +Playwright browsers from `/ms-playwright` to `/tmp/ms-playwright`. Recent job +logs show about 54 seconds of browser provisioning per container shard. The +same setup is repeated across all six parallel jobs, even though the browser +source is identical. + +The runtime image remains the reviewed source of the browser binaries. A cache +must reduce repeat work without allowing an unavailable or stale cache to +change test selection or block a correct E2E run. + +## Decision + +The host-runner container jobs use a GitHub Actions cache for the Playwright +browser directory. The workflow resolves the `runtime-latest` convenience tag +to a sha256 digest before it constructs the cache key. The key includes the +runner operating system, the Playwright browser source, the resolved image +digest, and a unique workflow run ID and attempt. A stable digest-scoped +restore prefix selects the newest compatible entry. + +The job restores the cache before browser provisioning and verifies Chromium +from the restored path. A verified exact or prefix hit skips the image pull and +browser copy. A miss, stale key, cache-service error, or failed verification +uses the digest-pinned runtime-image extraction and smoke check. A successful +fallback populates a new run-specific cache key, so a failed cache entry cannot +remain the newest exact candidate forever. If the image digest cannot be +resolved, the job fails before using the mutable tag. Cache restore and save +are best-effort acceleration steps and are reported separately from test +correctness. + +The cache contains browser binaries only. It does not contain source, +credentials, Docker state, or test artifacts. + +## Consequences + +- Warm container jobs avoid the repeated browser copy and usually start their + tests sooner. +- The first run after a browser-source change still pays the current setup + cost while creating a new cache entry. +- Cache service incidents do not block E2E when the pinned-image fallback is + healthy. +- Updating the runtime image digest invalidates the cache, including changes + that do not affect browsers. This is conservative and binds the cache to the + exact fallback image. +- The workflow must report cache state and provisioning duration so a cache + hit rate and actual wall-time reduction can be evaluated after merge. + +## Alternatives considered + +### Keep extracting browsers on every host runner + +Rejected. It has no cache failure mode, but repeats a measured setup cost in +every container shard and leaves an avoidable critical-path expense. + +### Upload browsers as a workflow artifact from the build job + +Rejected for this change. It would add a producer and consumer artifact path +to every run, increase coupling to the build job, and still require transfer +before the container cohort starts. The cache can be warmed lazily and uses +the same explicit source key. + +### Build a host image with browsers already installed + +Deferred. Host-image ownership would couple the Docker executor to runner +provisioning and would not help arbitrary GitHub-hosted runners. Revisit only +if cache hit rates or cache transfer time do not produce a measured gain. diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index 5c1a24c241..961aceed66 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -217,3 +217,4 @@ Read individual ADRs for full context. Create new ones via `/record decision` or | 2026-08-27-mixed-git-change-facets | [Preserve mixed Git changes as path facets](2026-08-27-mixed-git-change-facets.md) | accepted | backend, agentctl, frontend, protocol | 2026-08-27 | | 2026-08-28-bind-github-auto-merge-attempts-to-reviewed-head | [Bind GitHub Auto-Merge Attempts to the Reviewed Head](2026-08-28-bind-github-auto-merge-attempts-to-reviewed-head.md) | accepted | backend, frontend, protocol, security, GitHub | 2026-08-28 | | 2026-08-30-context-reset-quiesces-active-turn | [Quiesce Active Turns Before Context Reset](2026-08-30-context-reset-quiesces-active-turn.md) | accepted | backend, workflow | 2026-08-30 | +| 2026-08-30-e2e-browser-cache | [Cache host-runner E2E browser provisioning](2026-08-30-e2e-browser-cache.md) | accepted | infra, workflow | 2026-08-30 | diff --git a/docs/plans/e2e-ci-efficiency/plan.md b/docs/plans/e2e-ci-efficiency/plan.md new file mode 100644 index 0000000000..92ca56849b --- /dev/null +++ b/docs/plans/e2e-ci-efficiency/plan.md @@ -0,0 +1,158 @@ +--- +spec: ../../specs/platform/requirements/e2e-duration-aware-sharding.md +created: 2026-08-30 +status: in_progress +--- +# Implementation Plan: E2E CI setup and flake efficiency + +## Objective + +Reduce the E2E critical path after the duration-aware planner has already +removed most test-work imbalance. This package reuses verified Playwright +browsers on host-runner container jobs and removes one recurring false flake +from the review-diff regression test. It preserves the fourteen normal shards, +the six container shards, one worker per shard, existing retries, and the +required aggregate gate. + +## Evidence and diagnosis + +The recent merged E2E stabilization changes are relevant inputs: + +- [PR #3150](https://github.com/kdlbs/kandev/pull/3150) added a causal layout + wait for the PR status badge hover. [PR #3151](https://github.com/kdlbs/kandev/pull/3151) + then narrowed an automation-trigger notification assertion. Both are recent + examples of synchronization fixes that should be preferred over longer + waits. +- [PR #3133](https://github.com/kdlbs/kandev/pull/3133) removed shared-worker + cleanup races and reinforced task-owned test state. +- A fair successful-main run, [run 33250014029](https://github.com/kdlbs/kandev/actions/runs/33250014029), + had a roughly 16m28s normal-shard tail after the build completed. Its six + container jobs repeated browser provisioning. +- In [run 33308951784](https://github.com/kdlbs/kandev/actions/runs/33308951784), + the predicted-versus-actual normal shards stayed within the existing 1.25x + cohort-median target and the container cohort was also balanced. The + workflow's 75m44s end-to-end wall time was dominated by runner queue delay, + so queue capacity is tracked separately from code-level setup savings. +- Container job logs show about 54 seconds to pull/copy Playwright browsers + into `/tmp/ms-playwright` on each host runner. +- The same latest-main run recorded one passed-after-retry result in + `tests/review/review-file-status.spec.ts`. Its final assertion counted two + unrelated `Loading diff...` placeholders in lazy sections while checking a + moved file that had already rendered. The test has also appeared in earlier + flake history, so this is a reproducible synchronization defect rather than + a reason to increase retries. + +## Scope + +### In scope + +- Best-effort browser-cache restore and save in the existing host-runner + container cohort, keyed to the exact checked-in browser source. +- A browser smoke verification that decides whether the restored path is safe; + the current pinned-image extraction remains the fallback. +- Step-summary diagnostics for cache state, browser verification, and setup + duration so future runs can measure the gain. +- A selection-scoped readiness assertion in the review-file-status E2E test. +- Workflow contract coverage, focused E2E coverage, and rollout evidence. + +### Out of scope + +- Changing the duration-aware planner, timing-profile schema, 14/6 matrix, or + `workers: 1` default. +- Splitting mobile Chrome into a separate matrix. +- Increasing retries or timeouts, adding fixed sleeps, or weakening the E2E + gate. +- Replacing the runtime image, moving browsers to an external service, or + changing application behavior. +- Treating GitHub runner queue time as a test or sharding defect. + +## Technical approach + +The container jobs resolve the `runtime-latest` convenience tag to an immutable +sha256 digest, then restore `/tmp/ms-playwright` before the digest-pinned image +pull and copy step. The cache key includes the runner OS, browser source, +resolved image digest, and a run-specific primary-key suffix with a stable +digest-scoped restore prefix. A verified exact or prefix hit skips the +pull/copy step. A miss, stale, cache-service failure, or failed Chromium +verification runs the existing digest-pinned extraction path and can populate +the cache under a new primary key after successful setup. Cache actions are +best-effort and cannot turn a healthy fallback into a failed E2E job. + +The review test anchors the final moved-file assertions to the selected file's +existing review header/section boundary. Other lazy sections may still show a +loading marker and do not affect the selected file's readiness. + +## Work orders + +### Wave 1 + +- [ ] [task-01-cache-host-runner-browsers](task-01-cache-host-runner-browsers.md) +- [x] [task-02-scope-review-diff-readiness](task-02-scope-review-diff-readiness.md) + +The slices are independently verifiable, but execution remains sequential in +the primary session so workflow and test changes can be reviewed together. + +## Rollout and success criteria + +1. Run the workflow contract and action-pinning checks locally. +2. Run the focused review E2E test repeatedly with retries disabled. +3. Merge the workflow change and compare at least three successful `main` + runs with the preceding setup and execution measurements. +4. Confirm warm container jobs report a verified cache hit and omit browser + extraction. Confirm a forced key miss or unavailable cache uses the pinned + image fallback and still passes. +5. Retain the change only if the container setup component decreases without + new browser-verification failures, retries, or test failures. Continue to + report queue delay separately. + +The existing shard-tail target remains the acceptance threshold for test-work +balance. A cache change is not credited with a sharding improvement, and a +shorter queued workflow is not credited as a test-speed improvement. + +## Verification strategy + +The work orders own focused verification. The implementation handoff should +also run the repository specification lint after changing the design package: + +```bash +python3 scripts/lint-spec-files.py --all +git diff --check +``` + +Remote rollout evidence belongs in this plan after the branch workflow runs. +Until then, the cache hit rate and setup timings remain open operational +measurements. + +## Implementation results + +- The container workflow now resolves an immutable runtime image digest, + restores and saves a Playwright browser cache keyed by runner OS, browser + source, digest, and a run-specific suffix, and uses a stable digest-scoped + restore prefix. Cache verification gates the digest-pinned GHCR fallback, + and setup state is written to the job summary. Cache errors remain + non-blocking. +- The review-file-status regression now scopes the moved-file assertions to its + selected diff section. A 30-repeat host run with retries disabled passed all + 30 repetitions. +- PR run [#3163](https://github.com/kdlbs/kandev/pull/3163) passed all 20 E2E + shards and the full required check set. A container shard restored a + digest-scoped prefix cache, verified Chromium, skipped image extraction, and + reported about 9 seconds for browser setup. The preceding PR run exposed a + transient GHCR metadata lookup failure; the workflow now retries that lookup + three times before failing safely. +- Post-merge comparison across three successful `main` runs and a forced-miss + fallback measurement remain open rollout work. + +## Risks + +- GitHub Actions cache transfer can cost enough time to erase the browser-copy + saving. Step summaries must report restore and verification duration before + declaring success. +- Concurrent first-run cache saves can race. The fallback remains valid and + each run has a distinct primary key; the restore prefix selects the newest + compatible successful entry. +- The runtime image may change without a browser-version change. Including the + resolved image digest in the key is a conservative invalidation rule and + binds fallback extraction to the same image identity. +- Hosted-runner queue capacity can dominate total workflow wall time. The + rollout must compare job execution intervals and queue intervals separately. diff --git a/docs/plans/e2e-ci-efficiency/task-01-cache-host-runner-browsers.md b/docs/plans/e2e-ci-efficiency/task-01-cache-host-runner-browsers.md new file mode 100644 index 0000000000..305c8b14e9 --- /dev/null +++ b/docs/plans/e2e-ci-efficiency/task-01-cache-host-runner-browsers.md @@ -0,0 +1,81 @@ +--- +id: "01-cache-host-runner-browsers" +title: "Cache host-runner Playwright browsers" +status: in_progress +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/platform/requirements/e2e-duration-aware-sharding.md" +--- +# Task 01: Cache host-runner Playwright browsers + +## Outcome + +Remove repeat browser provisioning from warm container-backed E2E jobs while +preserving the current pinned-image path as a correctness-safe fallback. + +## Requirements and design + +- `REQ-PLATFORM-E2E-DURATION-AWARE-SHARDING-002` +- `AC-PLATFORM-E2E-DURATION-AWARE-SHARDING-002.1` +- `AC-PLATFORM-E2E-DURATION-AWARE-SHARDING-002.2` +- `docs/specs/platform/system-design/e2e-duration-aware-sharding.md`, + Browser cache contract and Observability sections +- `docs/decisions/2026-08-30-e2e-browser-cache.md` + +## Acceptance + +- The existing host-runner container jobs resolve the runtime image to an + immutable digest and restore `/tmp/ms-playwright` using a digest-scoped + restore prefix plus a run-specific primary key. A verified exact or prefix + hit skips the Docker pull/copy step. +- A cache miss, stale or unavailable entry, cache action error, or failed + browser verification runs the digest-pinned image extraction and smoke + check. A successful fallback uses a new primary key so a rejected cache + entry is not repeatedly selected. Cache operations cannot fail a healthy + fallback or change the test manifest, retries, timeout, or worker count. +- Each container job reports cache state, browser verification, and setup + timing. Workflow contract tests protect the key, conditional fallback, and + existing container setup. + +## Likely files + +- `.github/workflows/e2e-tests.yml` +- `.github/scripts/e2e-tests-workflow-contract_test.py` +- `apps/web/e2e/README.md` when the local/CI setup documentation needs the + cache and fallback behavior recorded + +## Verification + +```bash +python3 .github/scripts/e2e-tests-workflow-contract_test.py +python3 .github/scripts/lint-action-pinning_test.py +python3 .github/scripts/lint-action-pinning.py +git diff --check +``` + +After the branch workflow is available, inspect one cache-hit and one forced +miss run. The hit must omit the browser pull/copy step. The miss must complete +the existing extraction and E2E smoke path. Record restore, verification, +fallback, and test-start durations in the parent plan. + +## Dependencies and parallelism + +No implementation dependency. Execute sequentially in the primary session so +the workflow contract and fallback behavior are reviewed as one change. + +## Exclusions + +Do not change the runtime image's browser source, the normal E2E job, the +matrix sizes, or Docker resource ownership. Do not make cache availability a +required external service. + +## Results + +The workflow contract, action-pinning tests, targeted workflow security scan, +YAML parse, and diff checks pass locally. PR #3163 passed all container shards; +run 33325045980 restored a digest-scoped prefix cache, verified Chromium, and +skipped image extraction in a reported setup interval of about 9 seconds. The +workflow also retries transient immutable-image metadata lookup failures three +times. A forced-miss fallback run and the post-merge three-run comparison remain +pending for rollout validation. diff --git a/docs/plans/e2e-ci-efficiency/task-02-scope-review-diff-readiness.md b/docs/plans/e2e-ci-efficiency/task-02-scope-review-diff-readiness.md new file mode 100644 index 0000000000..8e72a57ab3 --- /dev/null +++ b/docs/plans/e2e-ci-efficiency/task-02-scope-review-diff-readiness.md @@ -0,0 +1,68 @@ +--- +id: "02-scope-review-diff-readiness" +title: "Scope review diff readiness assertions" +status: done +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/platform/requirements/e2e-duration-aware-sharding.md" +--- +# Task 02: Scope review diff readiness assertions + +## Outcome + +Remove the recurring false flake in the moved-file review regression test by +making its loading assertion apply only to the selected file section. + +## Requirements and design + +- `REQ-PLATFORM-E2E-DURATION-AWARE-SHARDING-002` +- `AC-PLATFORM-E2E-DURATION-AWARE-SHARDING-002.3` +- `AC-PLATFORM-E2E-DURATION-AWARE-SHARDING-002.4` +- `docs/specs/platform/system-design/e2e-duration-aware-sharding.md`, + Reliability and flake contract section + +## Acceptance + +- `review-file-status.spec.ts` anchors the moved-file explanation and loading + assertions to the selected moved-file diff section, using existing stable + review markup. Unselected lazy sections cannot fail the selected-file check. +- The test uses causal Playwright assertions and no fixed sleep, timeout + inflation, or retry-dependent success path. +- The focused test passes repeatedly with retries disabled and the existing + E2E lint and sleep-ratchet checks remain clean. + +## Likely files + +- `apps/web/e2e/tests/review/review-file-status.spec.ts` + +## Verification + +```bash +cd apps/web +pnpm e2e:run --host --no-build tests/review/review-file-status.spec.ts -- --repeat-each=30 --workers=1 --retries=0 +pnpm exec eslint --max-warnings 0 e2e/tests/review/review-file-status.spec.ts +pnpm run e2e:sleep-ratchet +``` + +The focused run must complete without retries or the observed global +`Loading diff...` count failure. If the selected section has a new stable +boundary, keep the selector scoped to that boundary and record the reason in +the test. + +## Dependencies and parallelism + +No implementation dependency. Execute sequentially in the primary session. + +## Exclusions + +Do not modify the recently merged PR-status hover or automation-notification +tests. Do not change the review product component unless the existing section +boundary cannot be addressed from the test. + +## Results + +The pre-fix stress run exposed a separate existing workspace-update fixture +race on one repetition. The targeted selector fix then passed once and passed +all 30 repetitions with retries disabled. ESLint and the E2E sleep ratchet also +pass. diff --git a/docs/specs/platform/README.md b/docs/specs/platform/README.md index d598e6b29b..756613526a 100644 --- a/docs/specs/platform/README.md +++ b/docs/specs/platform/README.md @@ -79,6 +79,7 @@ localization, feature toggles, health, and shared session recovery services. - [Diagnostic logging System Design Part 1](system-design/diagnostic-logging-01.md) - [Diagnostic logging System Design Part 2](system-design/diagnostic-logging-02.md) - [Expected runtime log severity](system-design/expected-runtime-log-severity.md) +- [Duration-aware E2E sharding and CI reliability](system-design/e2e-duration-aware-sharding.md) - [Health Endpoint — Surface the Running Version](system-design/health-endpoint-version.md) - [Internationalization (i18n)](system-design/i18n.md) - [LSP File Intelligence System Design Part 1](system-design/lsp-file-intelligence-01.md) diff --git a/docs/specs/platform/requirements/e2e-duration-aware-sharding.md b/docs/specs/platform/requirements/e2e-duration-aware-sharding.md index 3369a8d873..c1c615dca9 100644 --- a/docs/specs/platform/requirements/e2e-duration-aware-sharding.md +++ b/docs/specs/platform/requirements/e2e-duration-aware-sharding.md @@ -2,7 +2,7 @@ status: active system: platform created: 2026-08-10 -updated: 2026-08-23 +updated: 2026-08-30 owners: - kandev --- @@ -23,6 +23,20 @@ The E2E workflow currently uses Playwright's ordinal test-level sharding. It kee - **AC-PLATFORM-E2E-DURATION-AWARE-SHARDING-001.1:** **Normal:** the existing fourteen-shard Chromium and mobile-Chrome matrix. - **AC-PLATFORM-E2E-DURATION-AWARE-SHARDING-001.2:** **Containers:** the existing six-shard Docker and SSH matrix. +### REQ-PLATFORM-E2E-DURATION-AWARE-SHARDING-002: Efficient setup and actionable flake feedback + +**Intent:** Once test work is balanced, repeated setup and false-positive +assertions become the next avoidable sources of CI time and lost diagnosis. +Setup acceleration must preserve the existing fallback, and a green result +must not hide a test that only passed after a retry. + +#### Acceptance criteria + +- **AC-PLATFORM-E2E-DURATION-AWARE-SHARDING-002.1:** Container-backed E2E jobs shall resolve the runtime image to an immutable digest, reuse browser binaries from a matching digest-scoped cache when available, and use a per-run save key with a stable digest-scoped restore prefix. A miss, stale entry, cache-service failure, or incompatible entry shall use the digest-pinned image extraction path and shall not change test selection or make the cache a correctness dependency. If the image digest cannot be resolved, the job shall fail before using an unpinned image reference. +- **AC-PLATFORM-E2E-DURATION-AWARE-SHARDING-002.2:** E2E reporting shall separate test execution time from material setup costs, including dependency installation, runtime-image startup, and browser provisioning, and shall expose whether the timing plan used a profile or a fallback. +- **AC-PLATFORM-E2E-DURATION-AWARE-SHARDING-002.3:** E2E reporting shall identify each passed-after-retry result by stable test key, attempt status, error category, and available diagnostic artifacts. The explicit diagnostic lane shall be able to fail when a test passes only after retry. +- **AC-PLATFORM-E2E-DURATION-AWARE-SHARDING-002.4:** When an E2E test selects one lazy-loaded item, readiness assertions shall be scoped to that selected item. Unrelated loading placeholders shall not determine the selected item's result. + ## Migrated source detail ## Why @@ -240,4 +254,7 @@ manifests, rolling profile, retry diagnostics, and fail-closed runner have unit, type, and targeted E2E coverage. The prebuilt desktop image amendment is implemented and was proven by the branch image publish and pull-request smoke run recorded in `docs/plans/desktop-e2e-prebuilt-image/plan.md`. The balance -target remains an operational check after merge. +target remains an operational check after merge. The setup-reuse and +selection-scoped readiness follow-up is implemented on the current branch. +Remote cache hit/miss measurements and post-merge rollout evidence remain +pending. diff --git a/docs/specs/platform/system-design/e2e-duration-aware-sharding.md b/docs/specs/platform/system-design/e2e-duration-aware-sharding.md new file mode 100644 index 0000000000..bc242b5282 --- /dev/null +++ b/docs/specs/platform/system-design/e2e-duration-aware-sharding.md @@ -0,0 +1,226 @@ +--- +status: current +system: platform +requirements: + - REQ-PLATFORM-E2E-DURATION-AWARE-SHARDING-001 + - REQ-PLATFORM-E2E-DURATION-AWARE-SHARDING-002 +created: 2026-08-30 +updated: 2026-08-30 +owners: + - kandev +--- +# Duration-aware E2E sharding and CI reliability system design + +## Purpose and boundaries + +This design defines the operational boundary for Kandev's Playwright E2E +workflow. It keeps test work balanced, removes repeatable setup work from the +container-backed cohort where safe, and makes retry and timing evidence +actionable. + +The design covers GitHub Actions workflow orchestration, E2E planning and +reporting, browser provisioning, and the synchronization contract of the +affected regression test. It does not change application behavior, the +worker-scoped backend fixture model, the required E2E gate, or the normal +matrix shape. + +## Requirement mapping + +| Requirement | Design sections | +| --- | --- | +| `REQ-PLATFORM-E2E-DURATION-AWARE-SHARDING-001` | [Execution flow](#execution-flow), [Existing planning contract](#existing-planning-contract) | +| `REQ-PLATFORM-E2E-DURATION-AWARE-SHARDING-002` | [Browser cache contract](#browser-cache-contract), [Reliability and flake contract](#reliability-and-flake-contract), [Observability](#observability) | + +## Design invariants + +- The normal cohort remains fourteen shards covering Chromium and mobile + Chrome, and the container cohort remains six shards covering Docker and SSH. +- Each shard remains single-worker until a measured concurrency change passes + the existing isolation and retry criteria. +- A timing profile or browser cache accelerates planning or setup only. A + missing, stale, or inaccessible optimization input has a deterministic + fallback and cannot change the selected test set. +- A passing-after-retry result remains visible, even when the normal pull + request lane is allowed to finish green. +- CI time analysis separates runner queue time from job setup and test + execution time. Queue time is capacity evidence, not a test-synchronization + signal. + +## Components and responsibilities + +| Component | Responsibility | +| --- | --- | +| `.github/workflows/e2e-tests.yml` | Detect relevant changes, build artifacts, obtain timing data, run the two E2E cohorts, and publish reports. | +| `apps/web/e2e/scripts/e2e-timings.ts` | Maintain the bounded successful-`main` timing profile and classify profile fallbacks. | +| `apps/web/e2e/scripts/plan-shards.ts` | Discover the cohort catalog, create deterministic duration-aware manifests, and validate coverage. | +| `apps/web/e2e/scripts/run-planned-shard.ts` | Validate and execute one explicit manifest selection while preserving Playwright reporting and retry settings. | +| `apps/web/e2e/scripts/retry-summary.ts` and `flake-report.ts` | Group attempts by stable test key and publish retry and historical flake evidence. | +| `.github/docker/ci-base/Dockerfile` | Define the pinned Playwright browser source baked into the runtime image. | +| `apps/web/e2e/tests/review/review-file-status.spec.ts` | Verify the selected review file's status using a selection-scoped readiness assertion. | + +## Existing planning contract + +The build job remains the only producer of run-scoped normal and container +manifests. It obtains the latest eligible successful-`main` profile, discovers +the catalog from the authoritative Playwright configuration, and produces +explicit file selections. The planner continues to use longest-processing-time +bin packing at project/file granularity with deterministic tie-breaking. + +The normal and container matrix jobs download only their cohort manifest. They +do not add ordinal Playwright sharding as a hidden fallback. An unavailable +profile uses the existing count-based plan and reports that mode. An invalid +manifest fails before test execution. + +The report job remains responsible for merged reports, the next timing-profile +candidate, retry summaries, and predicted-versus-actual shard diagnostics. +The aggregate gate retains its current cancellation and failure semantics. + +## Execution flow + +1. The change-detection job decides whether the E2E workflow is required. +2. The build job installs dependencies, builds the backend and web artifacts, + packages the plugin fixture, resolves the eligible `main` profile, and + creates validated manifests for both cohorts. +3. Normal matrix jobs download artifacts and run their explicit manifest + selection in the existing runtime image. +4. Container matrix jobs restore the browser cache described below, use the + pinned image extraction fallback when needed, then run the explicit + Docker/SSH manifest selection on the host runner. +5. The report job merges blobs, records setup and execution diagnostics, + creates the retry and flake summaries, and publishes a new profile only + when the run is an eligible successful `main` result. +6. The required gate evaluates the final job results. + +## Browser cache contract + +The container-backed jobs currently copy `/ms-playwright` from the pinned +runtime image to `/tmp/ms-playwright` on every host runner. The cache removes +that repeated copy after the first successful warm-up without replacing the +image as the source of truth. + +The existing host job uses the following sequence: + +1. Resolve the runtime image manifest to a sha256 digest. If resolution fails, + stop before using a mutable image reference. +2. Restore `/tmp/ms-playwright` from the GitHub Actions cache before the + digest-pinned Docker pull and copy step. +3. Use the runner operating system, Playwright browser source, resolved image + digest, workflow run ID, and attempt number in the primary key. Supply a + stable prefix ending before the run-specific suffix as the restore key. A + browser-source or image change therefore cannot reuse an incompatible + entry, while a failed entry cannot win an exact-key lookup forever. +4. On a matching exact or prefix hit, set `PLAYWRIGHT_BROWSERS_PATH` to the + restored path, verify that Chromium is usable, and skip the image pull and + copy. +5. On a miss, stale entry, cache-service error, or failed verification, pull + the resolved digest, run the existing image extraction and browser smoke + check, and save a successful fallback under the current run's new primary + key. Cache restore and save failures are reported as setup diagnostics and + do not fail a test whose digest-pinned fallback succeeds. + +The cache contains browser binaries only. It does not contain repository +source, credentials, test results, or generated application artifacts. A +cache hit cannot alter the manifest, project filters, retries, timeout, or +worker count. The first run after a key change is expected to pay the current +provisioning cost. + +## Reliability and flake contract + +The retry summary continues to use the stable project, repository-relative +file, and full-title key. It records attempt count, each attempt's status, +error category, and links to available trace, screenshot, or error-context +artifacts. The historical report retains recent successful-main entries and +cross-checks repeat offenders against the Playwright result data. + +The explicit diagnostic lane sets `failOnFlakyTests` so a test that passes only +after retry fails that lane. The normal pull request lane may retain retries, +but its report must still expose the passed-after-retry result. + +For the review-file-status regression, the selected moved-file header is the +readiness anchor. Assertions about the moved-file explanation and the absence +of its own loading marker are scoped to that file's section. Loading markers +for other lazy-loaded files in the dialog are not evidence that the selected +file is unready and must not fail the test. + +This synchronization uses existing DOM state and causal Playwright assertions. +It does not add a fixed sleep or enlarge a timeout to mask the race. + +## Timing and setup data + +The report preserves these distinct measurements: + +- runner queue and job start delay, derived from workflow and job timestamps; +- dependency installation; +- runtime image startup and browser provisioning; +- Playwright test execution per shard; +- report merge and aggregate-gate delay. + +Predicted-versus-actual shard data remains keyed by cohort and shard number. +It includes the planning mode, unknown and warm counts, target duration, and +actual duration. The container summary emits the resolved image digest, +logical cache state, setup mode, restore/verification/extraction/save outcomes, +and total browser setup elapsed time. These values let setup savings be +compared with the test and queue budgets without claiming per-step durations +that the workflow does not measure. + +## Failure modes and recovery + +| Condition | Required behavior | +| --- | --- | +| Timing profile is absent or unusable | Use the validated count-based plan and report the fallback. | +| Manifest is missing, malformed, incomplete, or overlapping | Fail the planning or shard job before test execution. | +| Runtime image digest cannot be resolved | Fail before cache restore or image use; never fall back to the mutable convenience tag. | +| Browser cache hit | Verify the restored browser path, then skip image extraction only when verification succeeds. | +| Browser cache miss, stale key, cache outage, or failed verification | Use the digest-pinned runtime image extraction and smoke check, then save under a new run-specific key when possible. Test selection and correctness remain unchanged. | +| Cache save is unavailable | Finish the successful test run and report that the next run will need the fallback path. | +| Test passes after retry | Preserve the final pass, attempt details, error category, and diagnostic artifacts; fail the explicit diagnostic lane. | +| Selected review file is loaded while other lazy sections still show placeholders | Pass the selection-scoped assertion; unrelated placeholders are ignored. | +| Runner queue is long | Preserve job timestamps and execution diagnostics so capacity delay is not mistaken for shard imbalance. | + +## Security and workflow constraints + +- Keep all workflow actions pinned to their existing reviewed commit SHAs. +- Use the existing read-only package and artifact permissions. The browser + cache must not require a new secret or broaden pull-request credentials. +- Resolve `runtime-latest` to a sha256 digest and use the digest-pinned image + reference for fallback extraction. Restore only the exact browser path used + by the host job and use the digest-scoped cache prefix as a compatibility + boundary. Do not restore arbitrary files into the workspace or Docker state. +- Preserve the repository's workflow rules for untrusted pull requests, + checkout credentials, and container cleanup. + +## Observability + +The workflow retains the existing `e2e-timing-diagnostics`, +`e2e-retry-summary`, `e2e-flake-history`, timing-profile, manifest, blob, and +merged-report artifacts. The container jobs add a short step summary with +cache state and setup timings. The report continues to make the following +questions answerable from one run: + +- Did the run use a timing profile or count fallback? +- Was the slowest shard caused by test execution or setup? +- Which immutable image digest and cache state did the run use, and did the + cache hit remove the extraction step? +- Did any test pass only after retry, and is it a repeat offender? +- Was the total wall time dominated by runner queue capacity? + +Three successful `main` runs remain the rollout sample for the shard-tail +target. A cache change is retained only if it reduces the container setup +component without increasing retries, failed browser verification, or test +failures. + +## Out of scope + +- changing the fourteen normal or six container shards; +- enabling `workers: 2` by default; +- splitting the normal matrix into separate mobile and desktop cohorts; +- changing the Playwright retry count, test timeout, or required gate; +- replacing the rolling profile with a service or checked-in shard lists; +- hiding retry groups by increasing waits or weakening assertions; +- changing application code solely to accommodate this CI optimization. + +## Related decisions and delivery plan + +- [Duration-aware E2E sharding uses rolling `main` timings](../../../decisions/2026-08-10-duration-aware-e2e-sharding.md) +- [Browser cache for host-runner E2E provisioning](../../../decisions/2026-08-30-e2e-browser-cache.md) +- [E2E CI efficiency plan](../../../plans/e2e-ci-efficiency/plan.md)