diff --git a/.github/docker/ci-base/Dockerfile b/.github/docker/ci-base/Dockerfile index cdc7ba0bf6..4538692bc4 100644 --- a/.github/docker/ci-base/Dockerfile +++ b/.github/docker/ci-base/Dockerfile @@ -41,6 +41,7 @@ RUN apt-get update \ make \ jq \ tar \ + unzip \ xz-utils \ && rm -rf /var/lib/apt/lists/* diff --git a/.github/workflows/backend-tests.yml b/.github/workflows/backend-tests.yml index 9da7cee6e5..9f878fac6f 100644 --- a/.github/workflows/backend-tests.yml +++ b/.github/workflows/backend-tests.yml @@ -340,7 +340,9 @@ jobs: test-windows: name: Backend (windows) runs-on: windows-latest - timeout-minutes: 15 + # The race-sensitive Windows package suite can exceed 15 minutes on a + # busy hosted runner; keep the cap finite while allowing normal completion. + timeout-minutes: 20 # Least-privilege: this job only reads source and runs tests, no PR/issue # writes or release operations. Flagged by CodeQL workflow audit + matching # CodeRabbit nitpick. diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 576829245c..e42265fb45 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -26,6 +26,12 @@ on: # Manual trigger for verifying CI on draft PRs and ad-hoc reruns. The # default sharded run uses the same ref the user picks in the dispatch UI. workflow_dispatch: + inputs: + fail_on_flaky: + description: "Fail the run when Playwright needs a retry" + required: false + type: boolean + default: false concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -33,6 +39,8 @@ concurrency: permissions: contents: read + # The build/report jobs read the latest successful main timing artifact. + actions: read # Required so the containerized build / e2e shards / e2e-report jobs can # pull the kandev-ci image from GHCR via GITHUB_TOKEN. packages: read @@ -92,6 +100,98 @@ jobs: - name: Build Docker E2E helper binaries run: make build-backend-linux-helpers + - name: Locate latest successful main timing profile + id: main-timing-run + env: + GH_TOKEN: ${{ github.token }} + GITHUB_API_URL: ${{ github.api_url }} + GITHUB_REPOSITORY: ${{ github.repository }} + run: | + node <<'NODE' + const fs = require("node:fs"); + + (async () => { + const url = + process.env.GITHUB_API_URL + + "/repos/" + + process.env.GITHUB_REPOSITORY + + "/actions/workflows/e2e-tests.yml/runs?branch=main&status=success&per_page=20"; + const response = await fetch(url, { + headers: { + accept: "application/vnd.github+json", + authorization: "Bearer " + process.env.GH_TOKEN, + }, + }); + if (!response.ok) { + console.log( + "::warning::Unable to query successful main E2E runs (" + + response.status + + "); using count fallback.", + ); + fs.appendFileSync(process.env.GITHUB_OUTPUT, "run_id=\n"); + return; + } + const payload = await response.json(); + const run = (payload.workflow_runs ?? []).find( + (candidate) => candidate.head_branch === "main" && candidate.conclusion === "success", + ); + fs.appendFileSync(process.env.GITHUB_OUTPUT, "run_id=" + (run?.id ?? "") + "\n"); + console.log( + run + ? "Using successful main E2E run " + run.id + " for timing history." + : "No successful main E2E run found; using count fallback.", + ); + })().catch((error) => { + console.error(error); + fs.appendFileSync(process.env.GITHUB_OUTPUT, "run_id=\n"); + }); + NODE + + - name: Download latest main timing profile + if: steps.main-timing-run.outputs.run_id != '' + continue-on-error: true + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: e2e-timing-profile + path: e2e/previous-profile + repository: ${{ github.repository }} + run-id: ${{ steps.main-timing-run.outputs.run_id }} + github-token: ${{ github.token }} + + - name: Upload selected timing baseline for report job + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: e2e-selected-main-timing-profile + path: e2e/previous-profile/timing-profile.json + if-no-files-found: ignore + retention-days: 3 + + - name: Generate duration-aware E2E shard manifests + working-directory: apps/web + run: | + set -euo pipefail + rm -rf "$GITHUB_WORKSPACE/e2e-manifests" + if [[ -f "$GITHUB_WORKSPACE/e2e/previous-profile/timing-profile.json" ]]; then + echo "Planning from the latest successful main timing profile." + pnpm exec tsx e2e/scripts/plan-shards.ts \ + --web-root "$GITHUB_WORKSPACE/apps/web" \ + --output-dir "$GITHUB_WORKSPACE/e2e-manifests" \ + --profile "$GITHUB_WORKSPACE/e2e/previous-profile/timing-profile.json" + else + echo "Planning with deterministic count fallback; no usable main timing profile was downloaded." + pnpm exec tsx e2e/scripts/plan-shards.ts \ + --web-root "$GITHUB_WORKSPACE/apps/web" \ + --output-dir "$GITHUB_WORKSPACE/e2e-manifests" + fi + + - name: Upload E2E shard manifests + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: e2e-shard-manifests + path: e2e-manifests/ + retention-days: 3 + - name: Upload backend build uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: @@ -197,14 +297,26 @@ jobs: working-directory: apps/web run: tar -xf web-dist.tar && rm web-dist.tar + - name: Enable strict flaky-test diagnostics + if: github.event_name == 'workflow_dispatch' && inputs.fail_on_flaky + run: echo "E2E_FAIL_ON_FLAKY=1" >> "$GITHUB_ENV" + + - name: Download E2E shard manifests + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: e2e-shard-manifests + path: e2e-manifests + # Pinned to chromium + mobile-chrome so the dedicated `e2e-containers` # job below owns the real-Docker project (which covers both the Docker # executor and the SSH executor's sshd target). Without --project the # container-bound specs would run here too, fail seeding (Docker not # enabled for these shards), and spam every shard. - - name: Run E2E tests (shard ${{ matrix.shard }}/14) + - name: Run planned E2E tests (shard ${{ matrix.shard }}/14) working-directory: apps/web - run: npx playwright test --config e2e/playwright.config.ts --project=chromium --project=mobile-chrome --shard=${{ matrix.shard }}/14 + env: + E2E_SHARD: ${{ matrix.shard }} + run: bash e2e/scripts/run-planned-shard.sh "$GITHUB_WORKSPACE/e2e-manifests/normal/${{ matrix.shard }}.json" - name: Upload blob report if: always() @@ -225,9 +337,8 @@ jobs: # Real-Docker E2E: exercises the local_docker executor end-to-end. # ubuntu-latest ships with Docker preinstalled, so no service container is # required. Container-bound tests are slow (~2 min each), so we shard across - # 6 runners — Playwright's --shard splits the project's tests into N groups. - # Cleans up any leftover kandev-managed containers at the end so a leaky - # process doesn't poison the next run on the same self-hosted runner. + # 6 runners — the build job creates a duration-aware manifest for this + # cohort, and each runner executes its explicit file selection. # # This job runs the `containers` Playwright project, which covers both the # Docker executor and the SSH executor (since the SSH executor's e2e tests @@ -366,15 +477,22 @@ jobs: working-directory: apps/web run: tar -xf web-dist.tar && rm web-dist.tar - - name: Run container-backed E2E tests shard ${{ matrix.shard }}/6 (Docker executor + SSH executor) + - name: Enable strict flaky-test diagnostics + if: github.event_name == 'workflow_dispatch' && inputs.fail_on_flaky + run: echo "E2E_FAIL_ON_FLAKY=1" >> "$GITHUB_ENV" + + - name: Download E2E shard manifests + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: e2e-shard-manifests + path: e2e-manifests + + - name: Run planned container-backed E2E tests shard ${{ matrix.shard }}/6 (Docker executor + SSH executor) working-directory: apps/web env: KANDEV_E2E_CONTAINERS: "1" - run: npx playwright test --config e2e/playwright.config.ts --project=containers --shard=${{ matrix.shard }}/6 - - - name: Cleanup leftover kandev containers - if: always() - run: docker ps -aq --filter label=kandev.managed=true | xargs -r docker rm -f + E2E_SHARD: ${{ matrix.shard }} + run: bash e2e/scripts/run-planned-shard.sh "$GITHUB_WORKSPACE/e2e-manifests/containers/${{ matrix.shard }}.json" # Named with the same `blob-report-*` prefix the merge job globs for # so the containers results land in the unified HTML report alongside @@ -487,6 +605,16 @@ jobs: working-directory: apps run: pnpm install --frozen-lockfile + # The published runtime image may predate the report-tool dependency; + # keep older images usable while the image rebuild adds unzip permanently. + - name: Ensure timing report tools are installed + run: | + if ! command -v unzip >/dev/null 2>&1; then + apt-get update + apt-get install -y --no-install-recommends unzip + rm -rf /var/lib/apt/lists/* + fi + - name: Download blob reports uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: @@ -494,7 +622,165 @@ jobs: pattern: blob-report-* merge-multiple: true + - name: Download E2E shard manifests + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + continue-on-error: true + with: + name: e2e-shard-manifests + path: e2e-manifests + + - name: Download selected main timing profile + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + continue-on-error: true + with: + name: e2e-selected-main-timing-profile + path: e2e/previous-profile + + - name: Collect timing and retry diagnostics + if: always() + working-directory: apps/web + run: | + set -euo pipefail + if [[ -f "$GITHUB_WORKSPACE/e2e/previous-profile/timing-profile.json" ]]; then + pnpm exec tsx e2e/scripts/e2e-timings.ts \ + --input e2e/blob-reports \ + --output e2e/timing-profile.json \ + --web-root "$GITHUB_WORKSPACE/apps/web" \ + --branch "$GITHUB_REF_NAME" \ + --sha "$GITHUB_SHA" \ + --run-id "$GITHUB_RUN_ID" \ + --previous "$GITHUB_WORKSPACE/e2e/previous-profile/timing-profile.json" + else + pnpm exec tsx e2e/scripts/e2e-timings.ts \ + --input e2e/blob-reports \ + --output e2e/timing-profile.json \ + --web-root "$GITHUB_WORKSPACE/apps/web" \ + --branch "$GITHUB_REF_NAME" \ + --sha "$GITHUB_SHA" \ + --run-id "$GITHUB_RUN_ID" + fi + pnpm exec tsx e2e/scripts/retry-summary.ts \ + --input e2e/blob-reports \ + --output e2e/retry-summary.json \ + --manifest-dir "$GITHUB_WORKSPACE/e2e-manifests" \ + --attachment-output e2e/retry-artifacts \ + --artifact-name e2e-timing-diagnostics \ + --artifact-url "$GITHUB_SERVER_URL/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID#artifacts" + + - name: Write E2E timing summary + if: always() + working-directory: apps/web + run: | + node <<'NODE' + const fs = require("node:fs"); + const summaryPath = "e2e/retry-summary.json"; + if (!fs.existsSync(summaryPath)) process.exit(0); + const summary = JSON.parse(fs.readFileSync(summaryPath, "utf8")); + const counts = summary.counts; + const diagnosticTests = summary.tests.filter( + (test) => test.attempts > 1 || !["passed", "skipped"].includes(test.finalStatus), + ); + const markdown = (value) => String(value).replaceAll("|", "\\|").replace(/\s+/g, " "); + const lines = [ + "## E2E timing and retry summary", + "", + "- Plan mode: " + summary.planning.mode, + "- Unknown units: " + summary.planning.unknownUnits, + "- Warm units: " + summary.planning.warmUnits, + "- Stale profile units: " + summary.planning.staleUnits, + "- Passed after retry: " + counts.passedAfterRetry, + "- Final failures: " + (counts.failed + counts.timedOut), + "", + "| Cohort | Shard | Predicted (s) | Actual (s) | Delta (s) | Exit |", + "| --- | ---: | ---: | ---: | ---: | ---: |", + ...summary.shards.map( + (shard) => + "| " + + shard.cohort + + " | " + + shard.shard + + "/" + + shard.shardCount + + " | " + + shard.predictedSeconds.toFixed(1) + + " | " + + shard.actualSeconds.toFixed(1) + + " | " + + shard.deltaSeconds.toFixed(1) + + " | " + + shard.exitCode + + " |", + ), + ]; + if (diagnosticTests.length > 0) { + lines.push( + "", + "### Retry and failure details", + "", + "| Test | Attempts | Final status | Error | Artifacts |", + "| --- | ---: | --- | --- | --- |", + ...diagnosticTests.map((test) => { + const links = test.attachments + .map((attachment) => + attachment.artifactUrl + ? "[" + markdown(attachment.name + " — " + attachment.path) + "](" + + attachment.artifactUrl + + ")" + : markdown(attachment.name), + ) + .join(", "); + return ( + "| `" + + markdown(test.key) + + "` | " + + test.attempts + + " | " + + test.finalStatus + + " | " + + test.errorCategory + + " | " + + (links || "Not captured") + + " |" + ); + }), + ); + } + fs.appendFileSync(process.env.GITHUB_STEP_SUMMARY, lines.join("\n") + "\n"); + NODE + + - name: Upload timing profile candidate + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: e2e-timing-profile + path: apps/web/e2e/timing-profile.json + if-no-files-found: ignore + retention-days: 30 + + - name: Upload retry and shard timing summary + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: e2e-retry-summary + path: apps/web/e2e/retry-summary.json + if-no-files-found: ignore + retention-days: 14 + + - name: Upload timing diagnostics bundle + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: e2e-timing-diagnostics + path: | + apps/web/e2e/timing-profile.json + apps/web/e2e/retry-summary.json + apps/web/e2e/retry-artifacts/ + apps/web/e2e/blob-reports/*metadata.json + if-no-files-found: ignore + retention-days: 14 + - name: Merge reports + if: hashFiles('apps/web/e2e/blob-reports/**/*.zip') != '' working-directory: apps/web # `-c e2e/playwright.config.ts` is required because the blobs come from # two different absolute checkout paths: the containerized `e2e` shards diff --git a/apps/backend/internal/agent/runtime/lifecycle/container.go b/apps/backend/internal/agent/runtime/lifecycle/container.go index dbfb2454cd..438eb1af3c 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/container.go +++ b/apps/backend/internal/agent/runtime/lifecycle/container.go @@ -27,6 +27,8 @@ const ( dockerAgentctlInstancePortBase = 41001 dockerAgentctlInstancePortMax = 41100 boolStringTrue = "true" + e2eDockerScopeEnv = "KANDEV_E2E_DOCKER_SCOPE" + e2eDockerScopeLabel = "kandev.e2e.run" gitHubCredentialHelperConfigKey = "credential.https://github.com.helper" remoteAgentctlExecutablePath = "/usr/local/bin/agentctl" ) @@ -545,6 +547,9 @@ exec /usr/local/bin/agentctl`, }, AutoRemove: false, // We manage cleanup ourselves } + if scope := os.Getenv(e2eDockerScopeEnv); scope != "" { + containerCfg.Labels[e2eDockerScopeLabel] = scope + } if config.ExecutorProfileID != "" { containerCfg.Labels["kandev.executor_profile_id"] = config.ExecutorProfileID @@ -726,9 +731,11 @@ func generateBootstrapNonce() (string, error) { // ListManagedContainers returns all containers managed by kandev func (cm *ContainerManager) ListManagedContainers(ctx context.Context) ([]docker.ContainerInfo, error) { - return cm.dockerClient.ListContainers(ctx, map[string]string{ - "kandev.managed": boolStringTrue, - }) + labels := map[string]string{"kandev.managed": boolStringTrue} + if scope := os.Getenv(e2eDockerScopeEnv); scope != "" { + labels[e2eDockerScopeLabel] = scope + } + return cm.dockerClient.ListContainers(ctx, labels) } // GetContainerInfo returns information about a specific container diff --git a/apps/backend/internal/agent/runtime/lifecycle/container_config_test.go b/apps/backend/internal/agent/runtime/lifecycle/container_config_test.go index 9c2a7b9e9e..71f97911c9 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/container_config_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/container_config_test.go @@ -220,6 +220,21 @@ func TestBuildContainerConfig_LabelsExecutorProfileAndTaskEnvironment(t *testing assertLabel(t, got.Labels, "com.kandev.image", "kandev/agent:custom") } +func TestBuildContainerConfig_LabelsE2EDockerScope(t *testing.T) { + t.Setenv("KANDEV_E2E_DOCKER_SCOPE", "e2e-test-scope") + cm := newCMTest(t) + got, err := cm.buildContainerConfig(ContainerConfig{ + AgentConfig: newConfigStubAgent(), + InstanceID: "0123456789abcdef", + TaskID: "task-1", + }) + if err != nil { + t.Fatalf("buildContainerConfig: %v", err) + } + + assertLabel(t, got.Labels, "kandev.e2e.run", "e2e-test-scope") +} + func TestBuildContainerConfig_PublishesAgentctlPorts(t *testing.T) { cm := newCMTest(t) cfg := ContainerConfig{ diff --git a/apps/backend/internal/backendapp/storage_docker.go b/apps/backend/internal/backendapp/storage_docker.go index 3bbdf230db..22f065b361 100644 --- a/apps/backend/internal/backendapp/storage_docker.go +++ b/apps/backend/internal/backendapp/storage_docker.go @@ -3,6 +3,7 @@ package backendapp import ( "context" "errors" + "os" "time" agentdocker "github.com/kandev/kandev/internal/agent/docker" @@ -10,6 +11,11 @@ import ( "github.com/kandev/kandev/internal/system/storage/dockerstore" ) +const ( + e2eDockerScopeEnv = "KANDEV_E2E_DOCKER_SCOPE" + e2eDockerScopeLabel = "kandev.e2e.run" +) + type lazyStorageDocker struct { provider func() *agentdocker.Client activity *activity.Coordinator @@ -43,7 +49,7 @@ func (d *lazyStorageDocker) ListContainers( if err != nil { return nil, err } - return client.ListContainers(ctx, labels) + return client.ListContainers(ctx, scopedDockerLabels(labels)) } func (d *lazyStorageDocker) RemoveContainer(ctx context.Context, id string, force bool) error { @@ -59,7 +65,39 @@ func (d *lazyStorageDocker) DiskUsage(ctx context.Context) (agentdocker.DiskUsag if err != nil { return agentdocker.DiskUsage{}, err } - return client.DiskUsage(ctx) + usage, err := client.DiskUsage(ctx) + if err != nil { + return agentdocker.DiskUsage{}, err + } + return scopeDockerUsage(usage), nil +} + +func scopedDockerLabels(labels map[string]string) map[string]string { + scope := os.Getenv(e2eDockerScopeEnv) + if scope == "" { + return labels + } + scoped := make(map[string]string, len(labels)+1) + for key, value := range labels { + scoped[key] = value + } + scoped[e2eDockerScopeLabel] = scope + return scoped +} + +func scopeDockerUsage(usage agentdocker.DiskUsage) agentdocker.DiskUsage { + scope := os.Getenv(e2eDockerScopeEnv) + if scope == "" { + return usage + } + containers := make([]agentdocker.ContainerUsage, 0, len(usage.Containers)) + for _, container := range usage.Containers { + if container.Labels[e2eDockerScopeLabel] == scope { + containers = append(containers, container) + } + } + usage.Containers = containers + return usage } func (d *lazyStorageDocker) PruneBuildCache( diff --git a/apps/backend/internal/backendapp/storage_docker_test.go b/apps/backend/internal/backendapp/storage_docker_test.go new file mode 100644 index 0000000000..30ff26abbe --- /dev/null +++ b/apps/backend/internal/backendapp/storage_docker_test.go @@ -0,0 +1,45 @@ +package backendapp + +import ( + "testing" + + agentdocker "github.com/kandev/kandev/internal/agent/docker" +) + +func TestScopedDockerLabelsAddsOnlyTheE2EScope(t *testing.T) { + t.Setenv("KANDEV_E2E_DOCKER_SCOPE", "e2e-test-scope") + input := map[string]string{"kandev.managed": "true"} + + got := scopedDockerLabels(input) + + if got["kandev.e2e.run"] != "e2e-test-scope" { + t.Fatalf("scope label = %q, want e2e-test-scope", got["kandev.e2e.run"]) + } + if got["kandev.managed"] != "true" { + t.Fatalf("managed label = %q, want true", got["kandev.managed"]) + } + if _, ok := input["kandev.e2e.run"]; ok { + t.Fatal("scoping must not mutate the caller's label map") + } +} + +func TestScopeDockerUsageFiltersOnlyContainerRecords(t *testing.T) { + t.Setenv("KANDEV_E2E_DOCKER_SCOPE", "e2e-test-scope") + usage := agentdocker.DiskUsage{ + ImageLayerBytes: 10, + Images: []agentdocker.ImageUsage{{ID: "image"}}, + Containers: []agentdocker.ContainerUsage{ + {ID: "owned", Labels: map[string]string{"kandev.e2e.run": "e2e-test-scope"}}, + {ID: "other", Labels: map[string]string{"kandev.e2e.run": "other-scope"}}, + }, + } + + scoped := scopeDockerUsage(usage) + + if len(scoped.Containers) != 1 || scoped.Containers[0].ID != "owned" { + t.Fatalf("scoped containers = %#v, want only owned record", scoped.Containers) + } + if len(scoped.Images) != 1 || scoped.ImageLayerBytes != 10 { + t.Fatalf("image usage changed while scoping containers: %#v", scoped) + } +} diff --git a/apps/packages/ui/src/dropdown-menu.tsx b/apps/packages/ui/src/dropdown-menu.tsx index 561afde215..db3f74874f 100644 --- a/apps/packages/ui/src/dropdown-menu.tsx +++ b/apps/packages/ui/src/dropdown-menu.tsx @@ -22,27 +22,34 @@ function DropdownMenuTrigger({ return ; } -function DropdownMenuContent({ - className, - align = "start", - sideOffset = 4, - ...props -}: React.ComponentProps) { - return ( - - - - ); -} +type DropdownMenuContentProps = React.ComponentPropsWithoutRef< + typeof DropdownMenuPrimitive.Content +> & { + // Radix forwards this Menu.Content focus event at runtime, but its dropdown + // facade omits the prop from the public declaration. + onEntryFocus?: (event: Event) => void; + onOpenAutoFocus?: (event: Event) => void; +}; + +const DropdownMenuContent = React.forwardRef( + function DropdownMenuContent({ className, align = "start", sideOffset = 4, ...props }, ref) { + return ( + + + + ); + }, +); function DropdownMenuGroup({ ...props }: React.ComponentProps) { return ; diff --git a/apps/web/components/app-sidebar/app-sidebar-workspace-picker.tsx b/apps/web/components/app-sidebar/app-sidebar-workspace-picker.tsx index 567a1cfa50..2dad3768ba 100644 --- a/apps/web/components/app-sidebar/app-sidebar-workspace-picker.tsx +++ b/apps/web/components/app-sidebar/app-sidebar-workspace-picker.tsx @@ -1,6 +1,6 @@ "use client"; -import { useCallback, useState } from "react"; +import { useCallback, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import { useRouter } from "@/lib/routing/client-router"; import { DropdownMenu, DropdownMenuContent, DropdownMenuTrigger } from "@kandev/ui/dropdown-menu"; @@ -72,6 +72,29 @@ function useMenuOpenState(controlledOpen?: boolean, onOpenChange?: (open: boolea return { open: isControlled ? controlledOpen : uncontrolledOpen, setOpen }; } +function useControlledMenuFocus(controlledOpen: boolean | undefined) { + const contentRef = useRef(null); + const handleOpenAutoFocus = useCallback( + (event: Event) => { + if (controlledOpen === undefined) return; + event.preventDefault(); + contentRef.current?.focus({ preventScroll: true }); + }, + [controlledOpen], + ); + const handleEntryFocus = useCallback( + (event: Event) => { + if (controlledOpen !== undefined) event.preventDefault(); + }, + [controlledOpen], + ); + return { + ref: contentRef, + onOpenAutoFocus: handleOpenAutoFocus, + onEntryFocus: handleEntryFocus, + }; +} + function rememberSelectedWorkspace(workspace: WorkspaceItem) { if (workspaceType(workspace) === "office") { rememberLastOfficeWorkspace(workspace); @@ -103,6 +126,7 @@ export function AppSidebarWorkspacePicker({ const activeWorkspace = workspaces.items.find((w) => w.id === workspaces.activeId); const activeId = activeWorkspace?.id ?? null; const activeName = activeWorkspace?.name ?? t("sidebar:workspaceFallback"); + const menuFocus = useControlledMenuFocus(controlledOpen); const handleSelect = useCallback( (workspace: WorkspaceItem) => { @@ -166,7 +190,11 @@ export function AppSidebarWorkspacePicker({ className={triggerClassName} /> - + ({ // binaries the test runner pre-built, so containers can bind-mount them. ...(dockerEnabled ? { + KANDEV_E2E_DOCKER_SCOPE: E2E_DOCKER_SCOPE, KANDEV_AGENTCTL_LINUX_BINARY: agentctlLinuxBinary, KANDEV_MOCK_AGENT_LINUX_BINARY: mockAgentLinuxBinary, } diff --git a/apps/web/e2e/fixtures/docker-probe.ts b/apps/web/e2e/fixtures/docker-probe.ts index 4672839ca4..86ed843a1b 100644 --- a/apps/web/e2e/fixtures/docker-probe.ts +++ b/apps/web/e2e/fixtures/docker-probe.ts @@ -1,4 +1,5 @@ import { execFileSync, spawnSync } from "node:child_process"; +import { randomUUID } from "node:crypto"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; @@ -8,6 +9,8 @@ import path from "node:path"; * Tag is stable so layer caches survive across runs. */ export const E2E_IMAGE_TAG = "kandev-agent:e2e"; +/** Label value shared by resources created in this Playwright process. */ +export const E2E_DOCKER_SCOPE = `e2e-${process.pid}-${randomUUID().slice(0, 8)}`; const FAKE_LSP_SERVER = path.resolve(__dirname, "fake-lsp-server.mjs"); const E2E_DOCKERFILE = `FROM node:22-slim @@ -46,14 +49,22 @@ export function buildE2EImage(): void { } /** - * Remove all containers labeled as managed by kandev. Called at worker - * teardown to clean up anything the suite leaked. Safe to run on a host with - * no matching containers. + * Remove only containers owned by this E2E process. Never use a daemon-wide + * kandev.managed=true sweep: another shard can share the Docker daemon. */ -export function removeKandevContainers(): void { - const list = spawnSync("docker", ["ps", "-aq", "--filter", "label=kandev.managed=true"], { - encoding: "utf8", - }); +export function removeScopedKandevContainers(scope = E2E_DOCKER_SCOPE): void { + const list = spawnSync( + "docker", + [ + "ps", + "-aq", + "--filter", + "label=kandev.managed=true", + "--filter", + `label=kandev.e2e.run=${scope}`, + ], + { encoding: "utf8" }, + ); if (list.status !== 0) return; const ids = list.stdout .split("\n") diff --git a/apps/web/e2e/fixtures/docker-test-base.ts b/apps/web/e2e/fixtures/docker-test-base.ts index 9fa650b31e..d9501a7c80 100644 --- a/apps/web/e2e/fixtures/docker-test-base.ts +++ b/apps/web/e2e/fixtures/docker-test-base.ts @@ -3,7 +3,12 @@ import { execSync } from "node:child_process"; import fs from "node:fs"; import path from "node:path"; import { backendFixture, type BackendContext } from "./backend"; -import { buildE2EImage, E2E_IMAGE_TAG, hasDocker, removeKandevContainers } from "./docker-probe"; +import { + buildE2EImage, + E2E_IMAGE_TAG, + hasDocker, + removeScopedKandevContainers, +} from "./docker-probe"; import { ApiClient } from "../helpers/api-client"; import { makeGitEnv } from "../helpers/git-helper"; import type { WorkflowStep } from "../../lib/types/http"; @@ -104,7 +109,9 @@ export const dockerTest = backendFixture.extend< dockerExecutorProfileId: dockerProfile.id, }); } finally { - removeKandevContainers(); + // The backend owns containers created by the test task. Do not sweep + // the daemon here: another E2E shard may be using it concurrently. + removeScopedKandevContainers(); } }, { scope: "worker", timeout: 120_000 }, @@ -144,13 +151,19 @@ export const dockerTest = backendFixture.extend< // lazy fixture, so keeping reset there allowed tests that only request // apiClient/seedData to leave tasks and containers behind for the next test. dockerTest.beforeEach(async ({ apiClient, seedData }) => { - await apiClient.e2eReset(seedData.workspaceId, [seedData.workflowId]); - removeKandevContainers(); + try { + await apiClient.e2eReset(seedData.workspaceId, [seedData.workflowId]); + } finally { + removeScopedKandevContainers(); + } }); dockerTest.afterEach(async ({ apiClient, seedData }) => { - await apiClient.e2eReset(seedData.workspaceId, [seedData.workflowId]); - removeKandevContainers(); + try { + await apiClient.e2eReset(seedData.workspaceId, [seedData.workflowId]); + } finally { + removeScopedKandevContainers(); + } }); export { expect } from "@playwright/test"; diff --git a/apps/web/e2e/helpers/multi-pr-review.test.ts b/apps/web/e2e/helpers/multi-pr-review.test.ts new file mode 100644 index 0000000000..c1829e8cb8 --- /dev/null +++ b/apps/web/e2e/helpers/multi-pr-review.test.ts @@ -0,0 +1,9 @@ +import { describe, expect, it } from "vitest"; +import { reviewRepositoryName } from "./multi-pr-review"; + +describe("multi-PR review repository names", () => { + it("normalizes repository IDs while preserving distinct names", () => { + expect(reviewRepositoryName({ repositoryId: "Repo/ID #A" })).toBe("e2e-review-repoida"); + expect(reviewRepositoryName({ repositoryId: "Repo/ID #B" })).toBe("e2e-review-repoidb"); + }); +}); diff --git a/apps/web/e2e/helpers/multi-pr-review.ts b/apps/web/e2e/helpers/multi-pr-review.ts index e5481f1dbc..bf97af5474 100644 --- a/apps/web/e2e/helpers/multi-pr-review.ts +++ b/apps/web/e2e/helpers/multi-pr-review.ts @@ -2,9 +2,14 @@ import type { SeedData } from "../fixtures/test-base"; import type { ApiClient } from "./api-client"; export const REVIEW_OWNER = "testorg"; -export const REVIEW_REPO = "testrepo"; export const REVIEW_SHARED_FILE = "shared-pr.ts"; +/** Keep the mocked GitHub repository tied to the worker's seeded repository. */ +export function reviewRepositoryName(seedData: Pick): string { + const suffix = seedData.repositoryId.replace(/[^a-zA-Z0-9]/g, "").slice(0, 12); + return `e2e-review-${suffix.toLowerCase()}`; +} + export const REVIEW_PRS = [ { number: 121, @@ -28,6 +33,7 @@ export async function seedMultiPRReviewTask( title: string, description = "/e2e:simple-message", ) { + const repositoryName = reviewRepositoryName(seedData); await apiClient.mockGitHubReset(); await apiClient.mockGitHubSetUser("reviewer"); @@ -57,15 +63,15 @@ export async function seedMultiPRReviewTask( base_branch: "main", author_login: "reviewer", repo_owner: REVIEW_OWNER, - repo_name: REVIEW_REPO, - html_url: `https://github.com/${REVIEW_OWNER}/${REVIEW_REPO}/pull/${pr.number}`, + repo_name: repositoryName, + html_url: `https://github.com/${REVIEW_OWNER}/${repositoryName}/pull/${pr.number}`, additions: 1, deletions: 0, })), ); for (const pr of REVIEW_PRS) { - await apiClient.mockGitHubAddPRFiles(REVIEW_OWNER, REVIEW_REPO, pr.number, [ + await apiClient.mockGitHubAddPRFiles(REVIEW_OWNER, repositoryName, pr.number, [ { filename: REVIEW_SHARED_FILE, status: "added", @@ -78,7 +84,7 @@ export async function seedMultiPRReviewTask( workspace_id: seedData.workspaceId, task_id: task.id, repository_id: seedData.repositoryId, - pr_url: `https://github.com/${REVIEW_OWNER}/${REVIEW_REPO}/pull/${pr.number}`, + pr_url: `https://github.com/${REVIEW_OWNER}/${repositoryName}/pull/${pr.number}`, }); } diff --git a/apps/web/e2e/helpers/ssh-bastion.ts b/apps/web/e2e/helpers/ssh-bastion.ts index b05e8ee9cd..de923f83dc 100644 --- a/apps/web/e2e/helpers/ssh-bastion.ts +++ b/apps/web/e2e/helpers/ssh-bastion.ts @@ -3,6 +3,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { SSH_E2E_IMAGE_TAG } from "../fixtures/ssh-image"; +import { E2E_DOCKER_SCOPE } from "../fixtures/docker-probe"; import type { SSHServerHandle } from "./ssh"; /** @@ -36,7 +37,7 @@ const BASTION_BASE_PORT = 23000; export function startBastionAndTarget(workerIndex: number, workDir: string): SSHBastionHandles { fs.mkdirSync(workDir, { recursive: true }); - const networkName = `kandev-e2e-ssh-net-${workerIndex}`; + const networkName = `kandev-e2e-ssh-net-${E2E_DOCKER_SCOPE}-${workerIndex}`; ensureNetwork(networkName); // The SSH executor's ProxyJump string doesn't carry a separate identity for @@ -48,7 +49,7 @@ export function startBastionAndTarget(workerIndex: number, workDir: string): SSH const bastionPort = BASTION_BASE_PORT + workerIndex * 2; const bastion = launchInNetwork({ - name: `kandev-bastion-${workerIndex}`, + name: `kandev-bastion-${E2E_DOCKER_SCOPE}-${workerIndex}`, role: "bastion", networkName, hostPort: bastionPort, @@ -57,7 +58,7 @@ export function startBastionAndTarget(workerIndex: number, workDir: string): SSH }); const target = launchInNetwork({ - name: `kandev-target-${workerIndex}`, + name: `kandev-target-${E2E_DOCKER_SCOPE}-${workerIndex}`, role: "target", networkName, hostPort: null, // not reachable from host @@ -119,6 +120,8 @@ function launchInNetwork(opts: LaunchOpts): SSHServerHandle { "--label", "kandev.managed=true", "--label", + `kandev.e2e.run=${E2E_DOCKER_SCOPE}`, + "--label", `kandev.e2e.role=ssh-${opts.role}`, "--cap-add", "NET_ADMIN", diff --git a/apps/web/e2e/helpers/ssh.ts b/apps/web/e2e/helpers/ssh.ts index 6c6ad4ef7b..9cbab567ba 100644 --- a/apps/web/e2e/helpers/ssh.ts +++ b/apps/web/e2e/helpers/ssh.ts @@ -2,6 +2,7 @@ import { execFileSync, spawnSync } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { E2E_DOCKER_SCOPE } from "../fixtures/docker-probe"; /** * Live SSH server handle returned by startSSHServer. Tests use the host/port @@ -34,8 +35,8 @@ const SSH_BASE_PORT = 22000; * Start a fresh sshd container for a Playwright worker. Each worker gets: * - A unique published port (SSH_BASE_PORT + workerIndex). * - A unique ed25519 keypair generated into workDir. - * - A unique container name `kandev-sshd-e2e-` (labeled - * `kandev.managed=true` so the shared cleanup hook destroys it). + * - A process-scoped container name and ownership label so cleanup cannot + * remove another shard's SSH target. * * Blocks until sshd answers TCP connections and a stable host fingerprint * is observable via `ssh-keyscan`. @@ -47,7 +48,7 @@ export function startSSHServer( ): SSHServerHandle { fs.mkdirSync(workDir, { recursive: true }); const port = SSH_BASE_PORT + workerIndex; - const containerName = `kandev-sshd-e2e-${workerIndex}`; + const containerName = `kandev-sshd-e2e-${E2E_DOCKER_SCOPE}-${workerIndex}`; const identityFile = path.join(workDir, "id_ed25519"); const publicKeyFile = `${identityFile}.pub`; @@ -74,6 +75,8 @@ export function startSSHServer( "--label", "kandev.managed=true", "--label", + `kandev.e2e.run=${E2E_DOCKER_SCOPE}`, + "--label", "kandev.e2e.role=ssh-target", "--cap-add", "NET_ADMIN", // iptables-based fault injection diff --git a/apps/web/e2e/playwright.config.ts b/apps/web/e2e/playwright.config.ts index efab9c9d42..280438071f 100644 --- a/apps/web/e2e/playwright.config.ts +++ b/apps/web/e2e/playwright.config.ts @@ -4,10 +4,9 @@ const CI = !!process.env.CI; export default defineConfig({ testDir: "./tests", - // `fullyParallel: true` is required so `--shard=N/M` splits work at the test - // level (not the file level). With file-level sharding the suite was wildly - // unbalanced: largest shard ran 12 min, smallest 1.5 min, because spec files - // vary from 1 to 30+ tests. Test-level sharding flattens that distribution. + // `fullyParallel: true` keeps direct local `--shard=N/M` runs at test level. + // CI uses explicit duration-aware file manifests from e2e/scripts instead; + // this setting preserves the existing local debugging behavior. // // Concurrency is still capped by `workers: 1` below — only one test runs at a // time per shard process, preserving the worker-scoped backend invariant that @@ -24,7 +23,7 @@ export default defineConfig({ // the fixture's baseline env snapshot). fullyParallel: true, forbidOnly: CI, - failOnFlakyTests: !CI, + failOnFlakyTests: !CI || process.env.E2E_FAIL_ON_FLAKY === "1", retries: CI ? 2 : 0, workers: 1, timeout: 60_000, @@ -96,13 +95,10 @@ export default defineConfig({ testMatch: [/docker\/.*\.spec\.ts/, /ssh\/.*\.spec\.ts/], use: { ...devices["Desktop Chrome"] }, timeout: 180_000, - // Per-test sharding: CI runs `--shard=N/6` to split this project's - // tests across runners. Playwright only shards at the test level - // when fullyParallel is true (otherwise sharding is by file, and - // since this project has a single spec file every test would land - // in shard 1). Each shard is its own process with its own backend, - // and workers:1 still serializes tests within a shard, so this is - // safe. + // Local `--shard=N/6` runs can still split this project at test level. + // CI uses explicit files from the duration-aware containers manifests. + // Each CI shard is its own process, and workers:1 still serializes tests + // within a shard, so the worker-scoped backend remains safe. fullyParallel: true, }, ], diff --git a/apps/web/e2e/scripts/e2e-timings.test.ts b/apps/web/e2e/scripts/e2e-timings.test.ts new file mode 100644 index 0000000000..7c9d21e5a6 --- /dev/null +++ b/apps/web/e2e/scripts/e2e-timings.test.ts @@ -0,0 +1,253 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { + buildTimingProfile, + collectEligibleSamples, + hasCompleteShardSet, + normalizeReportPath, + parseBlobReportJsonl, + quantile, + readBlobReportJsonl, + readTimingProfile, + type ShardMetadata, + type TimingObservation, + type TimingProfile, +} from "./e2e-timings"; + +function event(method: string, params: Record): string { + return JSON.stringify({ method, params }); +} + +function reportJsonl(): string { + const project = { + name: "chromium", + suites: [ + { + title: "", + location: { file: "tests/chat/example.spec.ts", line: 1, column: 1 }, + entries: [ + { + testId: "test-1", + title: "shows the conversation", + location: { file: "tests/chat/example.spec.ts", line: 10, column: 1 }, + }, + ], + }, + ], + }; + + return [ + event("onBlobReportMetadata", { version: 2 }), + event("onProject", { project }), + event("onTestBegin", { + testId: "test-1", + result: { id: "result-1", retry: 0, workerIndex: 0, parallelIndex: 0, startTime: 1 }, + }), + event("onTestEnd", { + test: { testId: "test-1", expectedStatus: "passed", timeout: 60_000 }, + result: { id: "result-1", duration: 1200, status: "passed", errors: [] }, + }), + event("onTestBegin", { + testId: "test-1", + result: { id: "result-2", retry: 1, workerIndex: 0, parallelIndex: 0, startTime: 2 }, + }), + event("onTestEnd", { + test: { testId: "test-1", expectedStatus: "passed", timeout: 60_000 }, + result: { id: "result-2", duration: 6000, status: "passed", errors: [] }, + }), + ].join("\n"); +} + +describe("e2e timing collection", () => { + it("normalizes paths relative to Playwright's e2e test root", () => { + expect(normalizeReportPath("task/example.spec.ts")).toBe("tests/task/example.spec.ts"); + expect(normalizeReportPath("e2e/tests/task/example.spec.ts")).toBe( + "tests/task/example.spec.ts", + ); + }); + + it("parses project, file, title, retry, and duration from blob events", () => { + const observations = parseBlobReportJsonl(reportJsonl()); + + expect(observations).toHaveLength(2); + expect(observations[0]).toMatchObject({ + project: "chromium", + file: "tests/chat/example.spec.ts", + title: "shows the conversation", + retry: 0, + status: "passed", + durationSeconds: 1.2, + }); + expect(observations[1]?.retry).toBe(1); + }); + + it("reads an unpacked blob report", () => { + const directory = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-blob-report-")); + const filePath = path.join(directory, "report.jsonl"); + fs.writeFileSync(filePath, reportJsonl()); + + expect(readBlobReportJsonl(filePath)).toBe(reportJsonl()); + }); + + it("uses only first-attempt passing results as baseline samples", () => { + const observations = parseBlobReportJsonl(reportJsonl()); + + expect(collectEligibleSamples(observations)).toEqual([ + expect.objectContaining({ durationSeconds: 1.2, retry: 0 }), + ]); + }); + + it("computes interpolated quantiles", () => { + expect(quantile([1, 2, 3, 4], 0.5)).toBe(2.5); + expect(quantile([1, 2, 3, 4], 0.75)).toBe(3.25); + expect(quantile([], 0.75)).toBe(0); + }); + + it.each([ + ["missing percentile", { p75Seconds: undefined }], + ["non-numeric percentile", { p75Seconds: "slow" }], + ["non-finite sample", { samples: [{ durationSeconds: null }] }], + ])("rejects a malformed timing profile with %s", (_name, override) => { + const webRoot = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-profile-")); + const filePath = path.join(webRoot, "timing-profile.json"); + const entry = { ...previousTimingEntry(), ...override }; + fs.writeFileSync(filePath, JSON.stringify({ ...previousTimingProfile(), entries: [entry] })); + + expect(() => readTimingProfile(filePath)).toThrow(/Unsupported timing profile version/); + }); + + it("only treats a complete normal and container shard set as a deletion catalog", () => { + const metadata = (cohort: string, shard: number, shardCount: number): ShardMetadata => ({ + cohort, + shard, + shardCount, + startedAt: "2026-08-10T10:00:00.000Z", + finishedAt: "2026-08-10T10:00:01.000Z", + exitCode: 0, + predictedSeconds: 1, + unitIds: [], + }); + const complete = [ + ...Array.from({ length: 14 }, (_, index) => metadata("normal", index + 1, 14)), + ...Array.from({ length: 6 }, (_, index) => metadata("containers", index + 1, 6)), + ]; + + expect(hasCompleteShardSet(complete)).toBe(true); + expect(hasCompleteShardSet(complete.slice(0, -1))).toBe(false); + }); + + it("merges and bounds timing history", () => { + const previous: TimingProfile = { + version: 1, + generatedAt: "2026-08-09T00:00:00.000Z", + source: { branch: "main", sha: "old-sha", runId: "10" }, + entries: [ + { + key: "chromium::tests/chat/example.spec.ts::shows the conversation", + project: "chromium", + file: "tests/chat/example.spec.ts", + title: "shows the conversation", + fileHash: "hash-old", + samples: Array.from({ length: 20 }, (_, index) => ({ + durationSeconds: index + 1, + sourceSha: "old-sha", + runId: "10", + recordedAt: "2026-08-09T00:00:00.000Z", + })), + p50Seconds: 10.5, + p75Seconds: 15.25, + lastSeenSha: "old-sha", + lastSeenRunId: "10", + }, + ], + }; + const observations: TimingObservation[] = [ + { + key: "chromium::tests/chat/example.spec.ts::shows the conversation", + project: "chromium", + file: "tests/chat/example.spec.ts", + title: "shows the conversation", + retry: 0, + status: "passed", + durationSeconds: 100, + errors: [], + attachments: [], + }, + ]; + + const profile = buildTimingProfile(previous, observations, { + branch: "main", + sha: "new-sha", + runId: "11", + generatedAt: "2026-08-10T00:00:00.000Z", + fileHashes: new Map([["tests/chat/example.spec.ts", "hash-new"]]), + knownKeys: new Set([observations[0]!.key]), + }); + + expect(profile.entries[0]?.samples).toHaveLength(20); + expect(profile.entries[0]?.samples.at(-1)?.durationSeconds).toBe(100); + expect(profile.entries[0]?.fileHash).toBe("hash-new"); + expect(profile.entries[0]?.p75Seconds).toBeGreaterThan(15); + }); + + it("compacts previous entries that are no longer in the current catalog", () => { + const currentKey = "chromium::tests/chat/example.spec.ts::shows the conversation"; + const staleEntry = { + ...previousTimingEntry(), + key: "chromium::tests/removed.spec.ts::removed test", + file: "tests/removed.spec.ts", + }; + const profile = buildTimingProfile( + { ...previousTimingProfile(), entries: [previousTimingEntry(), staleEntry] }, + [ + { + key: currentKey, + project: "chromium", + file: "tests/chat/example.spec.ts", + title: "shows the conversation", + retry: 0, + status: "passed", + durationSeconds: 2, + errors: [], + attachments: [], + }, + ], + { + branch: "main", + sha: "new-sha", + runId: "11", + generatedAt: "2026-08-10T00:00:00.000Z", + fileHashes: new Map([["tests/chat/example.spec.ts", "hash-new"]]), + knownKeys: new Set([currentKey]), + }, + ); + + expect(profile.entries.map((entry) => entry.key)).toEqual([currentKey]); + }); +}); + +function previousTimingEntry(): TimingProfile["entries"][number] { + return { + key: "chromium::tests/chat/example.spec.ts::shows the conversation", + project: "chromium", + file: "tests/chat/example.spec.ts", + title: "shows the conversation", + fileHash: "hash-old", + samples: [], + p50Seconds: 1, + p75Seconds: 1, + lastSeenSha: "old-sha", + lastSeenRunId: "10", + }; +} + +function previousTimingProfile(): TimingProfile { + return { + version: 1, + generatedAt: "2026-08-09T00:00:00.000Z", + source: { branch: "main", sha: "old-sha", runId: "10" }, + entries: [previousTimingEntry()], + }; +} diff --git a/apps/web/e2e/scripts/e2e-timings.ts b/apps/web/e2e/scripts/e2e-timings.ts new file mode 100644 index 0000000000..0bc8a163c4 --- /dev/null +++ b/apps/web/e2e/scripts/e2e-timings.ts @@ -0,0 +1,581 @@ +import { execFileSync } from "node:child_process"; +import { createHash } from "node:crypto"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +export const TIMING_PROFILE_VERSION = 1; +export const MAX_SAMPLES_PER_TEST = 20; + +export type TimingStatus = "passed" | "failed" | "timedOut" | "skipped" | "interrupted" | "unknown"; + +export type TimingError = { + message?: string; + value?: string; + stack?: string; +}; + +export type TimingAttachment = { + name: string; + path?: string; + body?: string; +}; + +export type TimingObservation = { + key: string; + project: string; + file: string; + title: string; + retry: number; + status: TimingStatus; + durationSeconds: number; + errors: TimingError[]; + attachments: TimingAttachment[]; +}; + +export type TimingSample = { + durationSeconds: number; + sourceSha: string; + runId: string; + recordedAt: string; +}; + +export type TimingEntry = { + key: string; + project: string; + file: string; + title: string; + fileHash: string; + samples: TimingSample[]; + p50Seconds: number; + p75Seconds: number; + lastSeenSha: string; + lastSeenRunId: string; +}; + +export type TimingProfile = { + version: typeof TIMING_PROFILE_VERSION; + generatedAt: string; + source: { + branch: string; + sha: string; + runId: string; + }; + entries: TimingEntry[]; +}; + +export type TimingProfileBuildMetadata = { + branch: string; + sha: string; + runId: string; + generatedAt: string; + fileHashes: Map; + knownKeys?: Set; +}; + +export type ShardMetadata = { + cohort: string; + shard: number; + shardCount: number; + startedAt: string; + finishedAt: string; + exitCode: number; + predictedSeconds: number; + unitIds: string[]; +}; + +type JsonRecord = Record; + +type TestDescriptor = { + key: string; + project: string; + file: string; + title: string; +}; + +function asRecord(value: unknown): JsonRecord | undefined { + return value && typeof value === "object" ? (value as JsonRecord) : undefined; +} + +function asString(value: unknown, fallback = ""): string { + return typeof value === "string" ? value : fallback; +} + +function asNumber(value: unknown, fallback = 0): number { + return typeof value === "number" && Number.isFinite(value) ? value : fallback; +} + +function isFiniteDuration(value: unknown): value is number { + return typeof value === "number" && Number.isFinite(value) && value >= 0; +} + +export function normalizeReportPath(file: string): string { + const normalized = file.replaceAll("\\", "/").replace(/^\.\//, ""); + const testsIndex = normalized.indexOf("tests/"); + if (testsIndex >= 0) return normalized.slice(testsIndex); + const e2eIndex = normalized.indexOf("e2e/"); + if (e2eIndex >= 0) return normalized.slice(e2eIndex + "e2e/".length); + return normalized ? `tests/${normalized}` : normalized; +} + +export function stableTestKey(project: string, file: string, title: string): string { + return `${project}::${normalizeReportPath(file)}::${title}`; +} + +function collectProjectTests(project: JsonRecord, descriptors: Map): void { + const projectName = asString(project.name, "unknown"); + + const walkSuite = (suite: JsonRecord, parentTitles: string[]) => { + const suiteTitle = asString(suite.title); + const titles = suiteTitle ? [...parentTitles, suiteTitle] : parentTitles; + const entries = Array.isArray(suite.entries) ? suite.entries : []; + for (const rawEntry of entries) { + const entry = asRecord(rawEntry); + if (!entry) continue; + + const testId = asString(entry.testId); + if (testId) { + const location = asRecord(entry.location); + const file = normalizeReportPath(asString(location?.file)); + const fileName = path.posix.basename(file); + const title = [...titles.filter((value) => value !== fileName), asString(entry.title)] + .filter(Boolean) + .join(" › "); + descriptors.set(testId, { + key: stableTestKey(projectName, file, title), + project: projectName, + file, + title, + }); + continue; + } + + walkSuite(entry, titles); + } + }; + + const suites = Array.isArray(project.suites) ? project.suites : []; + for (const rawSuite of suites) { + const suite = asRecord(rawSuite); + if (suite) walkSuite(suite, []); + } +} + +function parseJsonLines(content: string): JsonRecord[] { + const events: JsonRecord[] = []; + for (const line of content.split(/\r?\n/)) { + if (!line.trim()) continue; + try { + const parsed = JSON.parse(line) as unknown; + const event = asRecord(parsed); + if (event) events.push(event); + } catch { + // A truncated report should not make all other shard timings unusable. + } + } + return events; +} + +type BlobParseState = { + descriptors: Map; + retries: Map; + attachments: Map; + observations: TimingObservation[]; +}; + +function parseTimingStatus(value: unknown): TimingStatus { + switch (asString(value)) { + case "passed": + case "failed": + case "timedOut": + case "skipped": + case "interrupted": + return asString(value) as TimingStatus; + default: + return "unknown"; + } +} + +function parseTimingErrors(value: unknown): TimingError[] { + if (!Array.isArray(value)) return []; + return value.flatMap((rawError) => { + const error = asRecord(rawError); + return error + ? [ + { + message: asString(error.message) || undefined, + value: asString(error.value) || undefined, + stack: asString(error.stack) || undefined, + }, + ] + : []; + }); +} + +function parseTimingAttachment(value: unknown): TimingAttachment | undefined { + const attachment = asRecord(value); + if (!attachment) return undefined; + return { + name: asString(attachment.name, "attachment"), + path: asString(attachment.path) || undefined, + body: asString(attachment.base64) || undefined, + }; +} + +function handleProjectEvent(state: BlobParseState, params: JsonRecord): void { + const project = asRecord(params.project); + if (project) collectProjectTests(project, state.descriptors); +} + +function handleTestBeginEvent(state: BlobParseState, params: JsonRecord): void { + const result = asRecord(params.result); + const resultId = asString(result?.id); + if (resultId) state.retries.set(resultId, asNumber(result?.retry)); +} + +function handleAttachEvent(state: BlobParseState, params: JsonRecord): void { + const resultId = asString(params.resultId); + if (!resultId) return; + + const existing = state.attachments.get(resultId) ?? []; + const rawAttachments = Array.isArray(params.attachments) ? params.attachments : []; + for (const rawAttachment of rawAttachments) { + const attachment = parseTimingAttachment(rawAttachment); + if (attachment) existing.push(attachment); + } + state.attachments.set(resultId, existing); +} + +function parseTestEndEvent( + state: BlobParseState, + params: JsonRecord, +): TimingObservation | undefined { + const test = asRecord(params.test); + const result = asRecord(params.result); + if (!test || !result) return undefined; + + const descriptor = state.descriptors.get(asString(test.testId)); + if (!descriptor) return undefined; + + const resultId = asString(result.id); + return { + ...descriptor, + retry: state.retries.get(resultId) ?? 0, + status: parseTimingStatus(result.status), + durationSeconds: asNumber(result.duration) / 1000, + errors: parseTimingErrors(result.errors), + attachments: state.attachments.get(resultId) ?? [], + }; +} + +export function parseBlobReportJsonl(content: string): TimingObservation[] { + const state: BlobParseState = { + descriptors: new Map(), + retries: new Map(), + attachments: new Map(), + observations: [], + }; + + for (const event of parseJsonLines(content)) { + const method = asString(event.method); + const params = asRecord(event.params); + if (!params) continue; + + switch (method) { + case "onProject": + handleProjectEvent(state, params); + break; + case "onTestBegin": + handleTestBeginEvent(state, params); + break; + case "onAttach": + handleAttachEvent(state, params); + break; + case "onTestEnd": { + const observation = parseTestEndEvent(state, params); + if (observation) state.observations.push(observation); + break; + } + default: + break; + } + } + + return state.observations; +} + +export function collectEligibleSamples(observations: TimingObservation[]): TimingObservation[] { + return observations.filter( + (observation) => + observation.status === "passed" && + observation.retry === 0 && + Number.isFinite(observation.durationSeconds) && + observation.durationSeconds >= 0, + ); +} + +export function quantile(values: number[], probability: number): number { + if (values.length === 0) return 0; + const sorted = [...values].sort((a, b) => a - b); + const position = Math.min(1, Math.max(0, probability)) * (sorted.length - 1); + const lower = Math.floor(position); + const upper = Math.ceil(position); + if (lower === upper) return sorted[lower] ?? 0; + const fraction = position - lower; + return (sorted[lower] ?? 0) + ((sorted[upper] ?? 0) - (sorted[lower] ?? 0)) * fraction; +} + +export function buildTimingProfile( + previous: TimingProfile | undefined, + observations: TimingObservation[], + metadata: TimingProfileBuildMetadata, +): TimingProfile { + const entries = new Map(); + for (const entry of previous?.entries ?? []) { + if (metadata.knownKeys && !metadata.knownKeys.has(entry.key)) continue; + entries.set(entry.key, { + ...entry, + samples: entry.samples.slice(-MAX_SAMPLES_PER_TEST), + }); + } + + for (const observation of collectEligibleSamples(observations)) { + const previousEntry = entries.get(observation.key); + const samples = [ + ...(previousEntry?.samples ?? []), + { + durationSeconds: observation.durationSeconds, + sourceSha: metadata.sha, + runId: metadata.runId, + recordedAt: metadata.generatedAt, + }, + ].slice(-MAX_SAMPLES_PER_TEST); + const durations = samples.map((sample) => sample.durationSeconds); + const fileHash = metadata.fileHashes.get(observation.file) ?? previousEntry?.fileHash ?? ""; + entries.set(observation.key, { + key: observation.key, + project: observation.project, + file: observation.file, + title: observation.title, + fileHash, + samples, + p50Seconds: quantile(durations, 0.5), + p75Seconds: quantile(durations, 0.75), + lastSeenSha: metadata.sha, + lastSeenRunId: metadata.runId, + }); + } + + return { + version: TIMING_PROFILE_VERSION, + generatedAt: metadata.generatedAt, + source: { branch: metadata.branch, sha: metadata.sha, runId: metadata.runId }, + entries: [...entries.values()].sort((left, right) => left.key.localeCompare(right.key)), + }; +} + +export function readTimingProfile(filePath: string): TimingProfile | undefined { + if (!fs.existsSync(filePath)) return undefined; + const parsed = JSON.parse(fs.readFileSync(filePath, "utf8")) as Partial; + const entries = Array.isArray(parsed.entries) ? parsed.entries : []; + const validEntries = entries.every((rawEntry) => { + const entry = asRecord(rawEntry); + if ( + !entry || + !isFiniteDuration(entry.p50Seconds) || + !isFiniteDuration(entry.p75Seconds) || + !Array.isArray(entry.samples) + ) { + return false; + } + return entry.samples.every((rawSample) => { + const sample = asRecord(rawSample); + return Boolean(sample && isFiniteDuration(sample.durationSeconds)); + }); + }); + if ( + parsed.version !== TIMING_PROFILE_VERSION || + !Array.isArray(parsed.entries) || + !validEntries + ) { + throw new Error(`Unsupported timing profile version in ${filePath}`); + } + return parsed as TimingProfile; +} + +export function hashSourceFile(webRoot: string, relativeFile: string): string { + const normalized = normalizeReportPath(relativeFile); + const candidates = [path.resolve(webRoot, normalized), path.resolve(webRoot, "e2e", normalized)]; + const filePath = candidates.find((candidate) => { + try { + return fs.statSync(candidate).isFile(); + } catch { + return false; + } + }); + if (!filePath) return ""; + return createHash("sha256").update(fs.readFileSync(filePath)).digest("hex"); +} + +function listFiles(root: string): string[] { + if (!fs.existsSync(root)) return []; + const stats = fs.statSync(root); + if (stats.isFile()) return [root]; + return fs.readdirSync(root, { withFileTypes: true }).flatMap((entry) => { + const child = path.join(root, entry.name); + return entry.isDirectory() ? listFiles(child) : [child]; + }); +} + +export function findBlobReportFiles(inputPath: string): string[] { + return listFiles(inputPath) + .filter((filePath) => filePath.endsWith(".zip") || filePath.endsWith(".jsonl")) + .sort(); +} + +export function findShardMetadataFiles(inputPath: string): string[] { + return listFiles(inputPath) + .filter((filePath) => /shard-(normal|containers)-\d+-metadata\.json$/.test(filePath)) + .sort(); +} + +export function readShardMetadata(inputPath: string): ShardMetadata[] { + return findShardMetadataFiles(inputPath).flatMap((filePath) => { + try { + const parsed = JSON.parse(fs.readFileSync(filePath, "utf8")) as Partial; + if ( + typeof parsed.cohort !== "string" || + typeof parsed.shard !== "number" || + typeof parsed.shardCount !== "number" || + typeof parsed.startedAt !== "string" || + typeof parsed.finishedAt !== "string" || + typeof parsed.exitCode !== "number" || + typeof parsed.predictedSeconds !== "number" || + !Array.isArray(parsed.unitIds) + ) { + return []; + } + return [ + { + cohort: parsed.cohort, + shard: parsed.shard, + shardCount: parsed.shardCount, + startedAt: parsed.startedAt, + finishedAt: parsed.finishedAt, + exitCode: parsed.exitCode, + predictedSeconds: parsed.predictedSeconds, + unitIds: parsed.unitIds.filter((unitId): unitId is string => typeof unitId === "string"), + }, + ]; + } catch { + return []; + } + }); +} + +export function hasCompleteShardSet(metadata: ShardMetadata[]): boolean { + const expected = new Map([ + ["normal", 14], + ["containers", 6], + ]); + const seen = new Set(); + for (const shard of metadata) { + const shardCount = expected.get(shard.cohort); + if ( + shardCount === undefined || + shard.shardCount !== shardCount || + !Number.isInteger(shard.shard) || + shard.shard < 1 || + shard.shard > shardCount + ) { + return false; + } + const key = `${shard.cohort}:${shard.shard}`; + if (seen.has(key)) return false; + seen.add(key); + } + return seen.size === 20; +} + +export function readBlobReportJsonl(filePath: string): string { + if (filePath.endsWith(".jsonl")) return fs.readFileSync(filePath, "utf8"); + const entries = execFileSync("unzip", ["-Z1", filePath], { encoding: "utf8" }) + .split(/\r?\n/) + .filter(Boolean); + const reportFile = entries.find((entry) => entry.endsWith("report.jsonl")); + if (!reportFile) throw new Error(`Blob report ${filePath} does not contain report.jsonl`); + + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-blob-report-")); + const outputPath = path.join(tempDir, "report.jsonl"); + const outputFd = fs.openSync(outputPath, "w"); + try { + execFileSync("unzip", ["-p", filePath, reportFile], { + stdio: ["ignore", outputFd, "pipe"], + }); + return fs.readFileSync(outputPath, "utf8"); + } finally { + fs.closeSync(outputFd); + fs.rmSync(tempDir, { recursive: true, force: true }); + } +} + +export function parseBlobReports(inputPath: string): TimingObservation[] { + return findBlobReportFiles(inputPath).flatMap((filePath) => + parseBlobReportJsonl(readBlobReportJsonl(filePath)), + ); +} + +function parseArguments(argv: string[]): Record { + const args: Record = {}; + for (let index = 0; index < argv.length; index += 1) { + const argument = argv[index]; + if (!argument?.startsWith("--")) continue; + const name = argument.slice(2); + const value = argv[index + 1]; + if (value && !value.startsWith("--")) { + args[name] = value; + index += 1; + } + } + return args; +} + +function runCli(): void { + const args = parseArguments(process.argv.slice(2)); + const input = args.input; + const output = args.output; + if (!input || !output) { + throw new Error("Usage: e2e-timings.ts --input --output "); + } + + const observations = parseBlobReports(input); + const webRoot = args["web-root"] ?? process.cwd(); + const fileHashes = new Map( + [...new Set(observations.map((observation) => observation.file))].map((file) => [ + file, + hashSourceFile(webRoot, file), + ]), + ); + const previous = args.previous ? readTimingProfile(args.previous) : undefined; + const completeShardSet = hasCompleteShardSet(readShardMetadata(input)); + const profile = buildTimingProfile(previous, observations, { + branch: args.branch ?? process.env.GITHUB_REF_NAME ?? "unknown", + sha: args.sha ?? process.env.GITHUB_SHA ?? "unknown", + runId: args["run-id"] ?? process.env.GITHUB_RUN_ID ?? "unknown", + generatedAt: args.generated ?? new Date().toISOString(), + fileHashes, + knownKeys: completeShardSet + ? new Set(observations.map((observation) => observation.key)) + : undefined, + }); + fs.mkdirSync(path.dirname(path.resolve(output)), { recursive: true }); + fs.writeFileSync(output, `${JSON.stringify(profile, null, 2)}\n`); + console.log( + `timing profile: ${profile.entries.length} entries, ${collectEligibleSamples(observations).length} new samples`, + ); +} + +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + runCli(); +} diff --git a/apps/web/e2e/scripts/plan-shards.test.ts b/apps/web/e2e/scripts/plan-shards.test.ts new file mode 100644 index 0000000000..2c82da7ec4 --- /dev/null +++ b/apps/web/e2e/scripts/plan-shards.test.ts @@ -0,0 +1,231 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { + createShardPlan, + discoverTestCatalog, + validateShardManifest, + type CatalogUnit, +} from "./plan-shards"; +import type { TimingProfile } from "./e2e-timings"; + +const profile: TimingProfile = { + version: 1, + generatedAt: "2026-08-10T00:00:00.000Z", + source: { branch: "main", sha: "main-sha", runId: "12" }, + entries: [ + { + key: "chromium::tests/a.spec.ts::a", + project: "chromium", + file: "tests/a.spec.ts", + title: "a", + fileHash: "hash-a", + samples: [], + p50Seconds: 8, + p75Seconds: 10, + lastSeenSha: "main-sha", + lastSeenRunId: "12", + }, + { + key: "chromium::tests/b.spec.ts::b", + project: "chromium", + file: "tests/b.spec.ts", + title: "b", + fileHash: "hash-old-b", + samples: [], + p50Seconds: 20, + p75Seconds: 20, + lastSeenSha: "main-sha", + lastSeenRunId: "12", + }, + ], +}; + +const units: CatalogUnit[] = [ + { project: "chromium", file: "tests/a.spec.ts", fileHash: "hash-a", testCount: 1 }, + { project: "chromium", file: "tests/b.spec.ts", fileHash: "hash-new-b", testCount: 1 }, + { project: "chromium", file: "tests/c.spec.ts", fileHash: "hash-c", testCount: 3 }, + { project: "mobile-chrome", file: "tests/mobile-c.spec.ts", fileHash: "hash-m", testCount: 2 }, +]; + +describe("duration-aware shard planning", () => { + it("discovers specs below the web e2e test root and hashes their source", () => { + const webRoot = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-planner-")); + const testDir = path.join(webRoot, "e2e", "tests", "chat"); + fs.mkdirSync(testDir, { recursive: true }); + fs.symlinkSync(path.resolve("node_modules"), path.join(webRoot, "node_modules"), "dir"); + fs.writeFileSync( + path.join(webRoot, "e2e", "playwright.config.ts"), + [ + 'import { defineConfig } from "@playwright/test";', + "export default defineConfig({", + ' testDir: "./tests",', + " projects: [", + ' { name: "chromium", testIgnore: [/mobile-.*\\.spec\\.ts$/, /tests\\/(docker|ssh)\\//] },', + ' { name: "mobile-chrome", testMatch: /mobile-.*\\.spec\\.ts$/ },', + ' { name: "containers", testMatch: /tests\\/(docker|ssh)\\/.*\\.spec\\.ts$/ },', + " ],", + "});", + ].join("\n"), + ); + fs.writeFileSync( + path.join(testDir, "example.spec.ts"), + [ + 'import { test } from "@playwright/test";', + 'test.describe("suite", () => {', + " test.beforeEach(async () => {});", + ' test("works", async () => {});', + ' test.skip("skipped", async () => {});', + ' for (const name of ["generated one", "generated two"]) {', + " test(name, async () => {});", + " }", + "});", + ].join("\n"), + ); + + expect(discoverTestCatalog(webRoot, "normal")).toEqual([ + expect.objectContaining({ + project: "chromium", + file: "tests/chat/example.spec.ts", + testCount: 4, + fileHash: expect.stringMatching(/^[a-f0-9]{64}$/), + }), + ]); + }); + + it("assigns weighted units with stable longest-processing-time packing", () => { + const first = createShardPlan(units, { + cohort: "normal", + shardCount: 2, + profile, + generatedAt: "2026-08-10T00:00:00.000Z", + }); + const second = createShardPlan(units, { + cohort: "normal", + shardCount: 2, + profile, + generatedAt: "2026-08-10T00:00:00.000Z", + }); + + expect(first).toEqual(second); + expect(first.shards).toHaveLength(2); + expect(first.shards.flatMap((shard) => shard.units)).toHaveLength(units.length); + expect(first.summary.warmUnits).toBe(1); + expect(first.summary.unknownUnits).toBe(2); + expect(first.shards[0]?.predictedSeconds).toBeGreaterThan(0); + }); + + it("validates exact coverage and rejects duplicate assignments", () => { + const plan = createShardPlan(units, { + cohort: "containers", + shardCount: 2, + generatedAt: "2026-08-10T00:00:00.000Z", + }); + expect(() => validateShardManifest(plan, units)).not.toThrow(); + + const duplicate = structuredClone(plan); + duplicate.shards[1]?.units.push(plan.shards[0]!.units[0]!); + expect(() => validateShardManifest(duplicate, units)).toThrow(/duplicate/i); + }); + + it("rejects units whose immutable catalog fields were changed", () => { + const plan = createShardPlan(units, { + cohort: "normal", + shardCount: 2, + generatedAt: "2026-08-10T00:00:00.000Z", + }); + const tampered = structuredClone(plan); + const unit = tampered.shards.find((shard) => shard.units.length > 0)?.units[0]; + expect(unit).toBeDefined(); + unit!.fileHash = "tampered"; + + expect(() => validateShardManifest(tampered, units)).toThrow(/catalog fields/i); + }); + + it("rejects duplicate and out-of-range shard indexes", () => { + const plan = createShardPlan(units, { + cohort: "normal", + shardCount: 2, + generatedAt: "2026-08-10T00:00:00.000Z", + }); + const duplicate = structuredClone(plan); + duplicate.shards[1]!.index = duplicate.shards[0]!.index; + expect(() => validateShardManifest(duplicate, units)).toThrow(/shard index/i); + + const outOfRange = structuredClone(plan); + outOfRange.shards[0]!.index = plan.shardCount + 1; + expect(() => validateShardManifest(outOfRange, units)).toThrow(/shard index/i); + }); + + it("uses count-based estimates when no profile exists", () => { + const plan = createShardPlan(units.slice(0, 2), { + cohort: "containers", + shardCount: 1, + generatedAt: "2026-08-10T00:00:00.000Z", + }); + + expect(plan.profile.mode).toBe("count-fallback"); + expect(plan.summary.unknownUnits).toBe(2); + expect(plan.shards[0]?.predictedSeconds).toBe(40); + }); + + it("adds a fallback estimate for new tests in a profiled file", () => { + const plan = createShardPlan( + [{ project: "chromium", file: "tests/a.spec.ts", fileHash: "hash-a", testCount: 2 }], + { + cohort: "normal", + shardCount: 1, + profile, + generatedAt: "2026-08-10T00:00:00.000Z", + }, + ); + + expect(plan.summary.unknownUnits).toBe(1); + expect(plan.shards[0]?.units[0]?.classification).toBe("unknown"); + expect(plan.shards[0]?.predictedSeconds).toBe(20); + }); + + it("reports a file-level unit that dominates its shard", () => { + const plan = createShardPlan( + [ + { project: "chromium", file: "tests/heavy.spec.ts", fileHash: "heavy", testCount: 100 }, + { project: "chromium", file: "tests/light.spec.ts", fileHash: "light", testCount: 1 }, + ], + { + cohort: "normal", + shardCount: 2, + generatedAt: "2026-08-10T00:00:00.000Z", + }, + ); + + expect(plan.summary.dominantUnits).toEqual([ + expect.objectContaining({ + id: "chromium::tests/heavy.spec.ts", + shard: 1, + shareOfShard: 1, + }), + ]); + }); + + it("reports profiled files that are no longer in the catalog", () => { + const plan = createShardPlan(units.slice(0, 1), { + cohort: "normal", + shardCount: 1, + profile: { + ...profile, + entries: [ + ...profile.entries, + { + ...profile.entries[0]!, + key: "chromium::tests/removed.spec.ts::removed", + file: "tests/removed.spec.ts", + }, + ], + }, + generatedAt: "2026-08-10T00:00:00.000Z", + }); + + expect(plan.summary.staleUnits).toBe(2); + }); +}); diff --git a/apps/web/e2e/scripts/plan-shards.ts b/apps/web/e2e/scripts/plan-shards.ts new file mode 100644 index 0000000000..8a1f21b75e --- /dev/null +++ b/apps/web/e2e/scripts/plan-shards.ts @@ -0,0 +1,469 @@ +import { execFileSync } from "node:child_process"; +import { createHash } from "node:crypto"; +import fs from "node:fs"; +import { createRequire } from "node:module"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { + hashSourceFile, + normalizeReportPath, + readTimingProfile, + type TimingProfile, +} from "./e2e-timings"; + +export const SHARD_MANIFEST_VERSION = 1; +export const NORMAL_SHARD_COUNT = 14; +export const CONTAINER_SHARD_COUNT = 6; +export const DEFAULT_NORMAL_TEST_SECONDS = 10; +export const DEFAULT_CONTAINER_TEST_SECONDS = 20; +export const CHANGED_FILE_MULTIPLIER = 1.25; +export const DOMINANT_UNIT_SHARE = 0.5; + +export type Cohort = "normal" | "containers"; +export type EstimateClassification = "profile" | "warm" | "unknown"; + +export type CatalogUnit = { + project: string; + file: string; + fileHash: string; + testCount: number; +}; + +export type PlannedUnit = CatalogUnit & { + id: string; + estimatedSeconds: number; + classification: EstimateClassification; +}; + +export type Shard = { + index: number; + predictedSeconds: number; + files: string[]; + units: PlannedUnit[]; +}; + +export type DominantUnitDiagnostic = { + shard: number; + id: string; + estimatedSeconds: number; + shareOfShard: number; +}; + +export type ShardManifest = { + version: typeof SHARD_MANIFEST_VERSION; + cohort: Cohort; + shardCount: number; + generatedAt: string; + projects: string[]; + profile: { + mode: "main" | "count-fallback"; + sourceSha: string; + sourceRunId: string; + }; + catalog: { + checksum: string; + unitCount: number; + }; + summary: { + predictedSeconds: number; + unknownUnits: number; + warmUnits: number; + staleUnits: number; + targetSeconds: number; + dominantUnits: DominantUnitDiagnostic[]; + }; + shards: Shard[]; +}; + +type PlaywrightListTest = { projectName?: unknown }; +type PlaywrightListSpec = { file?: unknown; tests?: unknown }; +type PlaywrightListSuite = { file?: unknown; specs?: unknown; suites?: unknown }; +type PlaywrightListReport = { suites: PlaywrightListSuite[] }; + +const playwrightReportCache = new Map(); + +function asSuites(value: unknown): PlaywrightListSuite[] { + return Array.isArray(value) + ? value.filter((entry): entry is PlaywrightListSuite => + Boolean(entry && typeof entry === "object"), + ) + : []; +} + +function readPlaywrightListReport(webRoot: string): PlaywrightListReport { + const resolvedRoot = path.resolve(webRoot); + const cached = playwrightReportCache.get(resolvedRoot); + if (cached) return cached; + + const playwrightCli = createRequire(import.meta.url).resolve("@playwright/test/cli"); + const output = execFileSync( + process.execPath, + [ + playwrightCli, + "test", + "--config", + path.join(resolvedRoot, "e2e", "playwright.config.ts"), + "--project=chromium", + "--project=mobile-chrome", + "--project=containers", + "--list", + "--reporter=json", + ], + { cwd: resolvedRoot, encoding: "utf8", maxBuffer: 64 * 1024 * 1024 }, + ); + const parsed = JSON.parse(output) as { suites?: unknown }; + const report = { suites: asSuites(parsed.suites) }; + playwrightReportCache.set(resolvedRoot, report); + return report; +} + +function collectSpecCounts( + specs: unknown, + suiteFile: unknown, + allowedProjects: ReadonlySet, + counts: Map, +): void { + if (!Array.isArray(specs)) return; + for (const rawSpec of specs) { + if (!rawSpec || typeof rawSpec !== "object") continue; + const spec = rawSpec as PlaywrightListSpec; + const fileValue = typeof spec.file === "string" ? spec.file : suiteFile; + if (typeof fileValue !== "string") continue; + const file = normalizeReportPath(fileValue); + const tests = Array.isArray(spec.tests) ? spec.tests : []; + for (const rawTest of tests) { + if (!rawTest || typeof rawTest !== "object") continue; + const project = (rawTest as PlaywrightListTest).projectName; + if (typeof project !== "string" || !allowedProjects.has(project)) continue; + const id = `${project}::${file}`; + const current = counts.get(id); + counts.set(id, { project, file, testCount: (current?.testCount ?? 0) + 1 }); + } + } +} + +function collectPlaywrightCounts( + suites: PlaywrightListSuite[], + allowedProjects: ReadonlySet, + counts: Map, +): void { + for (const suite of suites) { + collectSpecCounts(suite.specs, suite.file, allowedProjects, counts); + collectPlaywrightCounts(asSuites(suite.suites), allowedProjects, counts); + } +} + +export function discoverTestCatalog(webRoot: string, cohort: Cohort): CatalogUnit[] { + const allowedProjects = new Set( + cohort === "containers" ? ["containers"] : ["chromium", "mobile-chrome"], + ); + const counts = new Map(); + collectPlaywrightCounts(readPlaywrightListReport(webRoot).suites, allowedProjects, counts); + return [...counts.values()] + .map((unit) => ({ ...unit, fileHash: hashSourceFile(webRoot, unit.file) })) + .sort((left, right) => + `${left.project}::${left.file}`.localeCompare(`${right.project}::${right.file}`), + ); +} + +function unitId(unit: Pick): string { + return `${unit.project}::${normalizeReportPath(unit.file)}`; +} + +function catalogChecksum(units: CatalogUnit[]): string { + const value = units + .map((unit) => `${unitId(unit)}:${unit.fileHash}:${unit.testCount}`) + .sort() + .join("\n"); + return createHash("sha256").update(value).digest("hex"); +} + +function fallbackSeconds(cohort: Cohort): number { + return cohort === "containers" ? DEFAULT_CONTAINER_TEST_SECONDS : DEFAULT_NORMAL_TEST_SECONDS; +} + +function countStaleProfileUnits(profile: TimingProfile | undefined, units: CatalogUnit[]): number { + if (!profile) return 0; + const currentUnits = new Set(units.map((unit) => unitId(unit))); + const projects = new Set(units.map((unit) => unit.project)); + const profileUnits = new Set( + profile.entries + .filter((entry) => projects.has(entry.project)) + .map((entry) => unitId({ project: entry.project, file: entry.file })), + ); + return [...profileUnits].filter((id) => !currentUnits.has(id)).length; +} + +function estimateClassification(missingTestCount: number, warm: boolean): EstimateClassification { + if (missingTestCount > 0) return "unknown"; + return warm ? "warm" : "profile"; +} + +function estimateUnit( + unit: CatalogUnit, + cohort: Cohort, + profile: TimingProfile | undefined, + changedFileMultiplier: number, +): PlannedUnit { + const id = unitId(unit); + const entries = profile?.entries.filter( + (entry) => entry.project === unit.project && normalizeReportPath(entry.file) === unit.file, + ); + if (!entries || entries.length === 0) { + return { + ...unit, + id, + estimatedSeconds: unit.testCount * fallbackSeconds(cohort), + classification: "unknown", + }; + } + + const missingTestCount = Math.max(0, unit.testCount - entries.length); + const warm = entries.some((entry) => entry.fileHash !== unit.fileHash); + const estimate = + entries.reduce((total, entry) => total + Math.max(0, entry.p75Seconds), 0) + + missingTestCount * fallbackSeconds(cohort); + return { + ...unit, + id, + estimatedSeconds: estimate * (warm ? changedFileMultiplier : 1), + classification: estimateClassification(missingTestCount, warm), + }; +} + +export function createShardPlan( + units: CatalogUnit[], + options: { + cohort: Cohort; + shardCount: number; + profile?: TimingProfile; + generatedAt: string; + changedFileMultiplier?: number; + }, +): ShardManifest { + if (!Number.isInteger(options.shardCount) || options.shardCount < 1) { + throw new Error(`shardCount must be a positive integer, got ${options.shardCount}`); + } + const plannedUnits = units.map((unit) => + estimateUnit( + unit, + options.cohort, + options.profile, + options.changedFileMultiplier ?? CHANGED_FILE_MULTIPLIER, + ), + ); + const shards: Shard[] = Array.from({ length: options.shardCount }, (_, index) => ({ + index: index + 1, + predictedSeconds: 0, + files: [], + units: [], + })); + + const sortedUnits = [...plannedUnits].sort( + (left, right) => + right.estimatedSeconds - left.estimatedSeconds || left.id.localeCompare(right.id), + ); + for (const unit of sortedUnits) { + const shard = shards.reduce((lightest, candidate) => { + if (candidate.predictedSeconds < lightest.predictedSeconds) return candidate; + return candidate.index < lightest.index ? candidate : lightest; + }, shards[0]!); + shard.units.push(unit); + shard.predictedSeconds += unit.estimatedSeconds; + if (!shard.files.includes(unit.file)) shard.files.push(unit.file); + } + for (const shard of shards) { + shard.units.sort((left, right) => left.id.localeCompare(right.id)); + shard.files.sort(); + } + + const predictedSeconds = shards.reduce((total, shard) => total + shard.predictedSeconds, 0); + const targetSeconds = options.shardCount > 0 ? predictedSeconds / options.shardCount : 0; + const staleUnits = countStaleProfileUnits(options.profile, units); + const dominantUnits = shards.flatMap((shard) => { + const dominant = shard.units.reduce( + (current, unit) => + !current || unit.estimatedSeconds > current.estimatedSeconds ? unit : current, + undefined, + ); + if (!dominant || shard.predictedSeconds <= 0) return []; + const shareOfShard = dominant.estimatedSeconds / shard.predictedSeconds; + const isMaterialOutlier = + dominant.estimatedSeconds > targetSeconds || + (shareOfShard >= DOMINANT_UNIT_SHARE && + dominant.estimatedSeconds >= targetSeconds * DOMINANT_UNIT_SHARE); + if (!isMaterialOutlier) { + return []; + } + return [ + { + shard: shard.index, + id: dominant.id, + estimatedSeconds: dominant.estimatedSeconds, + shareOfShard, + }, + ]; + }); + return { + version: SHARD_MANIFEST_VERSION, + cohort: options.cohort, + shardCount: options.shardCount, + generatedAt: options.generatedAt, + projects: [...new Set(units.map((unit) => unit.project))].sort(), + profile: { + mode: options.profile ? "main" : "count-fallback", + sourceSha: options.profile?.source.sha ?? "", + sourceRunId: options.profile?.source.runId ?? "", + }, + catalog: { checksum: catalogChecksum(units), unitCount: units.length }, + summary: { + predictedSeconds, + unknownUnits: plannedUnits.filter((unit) => unit.classification === "unknown").length, + warmUnits: plannedUnits.filter((unit) => unit.classification === "warm").length, + staleUnits, + targetSeconds, + dominantUnits, + }, + shards, + }; +} + +function validateShardIndexes(shards: Shard[], shardCount: number): void { + const seen = new Set(); + for (const shard of shards) { + const invalid = + !Number.isInteger(shard.index) || + shard.index < 1 || + shard.index > shardCount || + seen.has(shard.index); + if (invalid) throw new Error(`Invalid or duplicate shard index: ${shard.index}`); + seen.add(shard.index); + } +} + +function validatePlannedUnit( + unit: PlannedUnit, + expected: Map, + seen: Set, +): void { + const catalogUnit = expected.get(unit.id); + if (!catalogUnit) throw new Error(`Unit is outside the catalog: ${unit.id}`); + const matchesCatalog = + unit.project === catalogUnit.project && + unit.file === catalogUnit.file && + unit.fileHash === catalogUnit.fileHash && + unit.testCount === catalogUnit.testCount; + if (!matchesCatalog) throw new Error(`Unit catalog fields do not match: ${unit.id}`); + if (seen.has(unit.id)) throw new Error(`Duplicate unit assignment: ${unit.id}`); + seen.add(unit.id); +} + +function validateShardContents( + shard: Shard, + expected: Map, + seen: Set, +): void { + for (const unit of shard.units) validatePlannedUnit(unit, expected, seen); + const predicted = shard.units.reduce((total, unit) => total + unit.estimatedSeconds, 0); + if (Math.abs(predicted - shard.predictedSeconds) > 0.000001) { + throw new Error(`Shard ${shard.index} predicted duration does not match its units`); + } + const files = new Set(shard.units.map((unit) => unit.file)); + const filesMatch = + files.size === shard.files.length && [...files].every((file) => shard.files.includes(file)); + if (!filesMatch) throw new Error(`Shard ${shard.index} file selections do not match its units`); +} + +export function validateShardManifest(manifest: ShardManifest, catalog: CatalogUnit[]): void { + if (manifest.version !== SHARD_MANIFEST_VERSION) { + throw new Error(`Unsupported shard manifest version: ${manifest.version}`); + } + if (manifest.shardCount !== manifest.shards.length) { + throw new Error("Shard manifest count does not match its shard entries"); + } + validateShardIndexes(manifest.shards, manifest.shardCount); + const expected = new Map(catalog.map((unit) => [unitId(unit), unit])); + const seen = new Set(); + for (const shard of manifest.shards) validateShardContents(shard, expected, seen); + const missing = [...expected.keys()].filter((id) => !seen.has(id)); + if (missing.length > 0) throw new Error(`Missing catalog units: ${missing.join(", ")}`); + if (manifest.catalog.unitCount !== catalog.length) { + throw new Error("Shard manifest catalog count does not match the discovered catalog"); + } + if (manifest.catalog.checksum !== catalogChecksum(catalog)) { + throw new Error("Shard manifest catalog checksum does not match the discovered catalog"); + } +} + +function parseArguments(argv: string[]): Record { + const args: Record = {}; + for (let index = 0; index < argv.length; index += 1) { + const argument = argv[index]; + if (!argument?.startsWith("--")) continue; + const name = argument.slice(2); + const value = argv[index + 1]; + if (value && !value.startsWith("--")) { + args[name] = value; + index += 1; + } + } + return args; +} + +function writePlans(outputDir: string, plans: ShardManifest[]): void { + fs.mkdirSync(outputDir, { recursive: true }); + for (const plan of plans) { + const cohortDir = path.join(outputDir, plan.cohort); + fs.mkdirSync(cohortDir, { recursive: true }); + for (const shard of plan.shards) { + fs.writeFileSync( + path.join(cohortDir, `${shard.index}.json`), + `${JSON.stringify(plan, null, 2)}\n`, + ); + } + fs.writeFileSync( + path.join(outputDir, `${plan.cohort}-summary.json`), + `${JSON.stringify(plan, null, 2)}\n`, + ); + } +} + +function runCli(): void { + const args = parseArguments(process.argv.slice(2)); + const webRoot = path.resolve(args["web-root"] ?? process.cwd()); + const outputDir = path.resolve(args["output-dir"] ?? path.join(webRoot, "e2e", "manifests")); + const profile = args.profile ? readTimingProfile(path.resolve(args.profile)) : undefined; + const generatedAt = args.generated ?? new Date().toISOString(); + const normalCatalog = discoverTestCatalog(webRoot, "normal"); + const containerCatalog = discoverTestCatalog(webRoot, "containers"); + const normal = createShardPlan(normalCatalog, { + cohort: "normal", + shardCount: NORMAL_SHARD_COUNT, + profile, + generatedAt, + }); + const containers = createShardPlan(containerCatalog, { + cohort: "containers", + shardCount: CONTAINER_SHARD_COUNT, + profile, + generatedAt, + }); + validateShardManifest(normal, normalCatalog); + validateShardManifest(containers, containerCatalog); + writePlans(outputDir, [normal, containers]); + console.log( + JSON.stringify( + { + outputDir, + normal: normal.summary, + containers: containers.summary, + }, + null, + 2, + ), + ); +} + +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + runCli(); +} diff --git a/apps/web/e2e/scripts/retry-summary.test.ts b/apps/web/e2e/scripts/retry-summary.test.ts new file mode 100644 index 0000000000..15c6d5abaf --- /dev/null +++ b/apps/web/e2e/scripts/retry-summary.test.ts @@ -0,0 +1,140 @@ +import { describe, expect, it } from "vitest"; +import { summarizeObservations } from "./retry-summary"; +import type { ShardMetadata } from "./e2e-timings"; +import type { TimingObservation } from "./e2e-timings"; + +function observation(overrides: Partial): TimingObservation { + return { + key: "chromium::tests/example.spec.ts::works", + project: "chromium", + file: "tests/example.spec.ts", + title: "works", + retry: 0, + status: "passed", + durationSeconds: 1, + errors: [], + attachments: [], + ...overrides, + }; +} + +describe("retry summary", () => { + it("distinguishes first-attempt passes, passed retries, and failures", () => { + const summary = summarizeObservations( + [ + observation({}), + observation({ + key: "chromium::tests/flaky.spec.ts::flaky", + title: "flaky", + status: "failed", + }), + observation({ + key: "chromium::tests/flaky.spec.ts::flaky", + title: "flaky", + retry: 1, + status: "passed", + durationSeconds: 2, + errors: [{ message: "temporary failure" }], + }), + observation({ + key: "chromium::tests/slow.spec.ts::slow", + title: "slow", + status: "timedOut", + errors: [{ message: "Timeout 60000ms exceeded" }], + }), + observation({ + key: "chromium::tests/terminal.spec.ts::terminal", + title: "terminal", + status: "failed", + errors: [{ message: "terminal failure" }], + attachments: [{ name: "trace", path: "resources/trace.zip" }], + }), + ], + "2026-08-10T10:02:00.000Z", + { + attachmentArtifact: { + name: "e2e-timing-diagnostics", + url: "https://github.com/kdlbs/kandev/actions/runs/42#artifacts", + pathPrefix: "retry-artifacts", + availablePaths: new Set(["resources/trace.zip"]), + }, + }, + ); + + expect(summary.counts).toEqual({ + passedFirstAttempt: 1, + passedAfterRetry: 1, + failed: 1, + timedOut: 1, + skipped: 0, + }); + expect(summary.tests).toEqual( + expect.arrayContaining([ + expect.objectContaining({ key: "chromium::tests/flaky.spec.ts::flaky", attempts: 2 }), + expect.objectContaining({ + key: "chromium::tests/slow.spec.ts::slow", + finalStatus: "timedOut", + }), + expect.objectContaining({ + key: "chromium::tests/terminal.spec.ts::terminal", + finalStatus: "failed", + errorCategory: "failure", + attachments: [ + { + name: "trace", + path: "retry-artifacts/trace.zip", + artifactName: "e2e-timing-diagnostics", + artifactUrl: "https://github.com/kdlbs/kandev/actions/runs/42#artifacts", + }, + ], + }), + ]), + ); + }); + + it("compares planned and actual shard durations and keeps plan health counters", () => { + const metadata: ShardMetadata[] = [ + { + cohort: "normal", + shard: 1, + shardCount: 2, + startedAt: "2026-08-10T10:00:00.000Z", + finishedAt: "2026-08-10T10:02:00.000Z", + exitCode: 0, + predictedSeconds: 90, + unitIds: ["chromium::tests/example.spec.ts"], + }, + ]; + + const summary = summarizeObservations([], "2026-08-10T10:02:00.000Z", { + shardMetadata: metadata, + planSummaries: [ + { + profileMode: "count-fallback", + unknownUnits: 3, + warmUnits: 1, + staleUnits: 2, + targetSeconds: 120, + }, + ], + }); + + expect(summary.planning).toEqual({ + mode: "count-fallback", + unknownUnits: 3, + warmUnits: 1, + staleUnits: 2, + targetSeconds: 120, + }); + expect(summary.shards).toEqual([ + expect.objectContaining({ + cohort: "normal", + shard: 1, + predictedSeconds: 90, + actualSeconds: 120, + deltaSeconds: 30, + exitCode: 0, + }), + ]); + }); +}); diff --git a/apps/web/e2e/scripts/retry-summary.ts b/apps/web/e2e/scripts/retry-summary.ts new file mode 100644 index 0000000000..e78dd2a64c --- /dev/null +++ b/apps/web/e2e/scripts/retry-summary.ts @@ -0,0 +1,325 @@ +import { execFileSync } from "node:child_process"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { + parseBlobReports, + findBlobReportFiles, + readShardMetadata, + type ShardMetadata, + type TimingAttachment, + type TimingError, + type TimingObservation, + type TimingStatus, +} from "./e2e-timings"; + +export type RetryAttachment = TimingAttachment & { + artifactName?: string; + artifactUrl?: string; +}; + +export type RetryTestSummary = { + key: string; + project: string; + file: string; + title: string; + attempts: number; + statuses: TimingStatus[]; + finalStatus: TimingStatus; + finalDurationSeconds: number; + errorCategory: "none" | "failure" | "timeout" | "interrupted"; + errors: TimingError[]; + attachments: RetryAttachment[]; +}; + +export type RetrySummary = { + generatedAt: string; + counts: { + passedFirstAttempt: number; + passedAfterRetry: number; + failed: number; + timedOut: number; + skipped: number; + }; + planning: { + mode: "main" | "count-fallback" | "mixed" | "unknown"; + unknownUnits: number; + warmUnits: number; + staleUnits: number; + targetSeconds: number; + }; + shards: Array<{ + cohort: string; + shard: number; + shardCount: number; + predictedSeconds: number; + actualSeconds: number; + deltaSeconds: number; + exitCode: number; + startedAt: string; + finishedAt: string; + }>; + tests: RetryTestSummary[]; +}; + +export type PlanSummaryInput = { + profileMode: "main" | "count-fallback"; + unknownUnits: number; + warmUnits: number; + staleUnits: number; + targetSeconds: number; +}; + +export type RetrySummaryOptions = { + shardMetadata?: ShardMetadata[]; + planSummaries?: PlanSummaryInput[]; + attachmentArtifact?: { + name: string; + url: string; + pathPrefix: string; + availablePaths: ReadonlySet; + }; +}; + +function retryAttachment( + attachment: TimingAttachment, + artifact: RetrySummaryOptions["attachmentArtifact"], +): RetryAttachment { + const resourcePath = attachment.path?.replaceAll("\\", "/"); + if (!artifact || !resourcePath || !artifact.availablePaths.has(resourcePath)) { + return attachment; + } + return { + ...attachment, + path: `${artifact.pathPrefix}/${path.posix.basename(resourcePath)}`, + artifactName: artifact.name, + artifactUrl: artifact.url, + }; +} + +function planMode(planModes: PlanSummaryInput["profileMode"][]): RetrySummary["planning"]["mode"] { + if (planModes.length === 0) return "unknown"; + if (planModes.length === 1) return planModes[0]!; + return "mixed"; +} + +function errorCategory( + status: TimingStatus, + errors: TimingError[], +): RetryTestSummary["errorCategory"] { + if (status === "timedOut" || errors.some((error) => /timeout/i.test(error.message ?? ""))) { + return "timeout"; + } + if (status === "interrupted") return "interrupted"; + if (status === "failed") return "failure"; + return "none"; +} + +export function summarizeObservations( + observations: TimingObservation[], + generatedAt = new Date().toISOString(), + options: RetrySummaryOptions = {}, +): RetrySummary { + const grouped = new Map(); + for (const observation of observations) { + const existing = grouped.get(observation.key) ?? []; + existing.push(observation); + grouped.set(observation.key, existing); + } + + const tests = [...grouped.values()] + .map((attempts) => { + const ordered = [...attempts].sort((left, right) => left.retry - right.retry); + const finalAttempt = ordered.at(-1)!; + const errors = ordered.flatMap((attempt) => attempt.errors); + return { + key: finalAttempt.key, + project: finalAttempt.project, + file: finalAttempt.file, + title: finalAttempt.title, + attempts: ordered.length, + statuses: ordered.map((attempt) => attempt.status), + finalStatus: finalAttempt.status, + finalDurationSeconds: finalAttempt.durationSeconds, + errorCategory: errorCategory(finalAttempt.status, errors), + errors, + attachments: ordered.flatMap((attempt) => + attempt.attachments.map((attachment) => + retryAttachment(attachment, options.attachmentArtifact), + ), + ), + } satisfies RetryTestSummary; + }) + .sort((left, right) => left.key.localeCompare(right.key)); + + const counts = { + passedFirstAttempt: tests.filter((test) => test.finalStatus === "passed" && test.attempts === 1) + .length, + passedAfterRetry: tests.filter((test) => test.finalStatus === "passed" && test.attempts > 1) + .length, + failed: tests.filter((test) => test.finalStatus === "failed").length, + timedOut: tests.filter((test) => test.finalStatus === "timedOut").length, + skipped: tests.filter((test) => test.finalStatus === "skipped").length, + }; + + const planModes = [...new Set((options.planSummaries ?? []).map((plan) => plan.profileMode))]; + const planning = { + mode: planMode(planModes), + unknownUnits: (options.planSummaries ?? []).reduce( + (total, plan) => total + plan.unknownUnits, + 0, + ), + warmUnits: (options.planSummaries ?? []).reduce((total, plan) => total + plan.warmUnits, 0), + staleUnits: (options.planSummaries ?? []).reduce((total, plan) => total + plan.staleUnits, 0), + targetSeconds: (options.planSummaries ?? []).reduce( + (total, plan) => total + plan.targetSeconds, + 0, + ), + }; + + const shards = (options.shardMetadata ?? []).map((metadata) => { + const actualSeconds = Math.max( + 0, + (Date.parse(metadata.finishedAt) - Date.parse(metadata.startedAt)) / 1000, + ); + return { + cohort: metadata.cohort, + shard: metadata.shard, + shardCount: metadata.shardCount, + predictedSeconds: metadata.predictedSeconds, + actualSeconds: Number.isFinite(actualSeconds) ? actualSeconds : 0, + deltaSeconds: Number.isFinite(actualSeconds) + ? actualSeconds - metadata.predictedSeconds + : -metadata.predictedSeconds, + exitCode: metadata.exitCode, + startedAt: metadata.startedAt, + finishedAt: metadata.finishedAt, + }; + }); + + return { generatedAt, counts, planning, shards, tests }; +} + +function readPlanSummaries(manifestDir: string): PlanSummaryInput[] { + return ["normal-summary.json", "containers-summary.json"].flatMap((name) => { + const filePath = path.join(manifestDir, name); + if (!fs.existsSync(filePath)) return []; + try { + const parsed = JSON.parse(fs.readFileSync(filePath, "utf8")) as { + profile?: { mode?: PlanSummaryInput["profileMode"] }; + summary?: { + unknownUnits?: number; + warmUnits?: number; + staleUnits?: number; + targetSeconds?: number; + }; + }; + if (!parsed.profile?.mode || !parsed.summary) return []; + return [ + { + profileMode: parsed.profile.mode, + unknownUnits: parsed.summary.unknownUnits ?? 0, + warmUnits: parsed.summary.warmUnits ?? 0, + staleUnits: parsed.summary.staleUnits ?? 0, + targetSeconds: parsed.summary.targetSeconds ?? 0, + }, + ]; + } catch { + return []; + } + }); +} + +function parseArguments(argv: string[]): Record { + const args: Record = {}; + for (let index = 0; index < argv.length; index += 1) { + const argument = argv[index]; + if (!argument?.startsWith("--")) continue; + const name = argument.slice(2); + const value = argv[index + 1]; + if (value && !value.startsWith("--")) { + args[name] = value; + index += 1; + } + } + return args; +} + +function blobResourceLocations(inputPath: string): Map { + const locations = new Map(); + for (const filePath of findBlobReportFiles(inputPath).filter((file) => file.endsWith(".zip"))) { + const entries = execFileSync("unzip", ["-Z1", filePath], { encoding: "utf8" }) + .split(/\r?\n/) + .filter((entry) => /^resources\/[A-Za-z0-9._-]+$/.test(entry)); + for (const entry of entries) { + if (!locations.has(entry)) locations.set(entry, filePath); + } + } + return locations; +} + +export function materializeRetryArtifacts( + inputPath: string, + outputDir: string, + resourcePaths: Iterable, +): Set { + const locations = blobResourceLocations(inputPath); + const available = new Set(); + for (const resourcePath of new Set(resourcePaths)) { + const sourceZip = locations.get(resourcePath); + if (!sourceZip) continue; + fs.mkdirSync(outputDir, { recursive: true }); + const outputPath = path.join(outputDir, path.posix.basename(resourcePath)); + const outputFd = fs.openSync(outputPath, "w"); + try { + execFileSync("unzip", ["-p", sourceZip, resourcePath], { + stdio: ["ignore", outputFd, "pipe"], + }); + available.add(resourcePath); + } finally { + fs.closeSync(outputFd); + } + } + return available; +} + +function runCli(): void { + const args = parseArguments(process.argv.slice(2)); + if (!args.input || !args.output) { + throw new Error("Usage: retry-summary.ts --input --output "); + } + const observations = parseBlobReports(args.input); + const attachmentOutput = args["attachment-output"]; + const availablePaths = attachmentOutput + ? materializeRetryArtifacts( + args.input, + path.resolve(attachmentOutput), + observations.flatMap((observation) => + observation.attachments.flatMap((attachment) => attachment.path ?? []), + ), + ) + : new Set(); + const attachmentArtifact = + args["artifact-name"] && args["artifact-url"] && attachmentOutput + ? { + name: args["artifact-name"], + url: args["artifact-url"], + pathPrefix: path.basename(path.resolve(attachmentOutput)), + availablePaths, + } + : undefined; + const summary = summarizeObservations(observations, new Date().toISOString(), { + shardMetadata: readShardMetadata(args.input), + planSummaries: args["manifest-dir"] ? readPlanSummaries(args["manifest-dir"]!) : [], + attachmentArtifact, + }); + fs.mkdirSync(path.dirname(path.resolve(args.output)), { recursive: true }); + fs.writeFileSync(args.output, `${JSON.stringify(summary, null, 2)}\n`); + console.log( + `retry summary: ${summary.counts.passedAfterRetry} passed after retry, ${summary.counts.failed} failed`, + ); +} + +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + runCli(); +} diff --git a/apps/web/e2e/scripts/run-planned-shard.sh b/apps/web/e2e/scripts/run-planned-shard.sh new file mode 100755 index 0000000000..8eeb31637e --- /dev/null +++ b/apps/web/e2e/scripts/run-planned-shard.sh @@ -0,0 +1,11 @@ +#!/usr/bin/env bash +set -euo pipefail + +manifest_path="${1:-}" +if [[ -z "$manifest_path" ]]; then + echo "usage: run-planned-shard.sh " >&2 + exit 2 +fi +shift + +exec pnpm exec tsx e2e/scripts/run-planned-shard.ts --manifest "$manifest_path" "$@" diff --git a/apps/web/e2e/scripts/run-planned-shard.test.ts b/apps/web/e2e/scripts/run-planned-shard.test.ts new file mode 100644 index 0000000000..49714b488f --- /dev/null +++ b/apps/web/e2e/scripts/run-planned-shard.test.ts @@ -0,0 +1,134 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it, vi } from "vitest"; +import { + buildPlaywrightArgs, + executeShard, + readManifest, + resolveShard, + writeShardMetadata, + type ShardManifest, +} from "./run-planned-shard"; + +function manifest(overrides: Partial = {}): ShardManifest { + const shard = { + index: 1, + predictedSeconds: 0, + files: [], + units: [], + }; + return { + version: 1, + cohort: "normal", + shardCount: 1, + generatedAt: "2026-08-10T00:00:00.000Z", + projects: ["chromium"], + profile: { mode: "count-fallback", sourceSha: "", sourceRunId: "" }, + catalog: { checksum: "", unitCount: 0 }, + summary: { + predictedSeconds: 0, + unknownUnits: 0, + warmUnits: 0, + staleUnits: 0, + targetSeconds: 0, + dominantUnits: [], + }, + shards: [shard], + ...overrides, + }; +} + +describe("planned shard runner", () => { + it("rejects an invalid manifest before catalog validation", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-runner-")); + const filePath = path.join(dir, "invalid.json"); + fs.writeFileSync(filePath, JSON.stringify({ version: 999, cohort: "normal", shards: [] })); + + expect(() => readManifest(filePath)).toThrow(/Invalid shard manifest/); + }); + + it("selects the requested shard from E2E_SHARD", () => { + const selected = resolveShard( + manifest({ + shardCount: 2, + shards: [ + { index: 1, predictedSeconds: 1, files: ["tests/one.spec.ts"], units: [] }, + { index: 2, predictedSeconds: 2, files: ["tests/two.spec.ts"], units: [] }, + ], + }), + "2", + ); + + expect(selected.index).toBe(2); + }); + + it("writes empty-shard metadata and propagates the child exit code", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "kandev-runner-")); + const current = manifest({ + shards: [{ index: 1, predictedSeconds: 0, files: [], units: [] }], + }); + writeShardMetadata(current, current.shards[0]!, "2026-08-10T00:00:00.000Z", 0, dir); + expect( + JSON.parse(fs.readFileSync(path.join(dir, "shard-normal-1-metadata.json"), "utf8")), + ).toMatchObject({ + shard: 1, + exitCode: 0, + }); + + const populated = manifest({ + projects: ["chromium", "mobile-chrome"], + shards: [ + { + index: 1, + predictedSeconds: 4, + files: ["tests/example.spec.ts"], + units: [], + }, + ], + }); + const spawn = vi.fn(() => ({ status: 7 })) as never; + const exitCode = executeShard(populated, populated.shards[0]!, spawn, dir); + + expect(exitCode).toBe(7); + expect(spawn).toHaveBeenCalledWith( + "pnpm", + [ + "exec", + "playwright", + "test", + "--config", + "e2e/playwright.config.ts", + "--project=chromium", + "--project=mobile-chrome", + "tests/example.spec.ts", + ], + expect.objectContaining({ + env: expect.objectContaining({ E2E_PLANNED_SHARD: "1" }), + }), + ); + }); + + it("builds the runner arguments from the manifest projects and files", () => { + const current = manifest({ + projects: ["containers"], + shards: [ + { + index: 1, + predictedSeconds: 1, + files: ["tests/docker/example.spec.ts"], + units: [], + }, + ], + }); + + expect(buildPlaywrightArgs(current, current.shards[0]!)).toEqual([ + "playwright", + "test", + "--config", + "e2e/playwright.config.ts", + "--project=containers", + "tests/docker/example.spec.ts", + ]); + }); +}); diff --git a/apps/web/e2e/scripts/run-planned-shard.ts b/apps/web/e2e/scripts/run-planned-shard.ts new file mode 100644 index 0000000000..a41d54bd4c --- /dev/null +++ b/apps/web/e2e/scripts/run-planned-shard.ts @@ -0,0 +1,124 @@ +import { spawnSync } from "node:child_process"; +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { + CONTAINER_SHARD_COUNT, + discoverTestCatalog, + NORMAL_SHARD_COUNT, + validateShardManifest, + type Shard, + type ShardManifest, +} from "./plan-shards"; + +export function readManifest(filePath: string): ShardManifest { + if (!fs.existsSync(filePath)) throw new Error(`Shard manifest does not exist: ${filePath}`); + const manifest = JSON.parse(fs.readFileSync(filePath, "utf8")) as ShardManifest; + if ( + manifest.version !== 1 || + !Array.isArray(manifest.shards) || + (manifest.cohort !== "normal" && manifest.cohort !== "containers") + ) { + throw new Error(`Invalid shard manifest: ${filePath}`); + } + const expectedShardCount = + manifest.cohort === "containers" ? CONTAINER_SHARD_COUNT : NORMAL_SHARD_COUNT; + if (manifest.shardCount !== expectedShardCount) { + throw new Error( + `Invalid ${manifest.cohort} shard count: expected ${expectedShardCount}, got ${manifest.shardCount}`, + ); + } + validateShardManifest(manifest, discoverTestCatalog(process.cwd(), manifest.cohort)); + return manifest; +} + +export function resolveShard( + manifest: ShardManifest, + requestedValue = process.env.E2E_SHARD, +): Shard { + const requested = Number(requestedValue ?? path.basename(process.cwd())); + if (Number.isInteger(requested) && requested > 0) { + const shard = manifest.shards.find((candidate) => candidate.index === requested); + if (shard) return shard; + } + if (manifest.shards.length === 1) return manifest.shards[0]!; + throw new Error("E2E_SHARD must identify one shard in a multi-shard manifest"); +} + +export function writeShardMetadata( + manifest: ShardManifest, + shard: Shard, + startedAt: string, + exitCode: number, + outputDir = path.resolve("e2e/blob-report"), +) { + fs.mkdirSync(outputDir, { recursive: true }); + const metadata = { + cohort: manifest.cohort, + shard: shard.index, + shardCount: manifest.shardCount, + startedAt, + finishedAt: new Date().toISOString(), + exitCode, + predictedSeconds: shard.predictedSeconds, + unitIds: shard.units.map((unit) => unit.id), + }; + fs.writeFileSync( + path.join(outputDir, `shard-${manifest.cohort}-${shard.index}-metadata.json`), + `${JSON.stringify(metadata, null, 2)}\n`, + ); +} + +export function buildPlaywrightArgs( + manifest: ShardManifest, + shard: Shard, + extraArgs: string[] = [], +): string[] { + return [ + "playwright", + "test", + "--config", + "e2e/playwright.config.ts", + ...manifest.projects.map((project) => `--project=${project}`), + ...shard.files, + ...extraArgs, + ]; +} + +export function executeShard( + manifest: ShardManifest, + shard: Shard, + spawn = spawnSync, + outputDir = path.resolve("e2e/blob-report"), + extraArgs: string[] = [], +): number { + const startedAt = new Date().toISOString(); + const result = spawn("pnpm", ["exec", ...buildPlaywrightArgs(manifest, shard, extraArgs)], { + stdio: "inherit", + env: { ...process.env, E2E_PLANNED_SHARD: String(shard.index) }, + }); + const exitCode = result.status ?? 1; + writeShardMetadata(manifest, shard, startedAt, exitCode, outputDir); + return exitCode; +} + +function run(): void { + const manifestPath = process.argv[process.argv.indexOf("--manifest") + 1] ?? process.argv[2]; + if (!manifestPath) throw new Error("Usage: run-planned-shard.ts --manifest "); + const manifest = readManifest(path.resolve(manifestPath)); + const shard = resolveShard(manifest); + if (shard.files.length === 0) { + writeShardMetadata(manifest, shard, new Date().toISOString(), 0); + console.log(`planned shard ${shard.index}: no tests assigned`); + return; + } + + const separator = process.argv.indexOf("--"); + const extraArgs = separator >= 0 ? process.argv.slice(separator + 1) : []; + const exitCode = executeShard(manifest, shard, spawnSync, undefined, extraArgs); + process.exitCode = exitCode; +} + +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + run(); +} diff --git a/apps/web/e2e/tests/automations-pr-merged-trigger.spec.ts b/apps/web/e2e/tests/automations-pr-merged-trigger.spec.ts index 1da3931352..35584b1e67 100644 --- a/apps/web/e2e/tests/automations-pr-merged-trigger.spec.ts +++ b/apps/web/e2e/tests/automations-pr-merged-trigger.spec.ts @@ -528,7 +528,7 @@ test.describe("automations — Pull request merged trigger", () => { workflowStepId: seedData.startStepId, // e2e:delay gives the test time to delete the target before the archive call prompt: - 'e2e:delay(500)\ne2e:mcp:kandev:archive_task_kandev({"task_id":"{{data.task_id}}"})\ne2e:message("deleted-done")', + 'e2e:delay(1000)\ne2e:mcp:kandev:archive_task_kandev({"task_id":"{{data.task_id}}"})\ne2e:message("deleted-done")', agentProfileId: seedData.agentProfileId, }); await apiClient.seedTrigger({ @@ -538,16 +538,15 @@ test.describe("automations — Pull request merged trigger", () => { enabled: true, }); - // Start the fire concurrently with the delete so the target is gone - // before the agent's archive call fires. - const firePromise = apiClient.firePRMerged({ + // Wait until the event has created the automation run, then delete the + // source task while the delay keeps the agent before its archive call. + const { run_task_id } = await apiClient.firePRMerged({ taskId: target.id, automationId: automation.id, owner: "acme", repo: "api", }); await apiClient.deleteTask(target.id); - const { run_task_id } = await firePromise; // Run must still complete cleanly — MCP error on a deleted task must not crash the turn await testPage.goto(`/t/${run_task_id}`); diff --git a/apps/web/e2e/tests/docker/storage-maintenance.spec.ts b/apps/web/e2e/tests/docker/storage-maintenance.spec.ts index 7c0a4e784a..37978941f6 100644 --- a/apps/web/e2e/tests/docker/storage-maintenance.spec.ts +++ b/apps/web/e2e/tests/docker/storage-maintenance.spec.ts @@ -1,7 +1,7 @@ import { execFileSync } from "node:child_process"; import type { Page } from "@playwright/test"; import { test, expect } from "../../fixtures/docker-test-base"; -import { E2E_IMAGE_TAG, removeKandevContainers } from "../../fixtures/docker-probe"; +import { E2E_DOCKER_SCOPE, E2E_IMAGE_TAG } from "../../fixtures/docker-probe"; import { dockerInspectExists, dockerRemove } from "../../helpers/docker"; function createStoppedContainer(labels: string[]): string { @@ -52,32 +52,58 @@ async function refreshStorageOverview(page: Page): Promise { }); } +test.describe.serial("process-scoped container cleanup", () => { + let previousTestContainer = ""; + + test.afterAll(() => { + if (previousTestContainer && dockerInspectExists(previousTestContainer)) { + dockerRemove(previousTestContainer); + } + }); + + test("allows a test to leave a process-owned container", () => { + previousTestContainer = createStoppedContainer([ + "kandev.managed=true", + `kandev.e2e.run=${E2E_DOCKER_SCOPE}`, + "kandev.task_id=e2e-cleanup-boundary", + ]); + expect(dockerInspectExists(previousTestContainer)).toBe(true); + }); + + test("starts the next test without the previous process-owned container", () => { + expect(dockerInspectExists(previousTestContainer)).toBe(false); + }); +}); + test("removes only stopped Kandev-labeled containers and gates daemon-wide cleanup", async ({ testPage, apiClient, seedData, }) => { + // Resources carry a process-unique ownership label. The storage API scopes + // managed-container usage to this process, so the count excludes another + // shard's containers while still exercising the exact cleanup contract. + const scopeLabel = `kandev.e2e.run=${E2E_DOCKER_SCOPE}`; test.setTimeout(240_000); - - // This test asserts the *global* count of kandev.managed=true containers the - // daemon reports ("2 managed containers"). The containers project shares one - // Docker daemon across all specs in the worker and only sweeps managed - // containers at worker teardown, so a sibling Docker-executor spec (or one of - // its retries) can leave managed containers behind and inflate the count — - // the flake seen in CI was "5 managed containers" instead of 2. Sweep any - // stray managed containers first so this test counts only its own fixtures. - removeKandevContainers(); const activeTask = await apiClient.createTask(seedData.workspaceId, "Retain active container", { workflow_id: seedData.workflowId, workflow_step_id: seedData.startStepId, }); const managed = createStoppedContainer([ "kandev.managed=true", + scopeLabel, `kandev.task_id=e2e-storage-missing-${Date.now()}`, ]); - const active = createStoppedContainer(["kandev.managed=true", `kandev.task_id=${activeTask.id}`]); - const unrelated = createStoppedContainer(["e2e.storage=unrelated"]); + const active = createStoppedContainer([ + "kandev.managed=true", + scopeLabel, + `kandev.task_id=${activeTask.id}`, + ]); + const unrelated = createStoppedContainer([scopeLabel, "e2e.storage=unrelated"]); try { + expect(dockerInspectExists(managed)).toBe(true); + expect(dockerInspectExists(active)).toBe(true); + expect(dockerInspectExists(unrelated)).toBe(true); await openStorageSettings(testPage); // The first overview can race Docker client startup and cache an // unavailable result. Analyze after creating the fixtures so this test diff --git a/apps/web/e2e/tests/layout/workspace-picker-shortcut.spec.ts b/apps/web/e2e/tests/layout/workspace-picker-shortcut.spec.ts index 635e09f8b2..746b6c6a48 100644 --- a/apps/web/e2e/tests/layout/workspace-picker-shortcut.spec.ts +++ b/apps/web/e2e/tests/layout/workspace-picker-shortcut.spec.ts @@ -46,6 +46,11 @@ test.describe("Workspace picker shortcut (global)", () => { expect(workspaceIds).toContain(second.id); expect(workspaceIds.length).toBeGreaterThanOrEqual(2); + // The shortcut opens a controlled menu. Wait for Radix's focus handoff + // before sending navigation keys, otherwise the first ArrowDown can land + // on the document body on a busy CI runner. + await expect(testPage.getByRole("menu")).toBeFocused(); + // Focus lands on the menu itself, so the first ArrowDown moves to item 1 // and the second to item 2 — no mouse needed at any point. await testPage.keyboard.press("ArrowDown"); diff --git a/apps/web/e2e/tests/review/mobile-review-multi-pr.spec.ts b/apps/web/e2e/tests/review/mobile-review-multi-pr.spec.ts index fde4ceb8e8..7ce0d0f0c3 100644 --- a/apps/web/e2e/tests/review/mobile-review-multi-pr.spec.ts +++ b/apps/web/e2e/tests/review/mobile-review-multi-pr.spec.ts @@ -2,14 +2,14 @@ import { test, expect } from "../../fixtures/test-base"; import { REVIEW_OWNER, REVIEW_PRS, - REVIEW_REPO, REVIEW_SHARED_FILE, + reviewRepositoryName, seedMultiPRReviewTask, } from "../../helpers/multi-pr-review"; import { SessionPage } from "../../pages/session-page"; import type { Page } from "@playwright/test"; -async function openMobileReview(testPage: Page, session: SessionPage) { +async function openMobileReview(testPage: Page, session: SessionPage, repositoryName: string) { await testPage.getByRole("button", { name: "Changes" }).tap(); const changesPanel = testPage.getByTestId("mobile-changes-panel"); await expect(changesPanel).toBeVisible({ timeout: 15_000 }); @@ -18,7 +18,7 @@ async function openMobileReview(testPage: Page, session: SessionPage) { for (const pr of REVIEW_PRS) { await expect( prFiles.locator( - `[data-changes-file=${JSON.stringify(REVIEW_SHARED_FILE)}][data-pr-key="${REVIEW_OWNER}/${REVIEW_REPO}/${pr.number}"]`, + `[data-changes-file=${JSON.stringify(REVIEW_SHARED_FILE)}][data-pr-key="${REVIEW_OWNER}/${repositoryName}/${pr.number}"]`, ), ).toBeVisible(); } @@ -35,30 +35,34 @@ test.describe("Review dialog multi-PR selector on mobile", () => { seedData, }) => { const task = await seedMultiPRReviewTask(apiClient, seedData, "Mobile Multi-PR Review E2E"); + const repositoryName = reviewRepositoryName(seedData); await testPage.goto(`/t/${task.id}`); const session = new SessionPage(testPage); await session.waitForLoad(); await session.waitForChatIdle(); - await openMobileReview(testPage, session); + await openMobileReview(testPage, session, repositoryName); const [firstPR, secondPR] = REVIEW_PRS; const selector = session.reviewPRSelectorTrigger(); await expect(selector).toBeVisible(); await expect(selector).toHaveAttribute("data-pr-number", String(firstPR.number)); await expect(session.reviewFileHeader(REVIEW_SHARED_FILE)).toBeVisible(); - await expect(session.reviewDialog().getByTestId("changes-repo-group")).toHaveAttribute( - "data-repository-name", - /.+/, - { timeout: 30_000 }, - ); + // The initial diff group can use the generated task-workspace scope, so + // only require a non-empty identity before selecting the second PR. + const repositoryGroup = session.reviewDialog().getByTestId("changes-repo-group"); + await expect(repositoryGroup).toHaveAttribute("data-repository-name", /\S+/, { + timeout: 30_000, + }); + const repositoryScope = await repositoryGroup.getAttribute("data-repository-name"); + expect(repositoryScope).toBeTruthy(); await expect .poll(() => session.reviewDiffText(), { timeout: 30_000 }) .toContain(firstPR.marker); await selector.tap(); const menu = session.reviewPRSelectorMenu(); - const secondItem = session.reviewPRSelectorItem(REVIEW_OWNER, REVIEW_REPO, secondPR.number); + const secondItem = session.reviewPRSelectorItem(REVIEW_OWNER, repositoryName, secondPR.number); await expect(menu).toBeVisible(); await expect(secondItem).toBeVisible(); @@ -89,11 +93,9 @@ test.describe("Review dialog multi-PR selector on mobile", () => { await expect(session.reviewDialog()).toBeVisible(); await expect(selector).toHaveAttribute("data-pr-number", String(secondPR.number)); await expect(session.reviewFileHeader(REVIEW_SHARED_FILE)).toBeVisible({ timeout: 20_000 }); - await expect(session.reviewDialog().getByTestId("changes-repo-group")).toHaveAttribute( - "data-repository-name", - /.+/, - { timeout: 30_000 }, - ); + await expect(repositoryGroup).toHaveAttribute("data-repository-name", secondPR.repositoryName, { + timeout: 30_000, + }); await expect .poll(() => session.reviewDiffText(), { timeout: 30_000 }) .toContain(secondPR.marker); @@ -113,6 +115,7 @@ test.describe("Review dialog multi-PR selector on mobile", () => { }) => { await testPage.setViewportSize({ width: 820, height: 900 }); const task = await seedMultiPRReviewTask(apiClient, seedData, "Tablet Multi-PR Review E2E"); + const repositoryName = reviewRepositoryName(seedData); await testPage.goto(`/t/${task.id}`); const session = new SessionPage(testPage); @@ -135,7 +138,7 @@ test.describe("Review dialog multi-PR selector on mobile", () => { expect(toolbarBox.height).toBeGreaterThanOrEqual(44); await dialogSelector.tap(); - await session.reviewPRSelectorItem(REVIEW_OWNER, REVIEW_REPO, REVIEW_PRS[1].number).tap(); + await session.reviewPRSelectorItem(REVIEW_OWNER, repositoryName, REVIEW_PRS[1].number).tap(); await expect(dialogSelector).toHaveAttribute("data-pr-number", String(REVIEW_PRS[1].number)); await expect.poll(() => session.reviewDiffText()).toContain(REVIEW_PRS[1].marker); }); diff --git a/apps/web/e2e/tests/review/review-multi-pr.spec.ts b/apps/web/e2e/tests/review/review-multi-pr.spec.ts index d3702dac41..9834e0a493 100644 --- a/apps/web/e2e/tests/review/review-multi-pr.spec.ts +++ b/apps/web/e2e/tests/review/review-multi-pr.spec.ts @@ -2,20 +2,20 @@ import { test, expect } from "../../fixtures/test-base"; import { REVIEW_OWNER, REVIEW_PRS, - REVIEW_REPO, REVIEW_SHARED_FILE, + reviewRepositoryName, seedMultiPRReviewTask, } from "../../helpers/multi-pr-review"; import { SessionPage } from "../../pages/session-page"; -async function openDesktopReview(session: SessionPage) { +async function openDesktopReview(session: SessionPage, repositoryName: string) { await session.clickTab("Changes"); const prFiles = session.prFilesSection(); await expect(prFiles).toBeVisible({ timeout: 20_000 }); for (const pr of REVIEW_PRS) { await expect( prFiles.locator( - `[data-changes-file=${JSON.stringify(REVIEW_SHARED_FILE)}][data-pr-key="${REVIEW_OWNER}/${REVIEW_REPO}/${pr.number}"]`, + `[data-changes-file=${JSON.stringify(REVIEW_SHARED_FILE)}][data-pr-key="${REVIEW_OWNER}/${repositoryName}/${pr.number}"]`, ), ).toBeVisible(); } @@ -32,23 +32,27 @@ test.describe("Review dialog multi-PR selector", () => { seedData, }) => { const task = await seedMultiPRReviewTask(apiClient, seedData, "Multi-PR Review E2E"); + const repositoryName = reviewRepositoryName(seedData); await testPage.goto(`/t/${task.id}`); const session = new SessionPage(testPage); await session.waitForLoad(); await session.waitForChatIdle(); - await openDesktopReview(session); + await openDesktopReview(session, repositoryName); const [firstPR, secondPR] = REVIEW_PRS; const selector = session.reviewPRSelectorTrigger(); await expect(selector).toBeVisible(); await expect(selector).toHaveAttribute("data-pr-number", String(firstPR.number)); await expect(session.reviewFileHeader(REVIEW_SHARED_FILE)).toBeVisible(); - await expect(session.reviewDialog().getByTestId("changes-repo-group")).toHaveAttribute( - "data-repository-name", - /.+/, - { timeout: 30_000 }, - ); + // The initial diff group can use the generated task-workspace scope, so + // only require a non-empty identity before selecting the second PR. + const repositoryGroup = session.reviewDialog().getByTestId("changes-repo-group"); + await expect(repositoryGroup).toHaveAttribute("data-repository-name", /\S+/, { + timeout: 30_000, + }); + const repositoryScope = await repositoryGroup.getAttribute("data-repository-name"); + expect(repositoryScope).toBeTruthy(); await expect .poll(() => session.reviewDiffText(), { timeout: 30_000 }) .toContain(firstPR.marker); @@ -56,17 +60,15 @@ test.describe("Review dialog multi-PR selector", () => { await selector.click(); const menu = session.reviewPRSelectorMenu(); await expect(menu).toBeVisible(); - await session.reviewPRSelectorItem(REVIEW_OWNER, REVIEW_REPO, secondPR.number).click(); + await session.reviewPRSelectorItem(REVIEW_OWNER, repositoryName, secondPR.number).click(); await expect(menu).toBeHidden(); await expect(session.reviewDialog()).toBeVisible(); await expect(selector).toHaveAttribute("data-pr-number", String(secondPR.number)); await expect(session.reviewFileHeader(REVIEW_SHARED_FILE)).toBeVisible({ timeout: 20_000 }); - await expect(session.reviewDialog().getByTestId("changes-repo-group")).toHaveAttribute( - "data-repository-name", - /.+/, - { timeout: 30_000 }, - ); + await expect(repositoryGroup).toHaveAttribute("data-repository-name", secondPR.repositoryName, { + timeout: 30_000, + }); await expect .poll(() => session.reviewDiffText(), { timeout: 30_000 }) .toContain(secondPR.marker); diff --git a/apps/web/e2e/tests/settings/config-management.spec.ts b/apps/web/e2e/tests/settings/config-management.spec.ts index 193625f091..b24717ab46 100644 --- a/apps/web/e2e/tests/settings/config-management.spec.ts +++ b/apps/web/e2e/tests/settings/config-management.spec.ts @@ -1,4 +1,5 @@ import { test, expect } from "../../fixtures/test-base"; +import { waitForLatestSessionDone } from "../../helpers/session"; import { SessionPage } from "../../pages/session-page"; import type { ApiClient } from "../../helpers/api-client"; import type { SeedData } from "../../fixtures/test-base"; @@ -24,12 +25,23 @@ function startConfigSession(apiClient: ApiClient, seedData: SeedData, prompt: st /** Navigate to session page and wait for the final marker message. */ async function runAndWait( testPage: import("@playwright/test").Page, + apiClient: ApiClient, taskId: string, marker: string, ) { + await waitForLatestSessionDone( + apiClient, + taskId, + 1, + `config chat session should complete before reading ${marker}`, + 30_000, + ); await testPage.goto(`/t/${taskId}`); const page = new SessionPage(testPage); await page.waitForLoad(); + // The API completion is the readiness signal. The idle wait allows the + // config-chat WS subscription to replay the completed turn into the UI. + await page.waitForChatIdle({ timeout: 30_000 }); await expect(page.activeChat().getByText(marker, { exact: true })).toBeVisible({ timeout: 60_000, }); @@ -66,7 +78,7 @@ test.describe("Config-mode MCP — workflow management", () => { ].join("\n"), ); - const page = await runAndWait(testPage, session.task_id, "Done listing"); + const page = await runAndWait(testPage, apiClient, session.task_id, "Done listing"); await expandToolCallGroups(page); await expect(page.chat.getByText("Kandev: List Workspaces")).toBeVisible({ timeout: 10_000 }); await expect(page.chat.getByText("Kandev: List Workflows")).toBeVisible({ timeout: 10_000 }); @@ -86,7 +98,7 @@ test.describe("Config-mode MCP — workflow management", () => { ].join("\n"), ); - await runAndWait(testPage, session.task_id, "Steps listed"); + await runAndWait(testPage, apiClient, session.task_id, "Steps listed"); // Verify via API const { steps } = await apiClient.listWorkflowSteps(workflow.id); @@ -125,7 +137,7 @@ test.describe("Config-mode MCP — workflow management", () => { ].join("\n"), ); - await runAndWait(testPage, session.task_id, "Step created"); + await runAndWait(testPage, apiClient, session.task_id, "Step created"); // Verify all fields via API const { steps } = await apiClient.listWorkflowSteps(workflow.id); @@ -150,7 +162,7 @@ test.describe("Config-mode MCP — workflow management", () => { ].join("\n"), ); - await runAndWait(testPage, session.task_id, "Workflow created"); + await runAndWait(testPage, apiClient, session.task_id, "Workflow created"); // Verify workflow was created via API. // Filter by both name and description since the seed workflow shares the same name. @@ -174,7 +186,7 @@ test.describe("Config-mode MCP — workflow management", () => { ].join("\n"), ); - await runAndWait(testPage, session.task_id, "Workflow updated"); + await runAndWait(testPage, apiClient, session.task_id, "Workflow updated"); // Verify via API const { workflows } = await apiClient.listWorkflows(seedData.workspaceId); @@ -200,7 +212,7 @@ test.describe("Config-mode MCP — workflow management", () => { ].join("\n"), ); - await runAndWait(testPage, session.task_id, "Workflow deleted"); + await runAndWait(testPage, apiClient, session.task_id, "Workflow deleted"); // Verify workflow was deleted via API const { workflows } = await apiClient.listWorkflows(seedData.workspaceId); @@ -234,7 +246,7 @@ test.describe("Config-mode MCP — workflow management", () => { ].join("\n"), ); - await runAndWait(testPage, session.task_id, "Step updated"); + await runAndWait(testPage, apiClient, session.task_id, "Step updated"); // Verify via API const { steps } = await apiClient.listWorkflowSteps(workflow.id); @@ -269,7 +281,7 @@ test.describe("Config-mode MCP — agent management", () => { ].join("\n"), ); - const page = await runAndWait(testPage, session.task_id, "Agents listed"); + const page = await runAndWait(testPage, apiClient, session.task_id, "Agents listed"); await expandToolCallGroups(page); await expect(page.chat.getByText("Kandev: List Agents")).toBeVisible({ timeout: 10_000 }); await expect(page.chat.getByText("Kandev: List Agent Profiles")).toBeVisible({ @@ -297,7 +309,7 @@ test.describe("Config-mode MCP — agent management", () => { ].join("\n"), ); - await runAndWait(testPage, createSession.task_id, "Profile created"); + await runAndWait(testPage, apiClient, createSession.task_id, "Profile created"); // Verify profile was created via API const { agents: afterCreate } = await apiClient.listAgents(); @@ -320,7 +332,7 @@ test.describe("Config-mode MCP — agent management", () => { ].join("\n"), ); - await runAndWait(testPage, deleteSession.task_id, "Profile deleted"); + await runAndWait(testPage, apiClient, deleteSession.task_id, "Profile deleted"); // Verify profile was deleted via API const { agents: afterDelete } = await apiClient.listAgents(); @@ -343,7 +355,7 @@ test.describe("Config-mode MCP — agent management", () => { ].join("\n"), ); - await runAndWait(testPage, session.task_id, "Agent updated"); + await runAndWait(testPage, apiClient, session.task_id, "Agent updated"); // Verify via API const { agents: updated } = await apiClient.listAgents(); @@ -362,7 +374,7 @@ test.describe("Config-mode MCP — agent management", () => { ].join("\n"), ); - await runAndWait(testPage, session.task_id, "Profile updated"); + await runAndWait(testPage, apiClient, session.task_id, "Profile updated"); // Verify via API const { agents } = await apiClient.listAgents(); @@ -393,7 +405,7 @@ test.describe("Config-mode MCP — agent management", () => { ].join("\n"), ); - await runAndWait(testPage, session.task_id, "Profile settings updated"); + await runAndWait(testPage, apiClient, session.task_id, "Profile settings updated"); // Verify via API const profile = await apiClient.getAgentProfile(seedData.agentProfileId); @@ -421,7 +433,7 @@ test.describe("Config-mode MCP — MCP server configuration", () => { ].join("\n"), ); - const page = await runAndWait(testPage, session.task_id, "MCP config updated"); + const page = await runAndWait(testPage, apiClient, session.task_id, "MCP config updated"); await expandToolCallGroups(page); await expect(page.chat.getByText("Kandev: Get MCP Config")).toBeVisible({ timeout: 10_000 }); await expect(page.chat.getByText("Kandev: Update MCP Config")).toBeVisible({ timeout: 10_000 }); @@ -455,7 +467,7 @@ test.describe("Config-mode MCP — task management", () => { ].join("\n"), ); - const page = await runAndWait(testPage, session.task_id, "Tasks listed"); + const page = await runAndWait(testPage, apiClient, session.task_id, "Tasks listed"); await expandToolCallGroups(page); await expect(page.chat.getByText("Kandev: List Tasks").first()).toBeVisible({ timeout: 10_000, @@ -483,7 +495,7 @@ test.describe("Config-mode MCP — task management", () => { ].join("\n"), ); - await runAndWait(testPage, session.task_id, "Task moved"); + await runAndWait(testPage, apiClient, session.task_id, "Task moved"); // Verify the task is now in the target step const { tasks } = await apiClient.listTasks(seedData.workspaceId); @@ -507,7 +519,7 @@ test.describe("Config-mode MCP — task management", () => { ].join("\n"), ); - await runAndWait(testPage, session.task_id, "Task archived"); + await runAndWait(testPage, apiClient, session.task_id, "Task archived"); // Verify the task no longer appears in active task list const { tasks } = await apiClient.listTasks(seedData.workspaceId); @@ -530,7 +542,7 @@ test.describe("Config-mode MCP — task management", () => { ].join("\n"), ); - await runAndWait(testPage, session.task_id, "Task deleted"); + await runAndWait(testPage, apiClient, session.task_id, "Task deleted"); // Verify the task is gone const { tasks } = await apiClient.listTasks(seedData.workspaceId); @@ -554,7 +566,7 @@ test.describe("Config-mode MCP — executor management", () => { ].join("\n"), ); - const page = await runAndWait(testPage, session.task_id, "Executors listed"); + const page = await runAndWait(testPage, apiClient, session.task_id, "Executors listed"); await expandToolCallGroups(page); await expect(page.chat.getByText("Kandev: List Executors", { exact: true })).toBeVisible({ timeout: 10_000, @@ -581,7 +593,7 @@ test.describe("Config-mode MCP — executor management", () => { ].join("\n"), ); - await runAndWait(testPage, createSession.task_id, "Executor profile created"); + await runAndWait(testPage, apiClient, createSession.task_id, "Executor profile created"); // Verify via API ({ executors } = await apiClient.listExecutors()); @@ -600,7 +612,7 @@ test.describe("Config-mode MCP — executor management", () => { ].join("\n"), ); - await runAndWait(testPage, deleteSession.task_id, "Executor profile deleted"); + await runAndWait(testPage, apiClient, deleteSession.task_id, "Executor profile deleted"); // Verify deleted const { executors: afterDelete } = await apiClient.listExecutors(); @@ -634,7 +646,7 @@ test.describe("Config-mode MCP — multi-tool workflow", () => { ].join("\n"), ); - await runAndWait(testPage, session.task_id, "Multi-tool config complete"); + await runAndWait(testPage, apiClient, session.task_id, "Multi-tool config complete"); // Verify the step was actually created const { steps } = await apiClient.listWorkflowSteps(workflow.id); @@ -666,7 +678,7 @@ test.describe("Config-mode MCP — multi-tool workflow", () => { ].join("\n"), ); - await runAndWait(testPage, session.task_id, "Full setup complete"); + await runAndWait(testPage, apiClient, session.task_id, "Full setup complete"); // Verify workflow step const { steps } = await apiClient.listWorkflowSteps(workflow.id); diff --git a/apps/web/e2e/tests/task/mobile-autopilot-mode.spec.ts b/apps/web/e2e/tests/task/mobile-autopilot-mode.spec.ts index 10d825954e..64b90eb823 100644 --- a/apps/web/e2e/tests/task/mobile-autopilot-mode.spec.ts +++ b/apps/web/e2e/tests/task/mobile-autopilot-mode.spec.ts @@ -106,6 +106,17 @@ test.describe("Mobile task autopilot", () => { hasText: "Mobile Autopilot Child", }); await expect(childRow).toBeVisible({ timeout: 30_000 }); + // The API state can settle before the sidebar's WS-backed task row has + // hydrated. Gate on both rendered state markers instead of asserting the + // first render, which was the source of the intermittent missing-icon + // failure. + await expect + .poll( + async () => + `${await childRow.getByTestId("task-autopilot-icon").count()}:${await childRow.getByTestId("task-state-waiting-for-input").count()}`, + { timeout: 30_000, message: "autopilot waiting state should reach the task switcher" }, + ) + .toBe("1:1"); await expect(childRow.getByTestId("task-autopilot-icon")).toBeVisible(); await expect(childRow.getByTestId("task-state-waiting-for-input")).toBeVisible({ timeout: 15_000, diff --git a/apps/web/e2e/tests/task/task-create-prompt-autocomplete.spec.ts b/apps/web/e2e/tests/task/task-create-prompt-autocomplete.spec.ts index 91ab46f3fc..5ad62ce067 100644 --- a/apps/web/e2e/tests/task/task-create-prompt-autocomplete.spec.ts +++ b/apps/web/e2e/tests/task/task-create-prompt-autocomplete.spec.ts @@ -1,10 +1,19 @@ import { test, expect } from "../../fixtures/test-base"; import { KanbanPage } from "../../pages/kanban-page"; +import type { Page } from "@playwright/test"; const PROMPT_NAME = "e2e-bug-template"; const PROMPT_CONTENT = "Reproduce, isolate, fix with a regression test."; const MENU_TITLE = /Mention tasks, files, prompts/i; +async function clearTaskCreateDrafts(page: Page): Promise { + await page.evaluate(() => { + for (const key of Object.keys(window.sessionStorage)) { + if (key.startsWith("kandev.taskCreateDraft.")) window.sessionStorage.removeItem(key); + } + }); +} + test.describe("Task creation: custom prompt autocomplete", () => { test.afterEach(async ({ apiClient }) => { const { prompts } = await apiClient.listPrompts(); @@ -25,6 +34,7 @@ test.describe("Task creation: custom prompt autocomplete", () => { const kanban = new KanbanPage(testPage); await kanban.goto(); + await clearTaskCreateDrafts(testPage); await kanban.createTaskButton.first().click(); const dialog = testPage.getByTestId("create-task-dialog"); @@ -51,6 +61,7 @@ test.describe("Task creation: custom prompt autocomplete", () => { const kanban = new KanbanPage(testPage); await kanban.goto(); + await clearTaskCreateDrafts(testPage); await kanban.createTaskButton.first().click(); await expect(testPage.getByTestId("create-task-dialog")).toBeVisible(); @@ -70,10 +81,12 @@ test.describe("Task creation: custom prompt autocomplete", () => { const kanban = new KanbanPage(testPage); await kanban.goto(); + await clearTaskCreateDrafts(testPage); await kanban.createTaskButton.first().click(); const dialog = testPage.getByTestId("create-task-dialog"); await expect(dialog).toBeVisible(); + await expect(testPage.getByTestId("task-description-input")).toHaveValue(""); // Fill title so the form would be otherwise submittable. await testPage.getByTestId("task-title-input").fill("autocomplete-enter-test"); diff --git a/docs/decisions/2026-08-10-duration-aware-e2e-sharding.md b/docs/decisions/2026-08-10-duration-aware-e2e-sharding.md new file mode 100644 index 0000000000..0a4b47a550 --- /dev/null +++ b/docs/decisions/2026-08-10-duration-aware-e2e-sharding.md @@ -0,0 +1,88 @@ +# Duration-aware E2E sharding uses rolling `main` timings + +**Status:** accepted +**Date:** 2026-08-10 +**Area:** infra, workflow + +## Context + +The E2E workflow currently uses ordinal Playwright sharding. Test counts are +balanced, but test durations are not. The investigated fourteen-shard normal +run ranged from 3m52 to 17m57, and the six-shard container run ranged from +about 3m to 9m59. New tests and changed test behavior would make committed +shard assignments increasingly stale. + +The planner needs recent timings without creating a new service or requiring a +developer to edit CI whenever the suite changes. It also needs a safe answer +when the timing artifact is missing, a test is new, or a file changed after +its timing sample was recorded. + +## Decision + +Use a rolling timing profile from successful `main` E2E runs as the source of +truth for duration-aware planning. Store it as a bounded workflow artifact +containing project/file/title keys, recent first-attempt passing samples, file +hashes, p50/p75 values, and source metadata. On each successful `main` run, +merge new eligible samples into the previous profile and compact the history. +Pull request runs may publish timing diagnostics but cannot update the +baseline. + +Generate normal and container manifests per run. Manifests are ephemeral, +validated artifacts and contain explicit project/file selections plus predicted +costs. The initial planner uses deterministic longest-processing-time bin +packing at the project/file level, with stable tie-breaking. Playwright keeps +one worker per shard. A later test-level split is allowed only for measured +file outliers. + +The planner uses p75 for unchanged entries, applies a conservative multiplier +to entries whose current source-file hash differs, and falls back to a +project/cohort estimate and then a deterministic count-based plan for unknown +or unavailable data. Every fallback is reported. Invalid or incomplete +manifests fail the planning job rather than silently mixing duration-aware and +ordinal assignments. + +## Consequences + +- Adding tests requires no shard-file maintenance; discovery and the next + `main` profile update include them automatically. +- Duration changes converge through the bounded sample history instead of + depending on a manual timing refresh. +- The first run after a source change is intentionally conservative and + reports warm entries, which can temporarily overestimate a shard. +- A missing artifact cannot block CI because the count-based fallback remains + deterministic and visible. +- Timing data is retained in GitHub Actions artifacts, so its retention and + access permissions must be maintained with the workflow. +- File-level planning is simple and stable, but a single very large file can + still limit balance; test-level splitting remains a measured extension. + +## Implementation status + +Accepted and implemented in `.github/workflows/e2e-tests.yml`, the E2E timing, +planner, runner, and retry scripts, and the scoped E2E/backend fixtures. The +initial worker default and matrix layout remain unchanged pending the measured +rollout checks described in the plan. + +## Alternatives considered + +### Commit shard assignments + +Rejected because new tests and timing changes would require manual edits and +would make stale assignments look authoritative. + +### Use the latest pull request timings as the baseline + +Rejected because a pull request may contain partial coverage, intentional +stress tests, or transient failures. Successful `main` is the comparable, +stable population. + +### Keep Playwright's ordinal shard selection + +Rejected because it ignores the observed duration skew and cannot express the +profile/fallback information needed to keep assignments current. + +### Store timings in an external service + +Deferred. An artifact is sufficient for this workflow, avoids new credentials +and availability dependencies, and can be replaced later behind the profile +contract if the suite grows beyond artifact needs. diff --git a/docs/decisions/INDEX.md b/docs/decisions/INDEX.md index 5d26359d40..aefa9c5053 100644 --- a/docs/decisions/INDEX.md +++ b/docs/decisions/INDEX.md @@ -143,4 +143,5 @@ Read individual ADRs for full context. Create new ones via `/record decision` or | 2026-08-08-go-launcher-owns-all-launch-modes | [The Go Launcher Owns Every Entrypoint; apps/cli Is a Publish-Only Shim](2026-08-08-go-launcher-owns-all-launch-modes.md) | accepted | backend, cli, infra | 2026-08-08 | | 2026-08-09-bind-automation-mutations-to-event-targets | [Bind Automation Mutations to Event Targets](2026-08-09-bind-automation-mutations-to-event-targets.md) | accepted | backend, agentctl, protocol, security, workflow | 2026-08-09 | | 2026-08-09-exclusive-runtime-state-ownership | [Lock Runtime State Before Backend Startup](2026-08-09-exclusive-runtime-state-ownership.md) | accepted | backend, infra | 2026-08-09 | +| 2026-08-10-duration-aware-e2e-sharding | [Duration-aware E2E sharding uses rolling `main` timings](2026-08-10-duration-aware-e2e-sharding.md) | accepted | infra, workflow | 2026-08-10 | | 2026-08-10-debug-launcher-profile-selection | [Keep Debug Launches on the Production Profile](2026-08-10-debug-launcher-profile-selection.md) | accepted | backend, frontend, cli | 2026-08-10 | diff --git a/docs/plans/e2e-duration-aware-sharding/plan.md b/docs/plans/e2e-duration-aware-sharding/plan.md new file mode 100644 index 0000000000..276e7d63eb --- /dev/null +++ b/docs/plans/e2e-duration-aware-sharding/plan.md @@ -0,0 +1,281 @@ +--- +spec: ../../specs/e2e-duration-aware-sharding/spec.md +created: 2026-08-10 +status: in_progress +--- + +# Implementation Plan: Duration-aware E2E sharding and CI reliability + +## Overview + +Replace ordinal E2E sharding with generated, duration-aware manifests while +keeping the current Playwright project boundaries and one worker per shard. +Build the timing profile from successful `main` runs, keep the generated +manifests ephemeral, and retain a deterministic count-based fallback. In the +same rollout, repair the seven retry groups identified during PR #2471, +publish passed-after-retry evidence, isolate container state, and run measured +experiments for worker concurrency, mobile project layout, and CI setup cost. + +The baseline to compare against is the investigated run: normal shard 2/14 +completed in 17m57 while shard 11/14 completed in 3m52; container shard 1/6 +completed in 9m59 while the lightest shards took about 3m. Build time was +6m18, including roughly 4m48 for the backend and E2E web build. The plan must +report whether improvements come from balancing test work or from reducing +fixed setup overhead. + +No production application behavior is changed. This is a CI/tooling and E2E +fixture package. + +## Timing profile and catalog + +Create a small, unit-tested timing module under `apps/web/e2e/scripts/`. + +- Parse the merged Playwright blob results and normalize each result to the + stable project/file/full-title key. +- Accept only first-attempt passing results as baseline samples. Exclude + failures, timeouts, and retry attempts. +- Merge the current eligible samples into the previous `main` profile and + retain a bounded recent history per key. Compute p50 and p75 values and + record the source commit, source file hash, run ID, and timestamps. +- Discover the current project/file catalog for the normal and container + cohorts. Keep the test matching rules in + `apps/web/e2e/playwright.config.ts` authoritative rather than duplicating + project selection in CI. +- Classify profile entries as unchanged, warm (source hash changed), unknown, + or stale. Apply the changed-file multiplier and project/cohort fallback + values from the spec, and expose the classification in planner output. + +Likely files: + +- `apps/web/e2e/scripts/e2e-timings.ts` +- `apps/web/e2e/scripts/e2e-timings.test.ts` +- `apps/web/e2e/scripts/plan-shards.ts` (shared catalog types may live here) +- `apps/web/e2e/scripts/plan-shards.test.ts` + +## Weighted shard planner + +Implement deterministic longest-processing-time bin packing for each cohort. +The planner should produce a versioned JSON manifest with: + +- cohort and selected projects; +- exact shard count; +- profile source and fallback mode; +- per-shard explicit project/file selections; +- predicted seconds, unknown count, warm count, and unit count; +- the full catalog checksum and validation result. + +Use project/file pairs as the first indivisible unit. If a file is selected by +more than one project, account for each project result in its estimate while +preserving a single unambiguous invocation. Sort by descending estimated cost +and use stable path/project ordering for ties. Validate exact coverage and no +overlap before the manifest can be uploaded. + +Keep a seam for a future `file:line` unit. Add a planner diagnostic that +identifies files which dominate their assigned shard so test-level splitting +can be enabled only when measurements justify its complexity. + +## CI lifecycle and runner integration + +Update `.github/workflows/e2e-tests.yml` so the build/planning job: + +1. resolves the latest successful `main` timing-profile artifact with + `actions: read` permission; +2. falls back to an empty/count-based profile when that artifact is absent; +3. runs catalog discovery and creates `normal/1.json` through + `normal/14.json` and `containers/1.json` through `containers/6.json` + manifests; +4. validates and uploads those manifests as a run-scoped artifact. + +Update both matrix jobs to download their cohort manifest and invoke a shared +runner. The runner must preserve the existing project filters, retries, +timeouts, blob reporter, output directories, `KANDEV_E2E_WS_ASSERT`, and +`workers: 1`. It must fail clearly for a missing shard manifest and must not +silently add ordinal `--shard` selection. + +Update the report job to retain the existing merged report and upload: + +- the merged timing profile candidate; +- retry/flakiness summary; +- predicted-versus-actual shard timing summary; +- plan validation and fallback counters. + +Only successful `main` report artifacts may be consumed as the next baseline. +Document the artifact names, retention expectations, fallback behavior, and +local dry-run commands in `apps/web/e2e/README.md` and keep workflow comments +aligned with the new ownership model. + +## E2E reliability repairs + +Implement the observed repairs as focused regression changes: + +- exact and scoped disabled-integration navigation locator; +- multi-PR review helper and desktop/mobile expectations based on the seeded + repository name; +- task-create prompt autocomplete setup that clears or explicitly verifies + restored session-storage drafts; +- config-management synchronization tied to the actual update completion + state rather than a brittle marker-only wait; +- mobile autopilot assertion that waits for the sidebar state to converge after + the API/WS transition; +- Docker storage-maintenance setup and assertions scoped to resources owned by + the test, with unique labels and no global managed-container deletion. + +Preserve the existing worker-scoped backend isolation. If an application +helper must change to expose a stable readiness signal, add the smallest +helper contract and cover its failure path. + +## Retry observability and policy + +Add a parser/summary path beside the timing collector that groups Playwright +results by stable test key and records attempts, final status, error category, +and links to trace/screenshot/video artifacts when present. Include counts for +passed-first-attempt, passed-after-retry, failed, timeout, and skipped. + +Keep the current PR retry behavior during initial rollout, but make retry +groups visible in the job summary and artifact. Add a scheduled or explicitly +diagnostic workflow mode with `failOnFlakyTests: true`; it should demonstrate +that the suite can enforce a no-flake policy without changing the normal PR +gate as part of this package. + +## Controlled performance experiments + +After the manifest path is stable, add reproducible experiment commands and a +record format rather than changing defaults immediately. + +- Run `workers: 2` on the known heavy normal shards and compare wall time, + CPU/memory pressure, backend isolation, and retries against `workers: 1`. +- Compare the unified normal matrix with a separate mobile matrix using the + same profile. Include browser/backend setup cost before judging the split. +- Measure the fixed costs observed in the run: build, pnpm install, runtime + image startup, and `/ms-playwright` extraction. Benchmark caching or + pre-baked images only after the test-work balance is visible. + +The default remains `workers: 1`, the current project matrix, and the existing +setup path until an experiment records a repeatable improvement without a +flake or resource regression. + +## Tests + +### Unit tests + +- timing result parsing, stable identity, retry exclusion, quantiles, bounded + merge history, file-hash warm classification, and fallback selection; +- planner bin packing, stable ties, empty/unknown catalogs, project/file + aggregation, changed-file multiplier, exact coverage, overlap rejection, + and manifest schema validation; +- retry grouping and artifact-link extraction; +- container ownership label construction and scoped cleanup predicates where + those helpers are extracted from the E2E fixtures. + +Run the focused tooling tests with: + +```sh +cd apps +pnpm --filter @kandev/web test -- e2e/scripts/e2e-timings.test.ts e2e/scripts/plan-shards.test.ts +``` + +### E2E and CI checks + +Run each repaired group directly against the existing config and retain the +normal retry count while debugging. The container test requires a Docker +daemon and the container profile: + +```sh +cd apps/web +pnpm exec playwright test --config e2e/playwright.config.ts --project=chromium e2e/tests/integrations/hide-disabled-integrations-nav.spec.ts e2e/tests/review/review-multi-pr.spec.ts e2e/tests/task/task-create-prompt-autocomplete.spec.ts +pnpm exec playwright test --config e2e/playwright.config.ts --project=chromium e2e/tests/settings/config-management.spec.ts +pnpm exec playwright test --config e2e/playwright.config.ts --project=mobile-chrome e2e/tests/review/mobile-review-multi-pr.spec.ts e2e/tests/task/mobile-autopilot-mode.spec.ts +KANDEV_E2E_CONTAINERS=1 pnpm exec playwright test --config e2e/playwright.config.ts --project=containers e2e/tests/docker/storage-maintenance.spec.ts +``` + +The workflow validation must prove that both cohorts execute every catalog +unit exactly once from generated manifests and that report artifacts include +the profile and retry summary. The three-run success criterion in the spec is +an operational rollout check, not a local unit-test substitute. + +## Implementation Waves And Parallel Candidates + +Wave 1 — independent evidence and fixture repairs: + +- [x] [Task 01: Build the rolling timing profile](task-01-timing-profile.md) +- [x] [Task 04: Repair deterministic E2E fixtures](task-04-deterministic-e2e-fixtures.md) +- [x] [Task 05: Repair asynchronous E2E synchronization](task-05-async-e2e-synchronization.md) +- [x] [Task 06: Isolate container E2E state](task-06-container-isolation.md) + +These tasks touch separate primary areas and are parallel candidates, but the +primary implementation conversation should execute them sequentially unless +the user explicitly delegates work. + +Wave 2 — planner: + +- [x] [Task 02: Generate duration-aware shard manifests](task-02-shard-planner.md) + +Depends on Task 01. + +Wave 3 — workflow integration: + +- [x] [Task 03: Wire manifests into the CI workflow](task-03-ci-manifest-lifecycle.md) + +Depends on Tasks 01 and 02. + +Wave 4 — reliability signal and measurement: + +- [x] [Task 07: Surface retry and timing evidence](task-07-retry-observability.md) +- [x] [Task 08: Run concurrency and setup experiments](task-08-performance-experiments.md) + +Both depend on Task 03. Task 08 also consumes the repaired container behavior +from Task 06. + +Wave 5 — rollout documentation: + +- [x] [Task 09: Document rollout and operating checks](task-09-rollout-documentation.md) + +Depends on Tasks 03, 06, and 07 so the README describes the final contracts. + +## Verification Results + +Implemented and verified in the working tree: + +- Timing, planner, retry-summary, runner, and helper tests: 5 files, 26 tests + passed. +- Backend Docker-scope tests: `go test ./internal/agent/runtime/lifecycle +./internal/backendapp` passed, 1,456 tests across 2 packages. +- Web typecheck and focused ESLint passed; Prettier check and `git diff --check` + passed. +- `make build-backend build-web-e2e` and + `make -C apps/backend e2e-plugin-package` passed. +- Planner dry-run generated and validated 14 normal plus 6 container manifests; + the live catalog contained 599 unknown normal units and 27 unknown container + units. The heaviest container file was reported as a 240-second dominant + indivisible unit (63.2% of its shard). The missing-manifest runner check + failed closed as designed. +- Reliability groups passed: normal Chromium 5/5, config-management 21/21, + mobile 3/3, containers 1/1. Two concurrent container runs also passed 1/1 + each, and no managed containers remained afterward. +- Task 08 completed its controlled local experiment on the rebuilt merged + checkout. The heavy normal shard passed 191/191 three times with workers=1 + (median 458.237s, CV 0.014%). A workers=2 run was 268.715s but produced two + flaky tests and exited 1 under `E2E_FAIL_ON_FLAKY=1`, so workers=2 is rejected. +- The unified-vs-mobile split simulation predicted 611.1s for the current + unified shape versus 653.7s for the best separate allocation before setup + overhead. Normal 14/container 6 remains the measured shape. CI setup was + separately recorded: 13s install, 4m44s backend/web build, 33s browser + extraction; cold-cache measurements remain a follow-up. +- The latest PR run [31469837346](https://github.com/kdlbs/kandev/actions/runs/31469837346) + passed 42 checks at merged head `d150554a5` and reduced full workflow wall + time from 28m30s on main run [31423650879](https://github.com/kdlbs/kandev/actions/runs/31423650879) + to 22m39s (20.5%). Both runs used count fallback because the latest main + run has no timing-profile artifact; the first post-merge main run remains + the profile bootstrap check. + +The three-successful-main-run balance criterion and cold-cache setup comparison +remain operational rollout checks after merge, not reasons to change the +shipping defaults now. + +## Open Questions + +No blocking questions remain for implementation. The initial defaults are a +bounded recent history, p75 planning, a conservative changed-file multiplier, +file-level units, and `workers: 1`. Task 01 may tune the history window and +Task 02 may tune the fallback estimate using real profile distributions, but +those changes must remain explicit in the profile metadata and tests. diff --git a/docs/plans/e2e-duration-aware-sharding/task-01-timing-profile.md b/docs/plans/e2e-duration-aware-sharding/task-01-timing-profile.md new file mode 100644 index 0000000000..870ac93b57 --- /dev/null +++ b/docs/plans/e2e-duration-aware-sharding/task-01-timing-profile.md @@ -0,0 +1,70 @@ +--- +id: "01-timing-profile" +title: "Build the rolling timing profile" +status: completed +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/e2e-duration-aware-sharding/spec.md" +--- + +# Task 01: Build the rolling timing profile + +## Acceptance + +- Merged Playwright blob results produce stable project/file/full-title keys + and exclude failures, timeouts, and retry attempts from baseline samples. +- A successful `main` profile merges a bounded recent sample history and + exposes p50/p75, source file hash, source run, and source commit metadata. +- New, changed, stale, and unavailable-profile cases select explicit fallback + estimates and are reported to the planner instead of being silently treated + as current timings. + +## Verification + +```sh +cd apps +pnpm --filter @kandev/web test -- e2e/scripts/e2e-timings.test.ts +pnpm --filter @kandev/web typecheck +``` + +The unit suite must cover duplicate results, retry-only passes, missing timing +data, changed source hashes, bounded history compaction, and quantile +calculation. + +## Files likely touched + +- `apps/web/e2e/scripts/e2e-timings.ts` +- `apps/web/e2e/scripts/e2e-timings.test.ts` +- `apps/web/e2e/scripts/plan-shards.ts` (shared profile/catalog types only if + needed) + +## Dependencies + +None. Use the Playwright blob result shape already emitted by CI and standard +Node/TypeScript dependencies already present in `apps/web`. + +## Parallelism + +Independent of the fixture repair tasks. The planner task depends on the +profile schema and classification contract from this task. + +## Inputs + +- The timing profile and fallback contracts in the linked spec and ADR. +- Current Playwright blob reports from `.github/workflows/e2e-tests.yml`. +- Existing `apps/web/e2e/playwright.config.ts` project matching rules. + +## Output contract + +Report the profile schema, accepted-sample rule, history bound, p50/p75 +implementation, fallback classifications, files changed, and exact targeted +test result. Keep the output format versioned so a malformed or old artifact is +rejected by the planner. + +## Implementation result + +Implemented `e2e-timings.ts` with versioned profiles, bounded 20-sample +histories, p50/p75 quantiles, stable project/file/title keys, source hashing, +and malformed-blob tolerance. The focused timing/planner/retry suite passed +26 tests across 5 files; web typecheck also passed. diff --git a/docs/plans/e2e-duration-aware-sharding/task-02-shard-planner.md b/docs/plans/e2e-duration-aware-sharding/task-02-shard-planner.md new file mode 100644 index 0000000000..b913220db3 --- /dev/null +++ b/docs/plans/e2e-duration-aware-sharding/task-02-shard-planner.md @@ -0,0 +1,76 @@ +--- +id: "02-shard-planner" +title: "Generate duration-aware shard manifests" +status: completed +wave: 2 +depends_on: + - "01-timing-profile" +plan: "plan.md" +spec: "../../specs/e2e-duration-aware-sharding/spec.md" +--- + +# Task 02: Generate duration-aware shard manifests + +## Acceptance + +- The planner creates deterministic fourteen-shard normal and six-shard + container manifests using weighted project/file units and stable + longest-processing-time bin packing. +- Manifest validation rejects missing, duplicated, out-of-cohort, or + unaccounted catalog units and records predicted cost, unknown count, warm + count, profile source, and fallback mode. +- The planner identifies indivisible files that dominate a shard and leaves a + tested seam for later `file:line` splitting without enabling it by default. + +## Verification + +```sh +cd apps +pnpm --filter @kandev/web test -- e2e/scripts/plan-shards.test.ts +pnpm --filter @kandev/web typecheck +``` + +Tests must cover stable tie-breaking, repeated planning with the same inputs, +empty/unknown catalogs, project aggregation, changed-file multipliers, exact +coverage, overlap rejection, and malformed manifest versions. + +## Files likely touched + +- `apps/web/e2e/scripts/plan-shards.ts` +- `apps/web/e2e/scripts/plan-shards.test.ts` +- `apps/web/e2e/scripts/e2e-timings.ts` (only for shared types or fallback + resolution) + +## Dependencies + +Depends on Task 01's versioned profile schema, stable test identity, and +classification/fallback contract. + +## Parallelism + +Sequential after Task 01. CI wiring must consume the manifest shape created +here rather than reimplementing assignment logic in YAML or shell. + +## Inputs + +- The current normal and container project definitions. +- The profile/catalog types and fallback rules from Task 01. +- The shard counts currently defined in `.github/workflows/e2e-tests.yml`. + +## Output contract + +Report the manifest schema, bin-packing rule, coverage validation behavior, +sample predicted distributions, heavy-file diagnostics, files changed, and +exact targeted test result. Include a small checked-in fixture input in the +unit tests, not a generated CI manifest. + +## Implementation result + +Implemented deterministic LPT planning and strict manifest validation. The +local dry-run produced 14 normal and 6 container manifests with complete +catalog coverage; the containers cohort reported the Docker launch file as a +dominant indivisible unit at 240 predicted seconds and 63.2% of its shard. +The focused planner tests passed as part of the 15-test tooling run. +Review remediation replaced source-regex counting with Playwright's discovered +project/test catalog. The planner now matches all 2,103 discovered tests (1,989 +normal and 114 container tests after the new cleanup regression cases). diff --git a/docs/plans/e2e-duration-aware-sharding/task-03-ci-manifest-lifecycle.md b/docs/plans/e2e-duration-aware-sharding/task-03-ci-manifest-lifecycle.md new file mode 100644 index 0000000000..952b2063c5 --- /dev/null +++ b/docs/plans/e2e-duration-aware-sharding/task-03-ci-manifest-lifecycle.md @@ -0,0 +1,81 @@ +--- +id: "03-ci-manifest-lifecycle" +title: "Wire manifests into the CI workflow" +status: completed +wave: 3 +depends_on: + - "01-timing-profile" + - "02-shard-planner" +plan: "plan.md" +spec: "../../specs/e2e-duration-aware-sharding/spec.md" +--- + +# Task 03: Wire manifests into the CI workflow + +## Acceptance + +- The build/planning job obtains the latest successful `main` profile when + available, falls back visibly when it is not, validates both cohorts, and + uploads run-scoped normal/container manifests. +- Normal and container matrix jobs download their own manifest and execute + explicit selections through one shared runner while preserving current + project filters, retries, timeouts, blob reporting, environment, output + isolation, and `workers: 1`; they do not silently use ordinal `--shard`. +- The report job emits a profile candidate, retry summary, predicted-versus- + actual timing summary, and validation/fallback counters, while only + successful `main` artifacts can become the next baseline. + +## Verification + +```sh +cd apps +pnpm --filter @kandev/web test -- e2e/scripts/e2e-timings.test.ts e2e/scripts/plan-shards.test.ts +pnpm --filter @kandev/web typecheck +cd .. +git diff --check +``` + +The workflow review must confirm the permission needed to read another run's +artifact, stable artifact names, failure behavior for missing manifests, and +that every matrix command receives a shard-specific explicit file list. + +## Files likely touched + +- `.github/workflows/e2e-tests.yml` +- `apps/web/e2e/scripts/run-planned-shard.sh` +- `apps/web/e2e/scripts/e2e-timings.ts` +- `apps/web/e2e/scripts/plan-shards.ts` + +## Dependencies + +Depends on Tasks 01 and 02. Do not duplicate profile parsing or bin packing in +the workflow. + +## Parallelism + +Sequential. This task changes the single CI execution path and must land only +after the manifest and fallback unit contracts are tested. + +## Inputs + +- Current build, normal matrix, container matrix, and report jobs in + `.github/workflows/e2e-tests.yml`. +- GitHub Actions artifact download permissions and run selection behavior. +- Existing `apps/web/e2e/scripts/run-e2e.sh` environment and output + conventions. + +## Output contract + +Report the exact job/data flow, artifact names, permission changes, fallback +behavior, command-line invocation, files changed, and local verification +results. Include a dry-run or fixture-based proof that a shard cannot run +without its validated manifest. + +## Implementation result + +The workflow now fetches the latest successful `main` profile when available, +creates and uploads run-scoped `normal/.json` and `containers/.json` +manifests, runs both matrices through the shared validated runner, and uploads +profile/retry/diagnostic artifacts. A missing-manifest invocation failed closed +with the expected error. Backend/web builds and the focused type/lint checks +passed. diff --git a/docs/plans/e2e-duration-aware-sharding/task-04-deterministic-e2e-fixtures.md b/docs/plans/e2e-duration-aware-sharding/task-04-deterministic-e2e-fixtures.md new file mode 100644 index 0000000000..ecb6734026 --- /dev/null +++ b/docs/plans/e2e-duration-aware-sharding/task-04-deterministic-e2e-fixtures.md @@ -0,0 +1,74 @@ +--- +id: "04-deterministic-e2e-fixtures" +title: "Repair deterministic E2E fixtures" +status: completed +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/e2e-duration-aware-sharding/spec.md" +--- + +# Task 04: Repair deterministic E2E fixtures + +## Acceptance + +- The disabled-integrations navigation assertion targets one intended link and + no longer fails because several visible links match `/GitHub/`. +- Desktop and mobile multi-PR review tests derive the expected repository name + from the seeded repository/task fixture, including dynamically generated + names. +- Prompt-autocomplete coverage starts from a verified empty or intentionally + seeded draft and cannot inherit an unrelated session-storage draft from a + prior test. + +## Verification + +```sh +cd apps/web +pnpm exec playwright test --config e2e/playwright.config.ts --project=chromium e2e/tests/integrations/hide-disabled-integrations-nav.spec.ts e2e/tests/review/review-multi-pr.spec.ts e2e/tests/task/task-create-prompt-autocomplete.spec.ts +pnpm exec playwright test --config e2e/playwright.config.ts --project=mobile-chrome e2e/tests/review/mobile-review-multi-pr.spec.ts +``` + +Each regression must fail for the original selector, repository-name, or +restored-draft defect before the repair and pass without increasing retries or +timeouts. + +## Files likely touched + +- `apps/web/e2e/tests/integrations/hide-disabled-integrations-nav.spec.ts` +- `apps/web/e2e/helpers/multi-pr-review.ts` +- `apps/web/e2e/tests/review/review-multi-pr.spec.ts` +- `apps/web/e2e/tests/review/mobile-review-multi-pr.spec.ts` +- `apps/web/e2e/tests/task/task-create-prompt-autocomplete.spec.ts` +- `apps/web/components/task-create-dialog-state.ts` or + `apps/web/e2e/fixtures/test-base.ts` if the smallest isolation boundary is + there + +## Dependencies + +None. The changes are independent of the shard planner. + +## Parallelism + +Parallel candidate with Tasks 01, 05, and 06 because it changes deterministic +test fixtures and selectors only. + +## Inputs + +- The seven retry groups observed in PR #2471. +- Dynamic repository creation in `apps/web/e2e/helpers/multi-pr-review.ts`. +- Draft restore/save behavior in + `apps/web/components/task-create-dialog-state.ts`. + +## Output contract + +Report each original failure mechanism, the deterministic fixture or locator +change, targeted test results for desktop and mobile, and any state-cleanup +contract added for future tests. + +## Implementation result + +The desktop regression set passed 5/5 and the mobile multi-PR test passed as +part of the 3/3 mobile run. The fixes use an exact integration href, derive +review repositories from the seeded ID, and clear all task-create draft keys +before autocomplete assertions. diff --git a/docs/plans/e2e-duration-aware-sharding/task-05-async-e2e-synchronization.md b/docs/plans/e2e-duration-aware-sharding/task-05-async-e2e-synchronization.md new file mode 100644 index 0000000000..d792e64c99 --- /dev/null +++ b/docs/plans/e2e-duration-aware-sharding/task-05-async-e2e-synchronization.md @@ -0,0 +1,70 @@ +--- +id: "05-async-e2e-synchronization" +title: "Repair asynchronous E2E synchronization" +status: completed +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/e2e-duration-aware-sharding/spec.md" +--- + +# Task 05: Repair asynchronous E2E synchronization + +## Acceptance + +- Config-management coverage waits for the actual MCP update completion state + and preserves useful error evidence when the state does not converge; it + does not rely on a marker that can be missed by a 30-second race. +- Mobile autopilot coverage waits for the API/WS/UI hydration boundary before + checking the sidebar state icon and distinguishes a missing event from a + missing render. +- Both regressions pass repeatedly with existing timeouts and expose a + bounded, state-aware wait instead of a larger arbitrary timeout. + +## Verification + +```sh +cd apps/web +pnpm exec playwright test --config e2e/playwright.config.ts --project=chromium e2e/tests/settings/config-management.spec.ts +pnpm exec playwright test --config e2e/playwright.config.ts --project=mobile-chrome e2e/tests/task/mobile-autopilot-mode.spec.ts +``` + +Repeat each targeted command enough to exercise the original retry path and +retain traces or logs when convergence fails. + +## Files likely touched + +- `apps/web/e2e/tests/settings/config-management.spec.ts` +- `apps/web/e2e/tests/task/mobile-autopilot-mode.spec.ts` +- the smallest existing E2E API, WS, or page helper needed to expose a stable + readiness assertion + +## Dependencies + +None. Preserve the current worker-scoped backend and WebSocket fixture +contracts. + +## Parallelism + +Parallel candidate with Tasks 01, 04, and 06. It must not modify the shard +planner or global retry policy. + +## Inputs + +- `runAndWait` and its exact-marker behavior in the config-management spec. +- The mobile autopilot API poll and sidebar assertion ordering. +- Existing Playwright trace, network, and WebSocket diagnostics. + +## Output contract + +Report the observed event/order race, the readiness condition chosen, targeted +repeat results, and evidence that the fix does not merely increase timeout or +retry values. + +## Implementation result + +Config-chat now waits for the backend session to complete before reading the +transcript, and the model round-trip uses an advertised mock model so later +tests cannot inherit an unsupported profile value. The full config-management +suite passed 21/21. Mobile autopilot waits for both hydrated state icons and +passed its focused test within the existing timeout. diff --git a/docs/plans/e2e-duration-aware-sharding/task-06-container-isolation.md b/docs/plans/e2e-duration-aware-sharding/task-06-container-isolation.md new file mode 100644 index 0000000000..95e3e67ada --- /dev/null +++ b/docs/plans/e2e-duration-aware-sharding/task-06-container-isolation.md @@ -0,0 +1,72 @@ +--- +id: "06-container-isolation" +title: "Isolate container E2E state" +status: completed +wave: 1 +depends_on: [] +plan: "plan.md" +spec: "../../specs/e2e-duration-aware-sharding/spec.md" +--- + +# Task 06: Isolate container E2E state + +## Acceptance + +- Storage-maintenance tests create and count only resources owned by the + current test or worker, using unique labels/identifiers rather than a global + `kandev.managed=true` count. +- Setup and teardown remove only resources created by that test and remain + safe when another container E2E shard is running on the same daemon. +- The original first-attempt timeout path has a regression assertion or + diagnostic that distinguishes resource leakage from genuine slow work. + +## Verification + +```sh +cd apps/web +KANDEV_E2E_CONTAINERS=1 pnpm exec playwright test --config e2e/playwright.config.ts --project=containers e2e/tests/docker/storage-maintenance.spec.ts +``` + +Run the targeted test with at least two container processes when the local +Docker daemon permits it, and verify that cleanup does not touch unrelated +Kandev-labelled resources. + +## Files likely touched + +- `apps/web/e2e/tests/docker/storage-maintenance.spec.ts` +- `apps/web/e2e/fixtures/docker-test-base.ts` +- `apps/web/e2e/fixtures/docker-probe.ts` +- related Docker/SSH fixture helpers only if ownership labels cross the shared + boundary + +## Dependencies + +None for implementation. Task 08's performance experiment depends on this +task's scoped cleanup guarantee. + +## Parallelism + +Parallel candidate with Tasks 01, 04, and 05. Coordinate only if a shared +Docker fixture helper is also being changed by another repair. + +## Inputs + +- The global cleanup and count behavior in the Docker fixture. +- The storage-maintenance test's `removeKandevContainers()` call and comments. +- The existing container project timeout and worker lifecycle. + +## Output contract + +Report the ownership label/identifier scheme, cleanup scope, concurrent-shard +test evidence, targeted result, and any daemon prerequisites. Do not solve the +flake by raising the 180-second test timeout. + +## Implementation result + +Docker ownership now propagates from the E2E process into backend-created +containers and storage accounting. The storage-maintenance test passed once +and in two concurrent container processes; no managed containers remained +afterward. Backend lifecycle/storage tests passed in the focused Go packages. +Review remediation now runs process-scoped container removal after every API +reset. A serial regression proves the following test cannot observe a scoped +container left by its predecessor; all three storage-maintenance tests passed. diff --git a/docs/plans/e2e-duration-aware-sharding/task-07-retry-observability.md b/docs/plans/e2e-duration-aware-sharding/task-07-retry-observability.md new file mode 100644 index 0000000000..cea50200f3 --- /dev/null +++ b/docs/plans/e2e-duration-aware-sharding/task-07-retry-observability.md @@ -0,0 +1,73 @@ +--- +id: "07-retry-observability" +title: "Surface retry and timing evidence" +status: completed +wave: 4 +depends_on: + - "03-ci-manifest-lifecycle" +plan: "plan.md" +spec: "../../specs/e2e-duration-aware-sharding/spec.md" +--- + +# Task 07: Surface retry and timing evidence + +## Acceptance + +- Every merged run publishes a retry summary with stable test key, attempts, + final status, error category, and available trace/screenshot/video links. +- The report compares predicted and actual shard durations and records unknown, + warm, fallback, and passed-after-retry counts. +- A scheduled or explicit diagnostic mode can enforce `failOnFlakyTests: true` + without changing the normal PR retry gate during initial rollout. + +## Verification + +```sh +cd apps +pnpm --filter @kandev/web test -- e2e/scripts/retry-summary.test.ts +pnpm --filter @kandev/web typecheck +cd .. +git diff --check +``` + +Add fixture-based tests for first-attempt pass, pass-after-retry, final +failure, timeout, and missing artifact links. + +## Files likely touched + +- `.github/workflows/e2e-tests.yml` +- `apps/web/e2e/scripts/retry-summary.ts` +- `apps/web/e2e/scripts/retry-summary.test.ts` +- `apps/web/e2e/playwright.config.ts` or a diagnostic config override + +## Dependencies + +Depends on Task 03's report artifact flow and the timing identity from Task 01. + +## Parallelism + +Sequential after CI manifest integration. It shares the report job and must +not make the timing collector accept retry samples. + +## Inputs + +- Current blob report merge job and Playwright retry fields. +- The seven PR #2471 retry groups and the persistent failures observed in the + adjacent PR run. +- Existing trace, screenshot, video, and job-summary artifact paths. + +## Output contract + +Report the summary schema, job-summary presentation, diagnostic-mode trigger, +normal PR policy, files changed, and exact focused test results. + +## Implementation result + +Implemented retry grouping with final status, error categories, attachments, +and predicted-versus-actual shard deltas. The report job uploads the retry and +timing diagnostics and supports `fail_on_flaky=true` without changing normal +PR retries. The focused retry-summary tests passed as part of the 15-test +tooling run. +Review remediation copies blob-internal retry resources into the retained +diagnostics artifact, records the artifact name and run URL, and renders +per-test retry/failure links in the GitHub job summary. diff --git a/docs/plans/e2e-duration-aware-sharding/task-08-performance-experiments.md b/docs/plans/e2e-duration-aware-sharding/task-08-performance-experiments.md new file mode 100644 index 0000000000..c164f039a6 --- /dev/null +++ b/docs/plans/e2e-duration-aware-sharding/task-08-performance-experiments.md @@ -0,0 +1,137 @@ +--- +id: "08-performance-experiments" +title: "Run concurrency and setup experiments" +status: completed +wave: 4 +depends_on: + - "03-ci-manifest-lifecycle" + - "06-container-isolation" +plan: "plan.md" +spec: "../../specs/e2e-duration-aware-sharding/spec.md" +--- + +# Task 08: Run concurrency and setup experiments + +## Acceptance + +- A repeatable comparison records `workers: 1` versus `workers: 2` on the + known heavy normal shards, including wall time, CPU/memory pressure, backend + isolation failures, and retry counts; the default remains one worker unless + evidence supports a later change. +- A repeatable comparison measures the unified normal matrix against a + separately balanced mobile matrix, including project/setup overhead and + actual shard tail; no matrix split is enabled without the report. +- Build, pnpm install, runtime-image startup, and Playwright browser extraction + are measured separately, and any cache or pre-baked-image change is kept + behind evidence of lower wall time without a reliability regression. + +## Verification + +```sh +cd apps/web +E2E_SHARD=2 /usr/bin/time -v bash e2e/scripts/run-planned-shard.sh \ + e2e/manifests/normal/2.json -- --workers=2 +E2E_SHARD=2 /usr/bin/time -v bash e2e/scripts/run-planned-shard.sh \ + e2e/manifests/normal/2.json -- --workers=1 +``` + +Generate `e2e/manifests/normal/2.json` with the Task 03 planner first. Run the +paired commands on the same runner class and record results in the experiment +report. The runner forwards the worker override after `--`, so both commands +execute the exact same duration-aware file selection. Do not compare unrelated +ordinal assignments. + +## Files likely touched + +- `.github/workflows/e2e-tests.yml` (diagnostic/manual experiment hooks only) +- `apps/web/e2e/README.md` (experiment procedure and result format) +- CI image/cache configuration if a measured setup optimization is approved + +## Dependencies + +Depends on Task 03 for reproducible manifests and Task 06 for safe concurrent +container behavior. + +## Parallelism + +Sequential after the default manifest workflow is stable. Experiments must be +isolated from the blocking PR lane. + +## Inputs + +- Current workers-one invariant and worker-scoped backend fixture. +- Heavy normal and container shard timing evidence from the investigation. +- Build and setup timings from the PR #2471 workflow run. + +## Output contract + +Report paired measurements, runner/environment details, flake/resource +observations, recommendation, and explicit statement of which defaults remain +unchanged. + +## Experiment report (2026-08-11) + +The README contains the controlled experiment procedure and result format, and +the runner forwards worker overrides after the manifest separator. The local +experiment used the post-merge checkout at `d150554a5`, a rebuilt backend/web +artifact, and the heaviest predicted normal shard (`normal/10.json`, 44 files, +191 tests). The runner image does not provide GNU `/usr/bin/time`, so Bash +`time -p` supplied wall/user/system time and a process-group sampler supplied +peak RSS and CPU for the monitored runs. + +### Environment + +- Node `v24.16.0`; pnpm `9.15.9`. +- AMD Ryzen 5 7640HS, 6 cores / 12 threads; 10 online CPUs reported to the + process. +- 18 GiB RAM, 15 GiB available at measurement start, 8 GiB swap. +- Docker `26.1.5`, Linux `amd64`. +- Local build artifacts were rebuilt at the measured commit before E2E runs. + +### CI comparison + +The latest successful main baseline, [run 31423650879](https://github.com/kdlbs/kandev/actions/runs/31423650879), took 28m30s from workflow creation to completion. The post-conflict PR run, [run 31469837346](https://github.com/kdlbs/kandev/actions/runs/31469837346), took 22m39s and passed all 42 checks: a 5m51s (20.5%) full-workflow reduction. + +The PR run's normal shard cohort ran from 07:45:35Z to the slowest job finish at 08:00:41Z (15m06s); the slowest normal job was 12m47s. The baseline's slowest normal job was 17m41s. The container tail improved from 9m26s to 5m14s. The PR retry summary recorded 2 tests that passed after one retry, 0 final failures, and 0 timeouts. + +Both the PR run and the latest successful main run used the deterministic +count-fallback path: the latest main run, [31469080336](https://github.com/kdlbs/kandev/actions/runs/31469080336), has no `e2e-timing-profile` artifact. The first successful main run after this change lands will bootstrap the rolling profile; until then, the planner is implemented but the CI timing-profile path is not yet exercised. + +### Worker experiment + +| workers | repetitions | wall time | result | sampled peak RSS | decision | +| ---: | ---: | --- | --- | ---: | --- | +| 1 | 3 | 458.110s / 458.237s / 458.252s; median 458.237s; CV 0.014% | 191/191 passed, no retries | 874–948 MiB on 2 monitored runs | keep default | +| 2 | 1 | 268.715s | 189 passed; 2 flaky tests passed after retry; exit 1 with `E2E_FAIL_ON_FLAKY=1` | 1.32 GiB | reject | + +The two-worker run failed the reliability gate with a passthrough-terminal +assertion and a `fetch failed` error. It is faster by 41.4%, but it does not +meet the zero-retry/zero-isolation-error requirement. The checked-in default +remains `workers: 1`; no candidate worker change was applied. + +### Matrix, shard-shape, and setup measurements + +- The current unified 14-shard profile simulation predicts a 611.1s maximum + shard. The best separate matrix allocation was 11 Chromium + 3 mobile + shards at 653.7s (+7.0%) before extra setup, backend, and report overhead. + The split was not executed because it failed the 10% improvement threshold. +- Normal shard simulation produced maxima of 855.3s at 10 shards, 611.1s at + 14, and 534.8s at 16. Container maxima were 210.9s at 4, 154.8s at 6, and + remained 154.8s at 8 because `docker-launch.spec.ts` is the dominant + indivisible unit. Keep 14 normal and 6 container shards. +- On the CI runner, dependency install took 13s, backend/web build took + 4m44s, and the complete Build job took 6m18s. Container setup extracted + Playwright browsers in 33s. Warm local measurements were pnpm install + 0.93s, browser install verification 0.57s, and a local runtime-image + startup probe 0.45s. Cold cache misses and image pulls still need a runner + measurement before changing setup or image strategy. + +### Decisions + +- **Adopt now:** keep the generated-manifest workflow, 14/6 matrix, unified + normal/mobile projects, and `workers: 1`; retain retry and shard diagnostics. +- **Needs CI confirmation:** consume the first post-merge main timing profile, + measure cold setup/cache behavior, and repeat any future worker candidate on + the CI runner class. +- **Reject:** workers=2 for the current shared-backend fixture, and a separate + mobile matrix based on the profile simulation. diff --git a/docs/plans/e2e-duration-aware-sharding/task-09-rollout-documentation.md b/docs/plans/e2e-duration-aware-sharding/task-09-rollout-documentation.md new file mode 100644 index 0000000000..7c5415f9d1 --- /dev/null +++ b/docs/plans/e2e-duration-aware-sharding/task-09-rollout-documentation.md @@ -0,0 +1,73 @@ +--- +id: "09-rollout-documentation" +title: "Document rollout and operating checks" +status: completed +wave: 5 +depends_on: + - "03-ci-manifest-lifecycle" + - "06-container-isolation" + - "07-retry-observability" +plan: "plan.md" +spec: "../../specs/e2e-duration-aware-sharding/spec.md" +--- + +# Task 09: Document rollout and operating checks + +## Acceptance + +- `apps/web/e2e/README.md` documents profile ownership, artifact names, + manifest generation, new/changed-test fallback behavior, local dry runs, + retry summaries, and container ownership requirements. +- Workflow comments and CI job summaries explain how to diagnose a stale or + unavailable profile, an invalid manifest, a passed-after-retry result, and a + shard predicted/actual mismatch. +- The rollout checklist records the three-successful-main-run balance check, + the no-hidden-flake check, and the default `workers: 1`/matrix decisions. + +## Verification + +```sh +cd apps/web +pnpm typecheck +cd ../.. +git diff --check +``` + +Review all paths in the linked spec, plan, and ADR after the implementation +changes so command names, artifact names, and status claims match the actual +workflow. + +## Files likely touched + +- `apps/web/e2e/README.md` +- `.github/workflows/e2e-tests.yml` +- `docs/specs/e2e-duration-aware-sharding/spec.md` (status or contract sync) +- `docs/decisions/2026-08-10-duration-aware-e2e-sharding.md` (consequence or + implementation-status sync if needed) + +## Dependencies + +Depends on the final CI, container, and retry contracts from Tasks 03, 06, and 07. It should not invent a second source of truth for those contracts. + +## Parallelism + +Sequential finalization after behavior stabilizes. + +## Inputs + +- The generated workflow behavior and report examples. +- Existing E2E README commands and container prerequisites. +- The spec success criteria and proposed ADR. + +## Output contract + +Report documentation paths changed, command examples verified, rollout status, +and any remaining discrepancies between artifacts and implementation. + +## Implementation result + +Updated the E2E README, workflow comments/job summary, spec, plan, ADR, and +task statuses to describe the final artifact names, fallback behavior, scoped +Docker ownership, retry diagnostics, and rollout checks. The README commands +were reviewed against the generated runner and workflow paths; typecheck and +diff checks passed. diff --git a/docs/specs/INDEX.md b/docs/specs/INDEX.md index d904a6de5a..5e8fc8323c 100644 --- a/docs/specs/INDEX.md +++ b/docs/specs/INDEX.md @@ -256,6 +256,7 @@ System pages (Radarr/Sonarr-style) for status, disk usage, database maintenance, | [browser-inspect-annotations-save](browser-inspect-annotations-save/spec.md) | shipped | | [automations-pr-merged-trigger](automations-pr-merged-trigger/spec.md) | draft | | [no-silent-model-fallback](no-silent-model-fallback/spec.md) | approved | +| [e2e-duration-aware-sharding](e2e-duration-aware-sharding/spec.md) | implemented | | [board-step-visibility-filter](board-step-visibility-filter/spec.md) | draft | --- diff --git a/docs/specs/e2e-duration-aware-sharding/spec.md b/docs/specs/e2e-duration-aware-sharding/spec.md new file mode 100644 index 0000000000..a53a4e97b8 --- /dev/null +++ b/docs/specs/e2e-duration-aware-sharding/spec.md @@ -0,0 +1,187 @@ +--- +status: implemented +created: 2026-08-10 +--- + +# Duration-aware E2E sharding and CI reliability + +## Why + +The E2E workflow currently uses Playwright's ordinal test-level sharding. It +keeps test counts roughly even, but not execution time. In the investigated +run, the normal matrix had fourteen shards ranging from 3m52 to 17m57, while +the container matrix had six shards ranging from about 3m to 9m59. The slowest +shards therefore determine most of the wall time even when the other runners +are idle. + +The same investigation found seven retry groups in the normal PR run. The +retry policy made the final run green, but several failures represented +unstable test data, leaked state, or missing synchronization rather than +transient infrastructure. Container tests also remove globally labelled +containers, so unrelated daemon state can affect their result. + +This capability makes shard assignment adapt to the suite as it grows. It +also turns the known reliability findings into deterministic regression work +and makes passed-after-retry results visible. + +## What + +The E2E workflow gains two duration-aware cohorts: + +- **Normal:** the existing fourteen-shard Chromium and mobile-Chrome matrix. +- **Containers:** the existing six-shard Docker and SSH matrix. + +Each run creates an ephemeral manifest for each cohort. The manifest contains +the explicit test files assigned to each shard, the estimated cost, and the +profile source used for that estimate. Playwright runs those files directly; +the first implementation keeps `workers: 1` and the existing project +boundaries. + +The estimates come from a rolling timing profile produced by successful runs +on `main`. A later `main` run merges its new successful samples into the +profile, so adding tests and changing test durations update future plans +without a hand-edited shard file. + +## Timing profile contract + +The profile is an artifact, not a checked-in source file. Its stable test key +is: + +```text + + + +``` + +Each entry records the source file hash, recent successful samples, a p50 and +p75 duration, the last source commit, and the last successful run. The +collector retains a bounded recent sample history per test. Only a successful +first attempt is a baseline sample. Failed, timed-out, and retry attempts are +excluded so a flake or an exceptional timeout does not permanently inflate the +plan. + +The latest successful `main` profile is authoritative. Pull request runs may +publish diagnostic timing artifacts, but they do not replace the `main` +baseline. The planner applies these fallbacks in order: + +1. Use the entry's p75 for an unchanged test. +2. Apply a conservative changed-file multiplier to a matching entry whose + source file hash differs from the profile. +3. Use a project/cohort fallback estimate for a new or unknown test. +4. If no profile can be downloaded, use a deterministic count-based plan. + +Unknown, changed, and stale counts are reported with every plan. Profile +entries that are no longer discovered are ignored by planning and eventually +removed when the bounded history is compacted. + +## Shard planner contract + +The planner discovers the same Playwright test catalog used by the selected +cohort. It treats a project/file pair as the initial indivisible work unit, +sums the selected project costs for a file when projects share it, sorts units +by estimated duration descending, and assigns each unit to the currently +lightest shard. Ties use stable project/file ordering and the lowest shard +number. This makes a plan reproducible for the same catalog and profile. + +Every manifest must pass these checks before it is uploaded: + +- every discovered project/file unit appears exactly once; +- no manifest contains a unit outside the selected cohort; +- the declared shard count is exactly fourteen or six as appropriate; +- the sum of assigned estimates matches the per-shard totals; +- unknown and changed units are listed rather than silently omitted. + +If a single file remains a material outlier after weighted file planning, a +later planner revision may split that file into `file:line` test selections. +That refinement is data-driven and does not change the initial manifest +contract. + +## CI lifecycle + +1. The build/planning job builds the existing artifacts and obtains the latest + successful `main` timing profile using the workflow token. +2. It discovers the catalog, creates normal and container manifests, validates + coverage, and uploads the manifests as a run-scoped artifact. +3. Each matrix job downloads only its cohort manifest and invokes a wrapper + with the explicit file selections. It preserves the existing blob reports, + project selection, retry count, timeout, and `workers: 1` behavior. +4. The report job merges the blob reports and emits the new timing profile, + retry summary, prediction-versus-actual summary, and plan health counters. +5. Only a successful `main` result is eligible to become the next baseline. + +If the `main` artifact is unavailable, the workflow continues with the +validated count-based fallback and records that fallback in the report. A +missing or malformed manifest is a hard planning failure, not permission to +silently revert one matrix shard to ordinal sharding. + +## Reliability contract + +The following findings from PR #2471 become focused E2E repairs: + +- scope the disabled-integrations link assertion to an exact, unique locator; +- derive multi-PR review expectations from the dynamically created repository + instead of a fixed repository name; +- clear or isolate restored task-create drafts before testing prompt + autocomplete; +- replace the fragile config-management marker wait with a state-aware + synchronization point; +- wait for mobile autopilot sidebar hydration/WS convergence before asserting + its state; +- make Docker storage-maintenance cleanup task-owned and use unique labels or + scoped counts instead of deleting or counting unrelated managed containers. + +The desktop and mobile multi-PR failures share the same fixture defect and +must be fixed together. Each repaired failure gets a regression assertion that +fails for the observed cause, not only a larger timeout. + +Playwright output also includes a retry summary containing the test key, +attempt count, final status, error context, and links to retry artifacts. The +normal PR lane may retain its current retry gate while the summary is being +rolled out. A scheduled or explicitly diagnostic lane runs with +`failOnFlakyTests: true` so passed-after-retry results can be treated as a +first-class reliability signal. + +## Performance experiments + +The default remains one worker per shard until measurements prove that higher +concurrency is safe for the worker-scoped backend fixture. A controlled +experiment measures `workers: 2` on the known heavy shards and records wall +time, CPU/memory pressure, and retries before considering a default change. + +A second experiment compares the unified normal matrix with a separately +balanced mobile matrix. It must use the same timing profile and report setup +overhead before changing the default matrix. Build/startup work is measured as +a separate budget, including package installation, runtime image startup, and +Playwright browser extraction; caching or pre-baked images are adopted only if +they improve wall time without increasing flakes. + +## Success criteria + +- Three consecutive successful `main` runs show materially smaller shard-tail + skew than the current baseline. The initial target is no shard above 1.25x + the cohort median unless one indivisible test unit dominates it. +- A newly added test appears in the next generated catalog without editing a + shard assignment file. +- A changed test file is marked warm and receives the conservative estimate + until successful `main` samples replace it. +- Manifest validation proves complete, non-overlapping coverage for both + cohorts on every run. +- The seven observed retry groups either have deterministic fixes or remain + visible in the retry summary with actionable evidence. +- The timing profile can be unavailable without blocking a correct, + deterministic fallback plan. + +## Out of scope + +- changing product behavior or the worker-scoped backend isolation model; +- making `workers: 2` the default before the experiment passes; +- committing per-shard test lists that require manual maintenance; +- moving timing data to an external database; +- globally deleting Docker resources to make a test pass; +- masking flakes by increasing timeouts or retries. + +## Implementation status + +Implemented in the CI workflow, E2E tooling, scoped Docker fixtures, and +focused reliability tests. The generated manifests, rolling profile, retry +diagnostics, and fail-closed runner are covered by unit/type checks and local +targeted E2E runs. The three-successful-main-run balance target remains an +operational check after merge.