From 8cdc7ddcb06c99374bccb600813f99d3211e40a1 Mon Sep 17 00:00:00 2001 From: Mohamed Aziz Mejri Date: Sat, 22 Aug 2026 23:36:26 +0100 Subject: [PATCH 1/9] feat: isolate E2E test execution in sandboxes --- plans/sandboxed-e2e-test-runtime.md | 489 ++++++++++++++++++ src/atoms/testRuntimeAtoms.ts | 2 +- src/ipc/handlers/app_handlers.ts | 2 + src/ipc/handlers/tests_handlers.test.ts | 172 +++++- src/ipc/handlers/tests_handlers.ts | 273 +++++++--- src/ipc/services/e2e_test_data_isolation.ts | 33 ++ src/ipc/services/e2e_test_runtime.test.ts | 75 +++ src/ipc/services/e2e_test_runtime.ts | 223 ++++++++ src/ipc/services/e2e_test_workspace.test.ts | 156 ++++++ src/ipc/services/e2e_test_workspace.ts | 192 +++++++ src/ipc/services/isolated_test_db.test.ts | 50 ++ src/ipc/services/isolated_test_db.ts | 44 +- src/ipc/utils/neon_utils.test.ts | 66 ++- src/ipc/utils/neon_utils.ts | 36 ++ src/ipc/utils/playwright_bootstrap.ts | 7 +- src/ipc/utils/test_screenshot.ts | 40 +- src/main.ts | 4 + .../handlers/local_agent/tools/run_tests.ts | 1 + 18 files changed, 1778 insertions(+), 87 deletions(-) create mode 100644 plans/sandboxed-e2e-test-runtime.md create mode 100644 src/ipc/services/e2e_test_data_isolation.ts create mode 100644 src/ipc/services/e2e_test_runtime.test.ts create mode 100644 src/ipc/services/e2e_test_runtime.ts create mode 100644 src/ipc/services/e2e_test_workspace.test.ts create mode 100644 src/ipc/services/e2e_test_workspace.ts diff --git a/plans/sandboxed-e2e-test-runtime.md b/plans/sandboxed-e2e-test-runtime.md new file mode 100644 index 0000000000..e752da5bd1 --- /dev/null +++ b/plans/sandboxed-e2e-test-runtime.md @@ -0,0 +1,489 @@ +# Sandboxed app runtime for user-triggered E2E tests + +## Status + +Implemented for host-runtime test execution. The Tests panel and agent test tool +now use a disposable workspace, run-scoped server, sandbox-only Neon env, and +retained artifact directory. Interactive test recording remains on its existing +preview-oriented lifecycle and was deliberately not migrated. + +## Problem + +Neon test execution currently isolates the data but not the app runtime. Dyad +creates a temporary Neon branch, rewrites the real app's `.env.local`, and +restarts the user's existing preview against that branch. Playwright then uses +the existing preview proxy. While the run is active, any user interaction with +the preview therefore reaches the throwaway branch; setup and teardown also +interrupt the user's dev server. + +The test runner also uses the real app directory as its working directory, so +reports and any test-time filesystem writes land in the user's project. + +## Goal + +Run user-triggered E2E tests against a disposable copy of the app with its own +environment, process, and port, while the normal preview continues running from +the real app directory and against the user's normal database configuration. + +The sandbox must represent the app as it exists when Run is pressed, including +tracked modifications and relevant untracked/ignored runtime files. It must not +be limited to the last Git commit. + +## Non-goals + +- Do not change the recorder. Recording continues to drive the normal preview + and use the existing database-isolation lifecycle. +- Do not replace the app preview iframe or introduce an Electron session + partition. +- Do not copy test-generated source changes back into the real app. +- Do not use a Git worktree as the required implementation. Git may be absent, + unhealthy, or behind the app's current on-disk state. +- Do not support concurrent test runs for the same app in the first version. +- Do not install dependencies independently inside every sandbox. +- Do not silently fall back to running a Neon test against the user's real + database or normal preview when sandbox setup fails. + +## Product behavior + +1. The user presses Run while their normal preview is running. +2. Setup progress reports distinct steps: preparing Playwright if necessary, + copying the current app, creating isolated data, and starting the test + server. +3. The normal preview remains available and is neither stopped nor restarted. +4. Playwright targets the sandbox server URL only. +5. Stop terminates Playwright, then the sandbox server, then remote test-data + resources, and finally deletes the sandbox directory. +6. Test results remain visible through the existing result model. Artifacts + needed by the result UI are promoted to a Dyad-owned artifact directory + before the sandbox is deleted. +7. A setup failure says which stage failed and confirms that the normal preview + was not changed. + +The first-ever Playwright bootstrap remains a transparent, one-time project +setup operation because it intentionally adds `@playwright/test`, Dyad's config, +and ignore entries to the user's app. It runs before the snapshot. Ordinary +runs after bootstrap do not mutate the real app. + +## Proposed architecture + +### 1. `E2eTestWorkspace` + +Add an E2E-only workspace service, separate from database isolation: + +```ts +interface E2eTestWorkspace { + realAppPath: string; + workspacePath: string; + artifactPath: string; + dispose(): Promise; +} + +createE2eTestWorkspace({ + appId, + appPath, + signal, + onProgress, +}): Promise +``` + +Place workspaces beneath a Dyad-owned user-data directory such as +`/test-sandboxes//`, rather than beside the user's repo. +This gives startup reconciliation a bounded, recognizable cleanup root and +prevents temp-directory policy differences from leaking into the feature. + +#### Copy strategy + +Use the existing repository pattern of recursive `fs.cp` with +`COPYFILE_FICLONE` and a filter on macOS/Linux. A reflink-capable filesystem +makes the initial clone copy-on-write. Fall back to an ordinary filtered copy +when cloning is unsupported. Use a filtered ordinary copy on Windows initially +and measure it before adding platform-native block cloning. + +Never hard-link application files: a write through a hard link would mutate the +real file. + +Exclude heavyweight or disposable roots, initially: + +- `.git` +- `node_modules` +- framework output/caches: `dist`, `build`, `out`, `.vite`, `.next`, `.nuxt`, + `.svelte-kit`, `.turbo`, `.cache` +- test output: `test-results`, `playwright-report`, `coverage` +- OS metadata such as `.DS_Store` + +Do not use `.gitignore` wholesale. Ignored files can be runtime inputs, including +environment files and generated assets. Preserve all other files as they exist +on disk, including uncommitted and untracked files. + +After copying the source tree, clone `node_modules` into the workspace with the +same copy-on-write strategy. Do not link the dependency root back to the real +app: Vite resolves that link to the real directory, after which Nitro runtime +entries fall outside the sandbox filesystem boundary and fail with +`ERR_LOAD_URL`. Preserve pnpm's relative links within the cloned dependency tree. +If dependencies are absent after the pre-snapshot bootstrap, fail setup with an +actionable error. + +Treat writable dependency caches as a measured risk. Verify Vite, Next, and the +supported package managers in tests; add sandbox-local cache environment +overrides or exclusions for any tool proven to write beneath `node_modules`. + +#### Snapshot consistency + +Take the copy while holding the app path and repository-worktree coordination +claims so Dyad-driven edits, restores, imports, and path changes cannot +interleave. Release those claims once the copy is complete so the user can keep +editing while Playwright runs against the captured state. External IDE writes +cannot be made globally atomic; document the snapshot boundary as the moment +Run is pressed and make copy failures fail closed. + +### 2. `E2eTestDataIsolation` + +Do not make `prepareIsolatedTestDatabase` serve two incompatible lifecycles. +Keep it unchanged for recording, including its real-preview restart and env +restoration behavior. Extract shared provider primitives where useful, then add +an E2E-only data-isolation service whose contract targets the workspace: + +```ts +interface PreparedE2eTestData { + isolation: TestIsolation; + testCredentials?: Record; + applyToWorkspace(workspacePath: string): Promise; + dispose(): Promise; +} +``` + +#### Neon + +1. Create and durably track the temporary branch using the existing Neon branch + primitives. +2. Read the already-copied workspace environment. +3. Call the existing framework-aware Neon env updater with `workspacePath`, + never `realAppPath`. +4. Provision the optional Better Auth test account as today. +5. On cleanup, delete/mark the branch using the existing durable ordering. + +There is no real `.env.local` restoration step because it was never changed. +Accordingly, remove E2E-only `envRestored` failure handling after migration; +retain it in the recorder path. Continue tracking the branch ID durably so a +crash can clean up a leaked remote branch even though the user's env is safe. + +#### Supabase + +Continue creating an RLS-scoped test user and cleaning up its data/user. The +sandbox runtime still provides filesystem/process isolation even though the +database endpoint does not change. + +#### No database + +Still use the workspace and test runtime. Return `mode: "none"` only for data +isolation; it must not mean runtime isolation was skipped. + +#### Runtime modes + +Implement the first version for host runtime. For Docker/cloud runtime, retain +the current behavior only where it is already safe and make the lack of runtime +sandboxing explicit in the UI. For Neon specifically, never degrade to the +normal preview/real database on sandbox failure. A later phase may provide +Docker-specific volumes or cloud sandbox clones behind the same contract. + +### 3. `E2eTestRuntime` + +Add a short-lived runtime owned by the test run: + +```ts +interface E2eTestRuntime { + baseUrl: string; + processId?: number; + stop(): Promise; +} + +startE2eTestRuntime({ + app, + workspacePath, + signal, + onOutput, +}): Promise +``` + +Extract and reuse app-runtime command detection, managed Node/pnpm selection, +environment construction, and readiness parsing where possible. Do not call the +normal `executeApp` path unchanged: it is keyed by `appId`, publishes into +`runningApps`, drives the app-run actor, owns the normal deterministic ports, +and could replace or stop the user's preview. + +The test runtime must instead: + +- have a run-scoped identity rather than using `appId` as a singleton key; +- run with `cwd: workspacePath`; +- receive a unique app port; +- remain outside `runningApps` and normal preview/app-run state; +- stream setup output into the existing test output channel with a clear + `[test server]` prefix; +- resolve only after an HTTP readiness probe succeeds; +- stop only the exact child/process tree it started; +- never call a cleanup helper that kills an unknown process merely because it + owns the desired port. + +Prefer Playwright targeting the test dev server directly. It does not need the +normal preview proxy because there is no iframe, stable browser origin, or +preview navigation UI to preserve. When the disposable Neon branch has Auth +enabled, register the test server's exact run-scoped origin as a trusted domain +after its random port is known and before Playwright starts. Registration must +preserve the HTTP scheme rather than applying the HTTPS normalization used for +deployment domains, and must target only the disposable branch; teardown +deletes it with that branch. If authorization fails, stop before Playwright +rather than producing misleading per-test `INVALID_ORIGIN` failures. Do not +disable Better Auth's origin checks, use a loopback wildcard, or reuse the +normal app proxy. + +Allocate an OS-available port and pass it through the same framework-specific +start-command machinery used by the normal runtime. Because selecting a free +port has a bind race, detect address-in-use startup failure and retry with a new +port a bounded number of times. Do not reserve a port by killing its listener. + +### 4. Make the test core accept explicit execution inputs + +Change `runAppTestsCore` so it no longer looks up the real app path and normal +preview URL internally: + +```ts +runAppTestsCore({ + appId, + appPath: workspace.workspacePath, + baseUrl: testRuntime.baseUrl, + artifactPath: workspace.artifactPath, + // existing selector, output, timeout, and credential options +}); +``` + +Keep path/URL resolution in the orchestration layer. Run `npx playwright` with +the sandbox as `cwd`, write the JSON report inside it, and parse paths relative +to the sandbox. Before disposal, copy retained traces/screenshots/report data to +the run's artifact directory and rewrite result attachment paths if the UI +stores them. + +Bootstrap Playwright against the real app before workspace creation. This +preserves the current user-visible project setup semantics and ensures the +generated Dyad config and dependency are present in the snapshot. Split +`ensurePlaywrightBootstrap` from the pure "verify ready" step so code inside the +sandbox cannot accidentally invoke an install through the shared +`node_modules` clone. + +### 5. Lifecycle orchestration and coordination + +Refactor the E2E handler into explicit stages with one cleanup stack: + +```text +register run/cancellation owner + -> bootstrap Playwright in real app (when required) + -> snapshot workspace + -> prepare isolated test data in workspace + -> start test runtime + -> run Playwright against explicit baseUrl + -> retain artifacts +finally + -> stop Playwright/process tree + -> stop test runtime + -> dispose isolated data + -> delete workspace + -> publish terminal result +``` + +Register every acquired cleanup immediately after acquisition. Run cleanup in +reverse order and continue after individual cleanup failures, reporting all +failures without hiding the original run error. Cancellation must be checked +between every awaited setup stage. Teardown is not abortable once a remote +branch/user exists. + +Do not hold the current broad coordinator claim for the entire Playwright run. +Use staged claims: + +1. **Bootstrap/snapshot:** read `app-path` and `repository-ref`; claim + `repository-worktree` and `test-files` while bootstrap files or the snapshot + may change/read them. +2. **Provider lifecycle:** hold `provider` from temporary branch/user creation + through deletion, preventing unlink/delete races. +3. **Sandbox runtime:** use a new run-scoped owner/registry rather than claiming + the normal app `runtime` or `runtime-config`, because those claims would + unnecessarily block the preview the feature is meant to preserve. + +Audit app deletion, app relocation, provider unlink, Stop, replacement Run, and +application shutdown. These must cancel and await a run where required; they +must not delete provider state or the sandbox root while cleanup is active. +Preserve the existing same-app single-run controller ordering. Runs for +different apps may proceed concurrently once port and resource ownership are +run-scoped. + +### 6. Crash recovery + +On Dyad startup: + +- remove abandoned directories only beneath the recognized + `test-sandboxes` root; +- never follow directory links during recursive cleanup; +- rely on the existing durable Neon branch marker/reconciliation to remove + leaked remote branches; +- ensure no real-env restoration gate is applied to E2E-only leaked branches, + because the real env was never modified; +- keep recorder recovery behavior unchanged. + +The durable branch state may need to distinguish `recorder-env-swapped` from +`e2e-cleanup-only`; add that distinction before sharing startup reconciliation. +Do not infer it solely from whether a sandbox directory still exists. + +## Implementation phases + +### Phase 1: Workspace primitive and benchmarks + +- Add the filtered reflink/copy workspace service and safe disposal. +- Add copy-on-write dependency-tree cloning with pnpm-link preservation. +- Add timing/size telemetry without recording absolute user paths. +- Benchmark representative Vite and Next apps on macOS, Windows, and Linux. +- Set a soft progress threshold (show ongoing file count/bytes after 500 ms), + not a hard correctness timeout. + +Exit criterion: a workspace contains current tracked/untracked source and env +files, excludes known heavy outputs, cannot mutate real source, and starts with +resolvable dependencies. + +### Phase 2: Unregistered host test runtime + +- Extract reusable command/env/readiness pieces from `app_runtime_service`. +- Implement the run-scoped process registry, port retry, output, readiness, and + process-tree teardown. +- Prove that starting/stopping it does not change `runningApps`, the app-run + actor, normal proxy URL, or normal preview process. + +Exit criterion: normal and test servers run simultaneously from different +directories and ports, and stopping either leaves the other alive. + +### Phase 3: E2E-only data isolation and runner wiring + +- Add workspace-targeted Neon env rewriting and shared provider primitives. +- Keep the recorder on `prepareIsolatedTestDatabase` unchanged. +- Pass explicit `appPath`/`baseUrl` into `runAppTestsCore`. +- Promote artifacts, then dispose the sandbox. +- Replace E2E env-restoration messaging with sandbox-cleanup messaging. + +Exit criterion: during a Neon test, the real `.env.local` is byte-identical, +the normal preview keeps its PID/URL and real branch, and Playwright reaches the +throwaway branch through the test server. + +### Phase 4: Coordination, recovery, and rollout + +- Narrow broad whole-run coordinator claims into staged ownership. +- Complete delete/relocate/unlink/shutdown/rapid-rerun audits. +- Add abandoned-workspace startup cleanup and distinguish durable Neon cleanup + states. +- Gate the new path behind a temporary feature flag for soak testing, but fail + closed rather than silently use the legacy runtime when the flag is enabled. +- Remove the legacy E2E env-swap path after cross-platform validation. Do not + remove or redirect the recorder path. + +## Test plan + +### Unit tests + +- Workspace filter includes modified/untracked/ignored runtime inputs and + excludes every declared heavyweight root. +- LF/CRLF environment files remain valid and only sandbox Neon values change. +- Reflink failure falls back to copy; cancellation removes a partial workspace. +- Pnpm dependency realpaths remain inside the sandbox and missing-dependency + errors are platform-correct. +- Sandbox disposal rejects paths outside its owned root and does not follow + malicious links. +- Port collision retries without killing the existing listener. +- Cleanup stack runs in reverse order, attempts every cleanup, and preserves the + primary error plus cleanup diagnostics. +- Explicit runner paths parse reports and attachments relative to the sandbox. + +### Vitest integration tests + +- Start a real fixture dev server normally, create a sandbox server, and assert + distinct CWDs, PIDs, ports, and environment values. +- Modify and add fixture files before Run; assert the sandbox sees them while a + post-snapshot edit affects only the normal workspace. +- Assert Playwright bootstrap happens before snapshot and no install command is + invoked from the sandbox. +- Cancel during copy, branch creation, server readiness, Playwright, and cleanup; + assert no live child/workspace remains and remote cleanup is attempted. +- Start Run twice rapidly; assert the second waits for the first cleanup and + cannot delete the successor's workspace or branch. +- Delete/unlink/relocate/shutdown coordination tests prove there is no orphaned + process or provider race. +- Recorder regression tests assert it still uses the existing preview-oriented + isolation service and restart behavior. + +### Playwright E2E coverage + +Add one broad test using a fixture app whose page displays a server-read env +marker: + +1. Start the normal preview with marker `real` and capture its PID/URL. +2. Run a test with sandbox marker `isolated`. +3. While it is running, verify the normal preview still displays `real` and is + interactive. +4. Verify the generated test observes `isolated`. +5. Stop or finish the run and assert the original PID/URL and `.env.local` + content never changed. +6. Assert the sandbox process/directory is gone and retained results remain + readable. + +Add a second targeted cancellation case only if the integration harness cannot +exercise real process-tree teardown reliably. + +## Observability and acceptance criteria + +Log structured timings for bootstrap, copy, data setup, server readiness, test +execution, artifact promotion, and cleanup. Include app ID/run ID but never env +contents, credentials, or absolute project paths. + +The feature is complete when: + +- the real app environment is byte-identical before, during, and after E2E; +- the normal preview process and URL do not change during E2E; +- Playwright can only reach the sandbox server URL supplied to its config; +- current uncommitted and relevant untracked files are tested; +- test writes and reports do not pollute the real app; +- Stop and app shutdown leave no child process, sandbox, test user, or Neon + branch after cleanup/reconciliation; +- filtered workspace creation is normally faster than test-server startup; +- Windows fallback performance is measured and acceptable; +- recorder behavior and its tests are unchanged. + +## Key risks and mitigations + +- **Shared `node_modules` is mutated:** prohibit sandbox installs, split + bootstrap verification from installation, and isolate proven writable caches. +- **Framework command logic diverges:** extract command construction/readiness + from the normal runtime rather than duplicating it. +- **Artifacts disappear with the workspace:** promote them before disposal and + test every attachment path consumed by the UI. +- **External editor writes during copy:** coordinate all Dyad writers and define + a best-effort on-disk snapshot boundary; fail on inconsistent filesystem + errors rather than silently mixing in the real directory later. +- **Port race:** bounded retry on bind failure; never kill an unknown listener. +- **Crash leaks remote data:** preserve durable branch markers and distinguish + E2E cleanup-only state from recorder env-restoration state. +- **Sandbox cleanup escapes its root:** validate canonical containment and do + not traverse links. +- **Normal preview is accidentally registered/restarted:** keep the test runtime + out of `runningApps` and add assertions around the app-run actor and proxy. + +## Likely code areas + +- `src/ipc/handlers/tests_handlers.ts`: orchestration, explicit runner inputs, + cancellation, artifacts, and staged coordination. +- `src/ipc/services/isolated_test_db.ts`: recorder-compatible service remains; + extract only provider primitives shared with the new E2E service. +- `src/ipc/services/app_runtime_service.ts`: extract reusable command/env and + readiness helpers without reusing singleton runtime registration. +- New `src/ipc/services/e2e_test_workspace.ts`. +- New `src/ipc/services/e2e_test_runtime.ts`. +- New `src/ipc/services/e2e_test_data_isolation.ts`. +- `src/ipc/utils/playwright_bootstrap.ts`: split mutating bootstrap from sandbox + readiness verification. +- `src/ipc/utils/neon_test_branch.ts`: durable cleanup-state distinction. +- Startup reconciliation and app deletion/shutdown paths that currently recover + or stop tests. +- Unit/integration tests beside each service plus one broad packaged E2E spec. diff --git a/src/atoms/testRuntimeAtoms.ts b/src/atoms/testRuntimeAtoms.ts index 176e089ab2..e4fb06a206 100644 --- a/src/atoms/testRuntimeAtoms.ts +++ b/src/atoms/testRuntimeAtoms.ts @@ -44,7 +44,7 @@ export type TestRunPhase = | "setup" // first-run Playwright bootstrap streaming | "running" // playwright test executing | "stopping" // Stop pressed; killing the Playwright process tree - | "cleaning-up"; // tests gone; isolation teardown still restoring the app + | "cleaning-up"; // tests gone; isolated provider data is still being removed export interface TestRunState { phase: TestRunPhase; diff --git a/src/ipc/handlers/app_handlers.ts b/src/ipc/handlers/app_handlers.ts index 4f10566413..c42fdeabef 100644 --- a/src/ipc/handlers/app_handlers.ts +++ b/src/ipc/handlers/app_handlers.ts @@ -146,6 +146,7 @@ import { trackedBranchId, } from "../utils/neon_test_branch"; import type { AppSearchResult } from "@/lib/schemas"; +import { endTestsForApp } from "./tests_handlers"; import { getRgExecutablePath, @@ -491,6 +492,7 @@ async function deleteAppById( const { envRestored } = await endRecordingForApp(appId, "app-stopped", { skipRestart: true, }); + await endTestsForApp(appId); if (!envRestored) { // The app directory is about to be removed, so a stale `.env.local` // inside it goes with it — this is diagnosis, not a refusal. diff --git a/src/ipc/handlers/tests_handlers.test.ts b/src/ipc/handlers/tests_handlers.test.ts index 1a92143d14..54c7ed934a 100644 --- a/src/ipc/handlers/tests_handlers.test.ts +++ b/src/ipc/handlers/tests_handlers.test.ts @@ -74,6 +74,10 @@ vi.mock("../services/git_service", () => ({ const queueCloudSandboxSnapshotSyncMock = vi.hoisted(() => vi.fn()); const prepareIsolatedTestDatabaseMock = vi.hoisted(() => vi.fn()); +const ensurePlaywrightBootstrapMock = vi.hoisted(() => vi.fn()); +const createE2eTestWorkspaceMock = vi.hoisted(() => vi.fn()); +const startE2eTestRuntimeMock = vi.hoisted(() => vi.fn()); +const spawnStreamingMock = vi.hoisted(() => vi.fn()); const broadcastToRegisteredWindowsMock = vi.hoisted(() => vi.fn()); // Partially mocked: this module is pulled in transitively by the runtime // service, so replacing it wholesale breaks whenever an unrelated export is @@ -94,6 +98,39 @@ vi.mock("../services/isolated_test_db", async (importOriginal) => { prepareIsolatedTestDatabase: prepareIsolatedTestDatabaseMock, }; }); +vi.mock("../utils/playwright_bootstrap", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + ensurePlaywrightBootstrap: ensurePlaywrightBootstrapMock, + }; +}); +vi.mock("../utils/socket_firewall", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + getPackageManagerCommandEnv: vi.fn( + (env: NodeJS.ProcessEnv = process.env) => env, + ), + }; +}); +vi.mock("../services/e2e_test_workspace", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + createE2eTestWorkspace: createE2eTestWorkspaceMock, + retainE2eTestArtifacts: vi.fn(), + }; +}); +vi.mock("../services/e2e_test_runtime", () => ({ + startE2eTestRuntime: startE2eTestRuntimeMock, +})); +vi.mock("../utils/spawn_streaming", () => ({ + spawnStreaming: spawnStreamingMock, +})); vi.mock("@/ipc/utils/window_broadcast", async (importOriginal) => { const actual = await importOriginal(); @@ -117,6 +154,30 @@ describe("tests handlers", () => { removeFileAndCommitMock.mockClear(); queueCloudSandboxSnapshotSyncMock.mockClear(); prepareIsolatedTestDatabaseMock.mockReset(); + ensurePlaywrightBootstrapMock.mockReset(); + ensurePlaywrightBootstrapMock.mockResolvedValue({ installed: false }); + createE2eTestWorkspaceMock.mockReset(); + createE2eTestWorkspaceMock.mockImplementation( + async ({ appPath }: { appPath: string }) => ({ + workspacePath: appPath, + artifactPath: path.join(TEMP_BASE, "artifacts"), + dispose: vi.fn(), + }), + ); + startE2eTestRuntimeMock.mockReset(); + startE2eTestRuntimeMock.mockResolvedValue({ + baseUrl: "http://127.0.0.1:49999", + process: null, + stop: vi.fn(), + }); + spawnStreamingMock.mockReset(); + spawnStreamingMock.mockResolvedValue({ + code: 1, + stdout: "", + stderr: "no report", + aborted: false, + timedOut: false, + }); broadcastToRegisteredWindowsMock.mockClear(); harness = setupHandlerTestHarness(); registerTestsHandlers(); @@ -142,7 +203,7 @@ describe("tests handlers", () => { } describe("tests:run", () => { - it("owns the working tree without excluding Git ref snapshots", async () => { + it("releases the working tree after snapshotting the sandbox", async () => { const appId = seedApp("app"); harness.db .update(apps) @@ -180,15 +241,23 @@ describe("tests handlers", () => { ).toMatchObject({ resources: [ { resource: "app-path", mode: "read" }, - { resource: "repository-ref", mode: "read" }, - "repository-worktree", "provider", - "runtime", - "runtime-config", "test-files", ], allowCompatibleQueueBypass: true, }); + expect( + requests.find( + ({ operation }) => operation === "prepare-e2e-test-workspace", + ), + ).toMatchObject({ + resources: [ + { resource: "app-path", mode: "read" }, + { resource: "repository-ref", mode: "read" }, + "repository-worktree", + "test-files", + ], + }); }); it("refuses atomically when a recording starts at coordinator admission", async () => { @@ -229,7 +298,7 @@ describe("tests handlers", () => { } }); - it("reports an unrestored test-run environment", async () => { + it("reports an unfinished isolated-database cleanup", async () => { const appId = seedApp("app"); harness.db .update(apps) @@ -250,7 +319,96 @@ describe("tests handlers", () => { source: "panel", }); - expect(result.infraError?.message).toMatch(/real database settings/i); + expect(result.infraError?.message).toMatch(/isolated test database/i); + expect(result.infraError?.message).toMatch(/settings were not changed/i); + }); + + it("authorizes the isolated server origin before Playwright starts", async () => { + const appId = seedApp("app"); + harness.db + .update(apps) + .set({ testingEnabled: true }) + .where(eq(apps.id, appId)) + .run(); + const events: string[] = []; + const authorizeRuntimeOrigin = vi.fn(async () => { + events.push("authorize"); + }); + prepareIsolatedTestDatabaseMock.mockResolvedValue({ + isolation: { mode: "neon-branch" }, + authorizeRuntimeOrigin, + teardown: vi.fn().mockResolvedValue({ envRestored: true }), + }); + startE2eTestRuntimeMock.mockImplementation(async () => { + events.push("server"); + return { + baseUrl: "http://127.0.0.1:49999/path", + process: null, + stop: vi.fn(), + }; + }); + spawnStreamingMock.mockImplementation(async () => { + events.push("playwright"); + return { + code: 1, + stdout: "", + stderr: "no report", + aborted: false, + timedOut: false, + }; + }); + + await runAppTestsWithIsolation({ + event: { sender: {} } as any, + appId, + source: "panel", + }); + + expect(authorizeRuntimeOrigin).toHaveBeenCalledWith( + "http://127.0.0.1:49999", + ); + expect(events).toEqual(["server", "authorize", "playwright"]); + }); + + it("stops and cleans up when Neon origin authorization fails", async () => { + const appId = seedApp("app"); + harness.db + .update(apps) + .set({ testingEnabled: true }) + .where(eq(apps.id, appId)) + .run(); + const stop = vi.fn().mockResolvedValue(undefined); + const teardown = vi.fn().mockResolvedValue({ envRestored: true }); + const dispose = vi.fn().mockResolvedValue(undefined); + createE2eTestWorkspaceMock.mockResolvedValue({ + workspacePath: path.join(TEMP_BASE, "app"), + artifactPath: path.join(TEMP_BASE, "artifacts"), + dispose, + }); + prepareIsolatedTestDatabaseMock.mockResolvedValue({ + isolation: { mode: "neon-branch" }, + authorizeRuntimeOrigin: vi + .fn() + .mockRejectedValue(new Error("Neon unavailable")), + teardown, + }); + startE2eTestRuntimeMock.mockResolvedValue({ + baseUrl: "http://127.0.0.1:49999", + process: null, + stop, + }); + + const result = await runAppTestsWithIsolation({ + event: { sender: {} } as any, + appId, + source: "panel", + }); + + expect(result.infraError?.message).toMatch(/Neon Auth/i); + expect(spawnStreamingMock).not.toHaveBeenCalled(); + expect(stop).toHaveBeenCalledOnce(); + expect(teardown).toHaveBeenCalledOnce(); + expect(dispose).toHaveBeenCalledOnce(); }); }); diff --git a/src/ipc/handlers/tests_handlers.ts b/src/ipc/handlers/tests_handlers.ts index 0a4ca04e54..41a320e9ff 100644 --- a/src/ipc/handlers/tests_handlers.ts +++ b/src/ipc/handlers/tests_handlers.ts @@ -53,10 +53,18 @@ import { parseTestCases } from "../utils/parse_test_cases"; import { getPackageManagerCommandEnv } from "../utils/socket_firewall"; import { queueCloudSandboxSnapshotSync } from "../utils/cloud_sandbox_provider"; import { sendTelemetryEvent } from "../utils/telemetry"; +import type { PreparedIsolation } from "../services/isolated_test_db"; +import { prepareE2eTestDataIsolation } from "../services/e2e_test_data_isolation"; import { - prepareIsolatedTestDatabase, - type PreparedIsolation, -} from "../services/isolated_test_db"; + createE2eTestWorkspace, + retainE2eTestArtifacts, + rewriteE2eArtifactPath, + type E2eTestWorkspace, +} from "../services/e2e_test_workspace"; +import { + startE2eTestRuntime, + type E2eTestRuntime, +} from "../services/e2e_test_runtime"; import { readTestScreenshotDataUrl } from "../utils/test_screenshot"; import { isRecordingActive } from "../services/recording_registry"; import { readSettings } from "@/main/settings"; @@ -95,6 +103,29 @@ function isNoTestsFoundOutput(output: string): boolean { return /\bno tests found\b/i.test(output); } +function rewriteResultArtifactPaths( + results: TestResult[], + workspacePath: string, + artifactPath: string, +): TestResult[] { + return results.map((result) => ({ + ...result, + screenshotPath: rewriteE2eArtifactPath( + result.screenshotPath, + workspacePath, + artifactPath, + ), + tests: result.tests?.map((test) => ({ + ...test, + screenshotPath: rewriteE2eArtifactPath( + test.screenshotPath, + workspacePath, + artifactPath, + ), + })), + })); +} + /** * The relative paths of every spec under the app's `e2e-tests/` folder, sorted. * Shared by the Tests panel listing and the agent's run_tests tool (so a @@ -167,6 +198,20 @@ export function isTestRunActive(appId: number): boolean { return testRunControllers.has(appId); } +/** Abort every sandbox/test runner during Electron's synchronous quit phase. */ +export function stopAllAppTestsSync(): void { + for (const run of testRunControllers.values()) { + run.controller.abort(); + } +} + +export async function endTestsForApp(appId: number): Promise { + const run = testRunControllers.get(appId); + if (!run) return; + run.controller.abort(); + await run.done; +} + async function getApp(appId: number) { const app = await db.query.apps.findFirst({ where: eq(apps.id, appId), @@ -209,6 +254,12 @@ function emitRunState( export interface RunAppTestsCoreOptions { appId: number; + /** Explicit execution directory for an isolated test workspace. */ + appPath?: string; + /** Explicit test-server URL. Legacy callers use the normal preview URL. */ + baseUrl?: string; + /** Bootstrap is performed against the real app before sandbox creation. */ + skipBootstrap?: boolean; /** When set, runs a single spec file (relative path); otherwise runs all. */ testFile?: string; /** @@ -253,12 +304,14 @@ export interface RunAppTestsCoreOptions { } /** - * Bootstrap Playwright (if needed), run the tests against the running dev - * server's proxy URL, and parse the JSON report. Backs the `tests:run` IPC - * handler (the UI "Run" button). + * Bootstrap Playwright when requested, run against an explicit sandbox server + * (or the legacy preview URL for direct callers), and parse the JSON report. */ export async function runAppTestsCore({ appId, + appPath: explicitAppPath, + baseUrl: explicitBaseUrl, + skipBootstrap = false, testFile, testLine, grep, @@ -270,7 +323,7 @@ export async function runAppTestsCore({ testEnv, }: RunAppTestsCoreOptions): Promise { const app = await getApp(appId); - const appPath = getDyadAppPath(app.path); + const appPath = explicitAppPath ?? getDyadAppPath(app.path); const emit = (chunk: string, phase: "setup" | "running") => onOutput?.(chunk, phase); const normalizedTestFile = @@ -287,7 +340,7 @@ export async function runAppTestsCore({ } // Gate: the dev server must be running so baseURL resolves. - const baseUrl = getRunningTestBaseUrl(appId); + const baseUrl = explicitBaseUrl ?? getRunningTestBaseUrl(appId); if (!baseUrl) { return { appId, @@ -301,17 +354,19 @@ export async function runAppTestsCore({ // 1. Lazy bootstrap (install Playwright + browser, write config), streamed. let installed = false; - try { - const result = await ensurePlaywrightBootstrap({ - appPath, - signal, - onOutput: (chunk) => emit(chunk, "setup"), - }); - installed = result.installed; - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - logger.error(`Playwright bootstrap failed: ${message}`); - return { appId, results: [], infraError: { message } }; + if (!skipBootstrap) { + try { + const result = await ensurePlaywrightBootstrap({ + appPath, + signal, + onOutput: (chunk) => emit(chunk, "setup"), + }); + installed = result.installed; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + logger.error(`Playwright bootstrap failed: ${message}`); + return { appId, results: [], infraError: { message } }; + } } if (signal?.aborted) { @@ -627,9 +682,8 @@ export async function runAppTestsWithIsolation({ testFile: normalizedTestFile ?? undefined, testLine, grep, - // Only `cleaning-up` carries this, and only so the UI can name the work - // accurately: the Neon path restarts the preview, the Supabase path - // touches nothing the user can see. + // Only `cleaning-up` carries this so the UI can name the remote provider + // cleanup accurately. The normal preview is not restarted. isolation, }); }; @@ -679,23 +733,20 @@ export async function runAppTestsWithIsolation({ emitOutput(event, appId, runId, chunk, phase); let finalResult: RunAppTestsResult = { appId, results: [] }; - // Set by isolation teardown below when `.env.local` couldn't be put back. The - // run may have produced perfectly good results, but the app is still pointed - // at the temporary branch, so the caller has to be told rather than left to - // relaunch it against isolated data. - let envRestoreFailed = false; + let workspace: E2eTestWorkspace | undefined; + // The real env is never changed. This only reports a sandbox/provider cleanup + // failure (for example, a temporary Neon branch left for startup recovery). + let isolationCleanupFailed = false; /** - * Fold a failed `.env.local` restore into a result. Applied on BOTH exits — - * an unexpected rejection inside the lock must not swallow it, or the run - * reports an ordinary infrastructure error while the app is still pointed at - * the temporary branch. + * Fold failed provider cleanup into a result. Applied on both exits so an + * unexpected rejection cannot hide a temporary branch left for recovery. */ - const withEnvRestoreWarning = ( + const withIsolationCleanupWarning = ( result: RunAppTestsResult, ): RunAppTestsResult => { - if (!envRestoreFailed) return result; + if (!isolationCleanupFailed) return result; const restoreMessage = - "Dyad couldn't restore your app's real database settings after the test run. Restore .env.local before running the app again."; + "Dyad couldn't finish cleaning up the isolated test database. Your app settings were not changed; Dyad will retry remote cleanup on next startup."; return { ...result, // Appended rather than substituted: an isolation-setup failure explains @@ -723,17 +774,45 @@ export async function runAppTestsWithIsolation({ // so this call exists only for the ordering barrier.) await getApp(appId); - // Own the runtime/test resources across the whole isolation lifecycle - // (prepare → run → teardown). Startup reconciliation owns the same - // resources, so a rapid Run after launch cannot interleave its env swap - // and dev-server restart with reconciliation and use the real database. + // Bootstrap and snapshot under the real working-tree claim, then release it + // before Playwright runs so ordinary app editing can continue against the + // normal preview while this run uses its captured filesystem state. + workspace = await appOperationCoordinator.run( + { + appId, + operation: "prepare-e2e-test-workspace", + resources: [ + readAppResource("app-path"), + readAppResource("repository-ref"), + "repository-worktree", + "test-files", + ], + refuseWhenRecording: "run tests", + }, + async () => { + const app = await getApp(appId); + const realAppPath = getDyadAppPath(app.path); + await ensurePlaywrightBootstrap({ + appPath: realAppPath, + signal: controller.signal, + onOutput: (chunk) => emit(chunk, "setup"), + }); + emit("Copying the app into an isolated test workspace…\n", "setup"); + return createE2eTestWorkspace({ + appId, + appPath: realAppPath, + signal: controller.signal, + onProgress: (message) => emit(message, "setup"), + }); + }, + ); + + // The live test only owns provider/test inputs. It deliberately does not + // claim the normal runtime or runtime-config: its process is run-scoped and + // never registered in runningApps. const testRunResources = [ readAppResource("app-path"), - readAppResource("repository-ref"), - "repository-worktree", "provider", - "runtime", - "runtime-config", "test-files", ] as const; if (appOperationCoordinator.isBusy(appId, testRunResources)) { @@ -759,6 +838,7 @@ export async function runAppTestsWithIsolation({ }, async () => { let prepared: PreparedIsolation | undefined; + let testRuntime: E2eTestRuntime | undefined; try { const app = await getApp(appId); @@ -774,14 +854,27 @@ export async function runAppTestsWithIsolation({ } const runtimeMode = readSettings().runtimeMode2 ?? "host"; + if (runtimeMode !== "host") { + return { + appId, + results: [], + infraError: { + message: `Isolated E2E test servers currently require host runtime. Switch from ${runtimeMode} runtime before running tests.`, + }, + isolation: { + mode: "none" as const, + reason: `Sandboxed E2E execution is not available in ${runtimeMode} runtime yet.`, + }, + }; + } // Set up isolation so the run never mutates the user's real data: // Neon apps get a throwaway copy-on-write branch, Supabase apps get // a throwaway RLS-scoped test user, and no-DB apps run as-is. - prepared = await prepareIsolatedTestDatabase({ + prepared = await prepareE2eTestDataIsolation({ app, + workspacePath: workspace!.workspacePath, emit, - runtimeMode, signal: controller.signal, }); @@ -796,8 +889,43 @@ export async function runAppTestsWithIsolation({ }; } + emit("Starting the isolated test server…\n", "setup"); + testRuntime = await startE2eTestRuntime({ + workspacePath: workspace!.workspacePath, + startCommand: app.startCommand, + signal: controller.signal, + onOutput: (chunk) => emit(chunk, "setup"), + }); + + if (prepared.authorizeRuntimeOrigin) { + const runtimeOrigin = new URL(testRuntime.baseUrl).origin; + emit( + "Authorizing the isolated test server for sign-in…\n", + "setup", + ); + try { + await prepared.authorizeRuntimeOrigin(runtimeOrigin); + } catch (error) { + logger.error( + `Failed to authorize isolated E2E origin ${runtimeOrigin} for app ${appId}: ${error}`, + ); + return { + appId, + results: [], + infraError: { + message: + "Dyad couldn't authorize the isolated test server with Neon Auth, so the tests were not run. Check your Neon connection and try again.", + }, + isolation: prepared.isolation, + }; + } + } + const result = await runAppTestsCore({ appId, + appPath: workspace!.workspacePath, + baseUrl: testRuntime.baseUrl, + skipBootstrap: true, testFile: normalizedTestFile ?? undefined, testLine, grep, @@ -808,29 +936,37 @@ export async function runAppTestsWithIsolation({ onOutput: emit, testEnv: prepared.testCredentials, }); + await retainE2eTestArtifacts(workspace!); + result.results = rewriteResultArtifactPaths( + result.results, + workspace!.workspacePath, + workspace!.artifactPath, + ); return { ...result, isolation: prepared.isolation }; } finally { - // Always restore the app to its real database, even on the - // infraError early-return, abort, or throw. `teardown` is safe to - // call exactly once; on the infraError path it's a NOOP (isolation - // already restored). + if (testRuntime) { + try { + await testRuntime.stop(); + } catch (error) { + logger.error( + `Failed to stop isolated test server for app ${appId}: ${error}`, + ); + } + } + // Always clean up provider isolation, even on an infraError, abort, or + // throw. The sandbox env can be discarded, but remote branches/users + // still require their guaranteed teardown. if (prepared) { try { // Announce the teardown before it starts. It restores - // `.env.local`, restarts the dev server and deletes the temporary - // branch/user, takes no AbortSignal, and routinely outlasts the - // process kill by a wide margin (the Neon branch delete retries - // with backoff). Without this the UI reports "running" for the - // whole wait. Skipped for `none`, whose teardown is a NOOP that - // would only flash the label. + // the temporary branch/user, takes no AbortSignal, and may + // outlast the process kill because Neon deletion retries with + // backoff. Skipped for `none`, whose teardown is a NOOP. if (prepared.isolation.mode !== "none") { emitProgress("cleaning-up", prepared.isolation); } - // Fail closed across the await: a teardown that throws has said - // nothing about whether the env came back, and "unknown" has to - // read the same as "no". - envRestoreFailed = true; - envRestoreFailed = !(await prepared.teardown()).envRestored; + isolationCleanupFailed = true; + isolationCleanupFailed = !(await prepared.teardown()).envRestored; } catch (error) { logger.error( `Failed to tear down isolated test environment for app ${appId}: ${error}`, @@ -840,12 +976,12 @@ export async function runAppTestsWithIsolation({ } }, ); - finalResult = withEnvRestoreWarning(finalResult); + finalResult = withIsolationCleanupWarning(finalResult); return finalResult; } catch (error) { // Surface an unexpected failure as an infra error on the run-state event so // the panel leaves its spinner state, then rethrow for the caller. - finalResult = withEnvRestoreWarning({ + finalResult = withIsolationCleanupWarning({ appId, results: [], infraError: { @@ -861,6 +997,15 @@ export async function runAppTestsWithIsolation({ cause: error, }); } finally { + if (workspace) { + try { + await workspace.dispose(); + } catch (error) { + logger.error( + `Failed to remove isolated test workspace for app ${appId}: ${error}`, + ); + } + } if (externalSignal) { externalSignal.removeEventListener("abort", onExternalAbort); } @@ -939,7 +1084,11 @@ export function registerTestsHandlers() { const app = await getApp(params.appId); const appPath = getDyadAppPath(app.path); return { - dataUrl: await readTestScreenshotDataUrl(appPath, params.path), + dataUrl: await readTestScreenshotDataUrl( + appPath, + params.path, + params.appId, + ), }; }, ); diff --git a/src/ipc/services/e2e_test_data_isolation.ts b/src/ipc/services/e2e_test_data_isolation.ts new file mode 100644 index 0000000000..b27b0c5126 --- /dev/null +++ b/src/ipc/services/e2e_test_data_isolation.ts @@ -0,0 +1,33 @@ +import { apps } from "@/db/schema"; +import { + prepareIsolatedTestDatabase, + type PreparedIsolation, +} from "./isolated_test_db"; + +type AppRow = typeof apps.$inferSelect; + +/** + * E2E-only adapter for provider isolation. Unlike the recorder-facing default, + * this writes only inside the disposable workspace and never restarts the + * normal preview. + */ +export function prepareE2eTestDataIsolation({ + app, + workspacePath, + emit, + signal, +}: { + app: AppRow; + workspacePath: string; + emit: (chunk: string, phase: "setup" | "running") => void; + signal?: AbortSignal; +}): Promise { + return prepareIsolatedTestDatabase({ + app, + emit, + runtimeMode: "host", + signal, + appPathOverride: workspacePath, + restartApp: false, + }); +} diff --git a/src/ipc/services/e2e_test_runtime.test.ts b/src/ipc/services/e2e_test_runtime.test.ts new file mode 100644 index 0000000000..34089c0479 --- /dev/null +++ b/src/ipc/services/e2e_test_runtime.test.ts @@ -0,0 +1,75 @@ +// @vitest-environment node + +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; + +import { + buildE2eTestStartCommand, + startE2eTestRuntime, +} from "./e2e_test_runtime"; +import { runningApps } from "@/ipc/utils/process_manager"; + +describe("buildE2eTestStartCommand", () => { + it("starts npm without reinstalling dependencies", () => { + const command = buildE2eTestStartCommand({ + workspacePath: path.resolve("app"), + port: 45678, + }); + expect(command.command).toBe("npm run dev -- --port 45678"); + expect(command.command).not.toContain("install"); + expect(command.env.PORT).toBe("45678"); + }); + + it("supports an explicit port placeholder in custom commands", () => { + const command = buildE2eTestStartCommand({ + workspacePath: path.resolve("app"), + port: 45678, + startCommand: "custom-server --listen {port}", + }); + expect(command.command).toBe("custom-server --listen 45678"); + }); + + it("uses pnpm when the sandbox contains its lockfile", () => { + const root = fs.mkdtempSync(path.join(process.cwd(), ".e2e-runtime-test-")); + try { + fs.writeFileSync(path.join(root, "pnpm-lock.yaml"), ""); + const command = buildE2eTestStartCommand({ + workspacePath: root, + port: 45678, + }); + expect(command.command).toContain("pnpm"); + expect(command.command).not.toContain("install"); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it("starts and stops a server without registering the normal app runtime", async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "dyad-e2e-runtime-")); + fs.writeFileSync( + path.join(root, "server.mjs"), + `import http from "node:http"; +const port = Number(process.argv[2]); +http.createServer((_request, response) => response.end("sandbox")) + .listen(port, "127.0.0.1"); +`, + ); + let runtime: Awaited> | undefined; + const registeredRuntimeCount = runningApps.size; + try { + runtime = await startE2eTestRuntime({ + workspacePath: root, + startCommand: `"${process.execPath}" server.mjs {port}`, + }); + await expect( + fetch(runtime.baseUrl).then((response) => response.text()), + ).resolves.toBe("sandbox"); + expect(runningApps.size).toBe(registeredRuntimeCount); + } finally { + await runtime?.stop(); + fs.rmSync(root, { recursive: true, force: true }); + } + }); +}); diff --git a/src/ipc/services/e2e_test_runtime.ts b/src/ipc/services/e2e_test_runtime.ts new file mode 100644 index 0000000000..61d3446b5a --- /dev/null +++ b/src/ipc/services/e2e_test_runtime.ts @@ -0,0 +1,223 @@ +import { spawn, type ChildProcess } from "node:child_process"; +import fs from "node:fs"; +import net from "node:net"; +import path from "node:path"; +import log from "electron-log"; + +import { killProcess } from "@/ipc/utils/process_manager"; +import { + getPackageManagerCommandEnv, + PNPM_PM_ON_FAIL_IGNORE_ARG, +} from "@/ipc/utils/socket_firewall"; + +const logger = log.scope("e2e_test_runtime"); +const SERVER_READY_TIMEOUT_MS = 120_000; +const SERVER_READY_POLL_MS = 250; + +export interface E2eTestRuntime { + baseUrl: string; + process: ChildProcess; + stop(): Promise; +} + +export async function allocateE2eTestPort(): Promise { + return new Promise((resolve, reject) => { + const server = net.createServer(); + server.unref(); + server.once("error", reject); + server.listen(0, "127.0.0.1", () => { + const address = server.address(); + if (!address || typeof address === "string") { + server.close(() => + reject(new Error("Could not allocate a test port.")), + ); + return; + } + server.close((error) => (error ? reject(error) : resolve(address.port))); + }); + }); +} + +export function buildE2eTestStartCommand({ + workspacePath, + port, + startCommand, +}: { + workspacePath: string; + port: number; + startCommand?: string | null; +}): { command: string; env: NodeJS.ProcessEnv } { + if (startCommand?.trim()) { + const command = startCommand.includes("{port}") + ? startCommand.replaceAll("{port}", String(port)) + : `${startCommand.trim()} -- --port ${port}`; + return { command, env: { ...process.env, PORT: String(port) } }; + } + + if (fs.existsSync(path.join(workspacePath, "pnpm-lock.yaml"))) { + return { + command: `pnpm ${PNPM_PM_ON_FAIL_IGNORE_ARG} run dev --port ${port}`, + env: { ...getPackageManagerCommandEnv(), PORT: String(port) }, + }; + } + return { + command: `npm run dev -- --port ${port}`, + env: { ...process.env, PORT: String(port) }, + }; +} + +function delay(ms: number, signal?: AbortSignal): Promise { + return new Promise((resolve, reject) => { + if (signal?.aborted) { + reject(new Error("Test run stopped.")); + return; + } + const timer = setTimeout(resolve, ms); + signal?.addEventListener( + "abort", + () => { + clearTimeout(timer); + reject(new Error("Test run stopped.")); + }, + { once: true }, + ); + }); +} + +async function waitForReady({ + baseUrl, + process: child, + signal, + outputTail, + spawnError, +}: { + baseUrl: string; + process: ChildProcess; + signal?: AbortSignal; + outputTail: () => string; + spawnError: () => Error | undefined; +}): Promise { + const deadline = Date.now() + SERVER_READY_TIMEOUT_MS; + while (Date.now() < deadline) { + if (signal?.aborted) throw new Error("Test run stopped."); + const startError = spawnError(); + if (startError) { + throw new Error( + `Could not start the isolated test server: ${startError.message}`, + ); + } + if (child.exitCode !== null || child.signalCode !== null) { + throw new Error( + `The isolated test server exited before becoming ready.\n${outputTail()}`, + ); + } + try { + const response = await fetch(baseUrl, { + signal: AbortSignal.timeout(1_000), + }); + if (response.status < 500) return; + } catch { + // The server has not bound yet. + } + await delay(SERVER_READY_POLL_MS, signal); + } + throw new Error( + `The isolated test server did not become ready within 2 minutes.\n${outputTail()}`, + ); +} + +async function startE2eTestRuntimeOnce({ + workspacePath, + startCommand, + signal, + onOutput, +}: { + workspacePath: string; + startCommand?: string | null; + signal?: AbortSignal; + onOutput?: (chunk: string) => void; +}): Promise { + if (signal?.aborted) throw new Error("Test run stopped."); + const port = await allocateE2eTestPort(); + const baseUrl = `http://127.0.0.1:${port}`; + const { command, env } = buildE2eTestStartCommand({ + workspacePath, + port, + startCommand, + }); + const child = spawn(command, [], { + cwd: workspacePath, + env, + shell: true, + stdio: "pipe", + detached: false, + }); + + let tail = ""; + const append = (data: unknown) => { + const chunk = String(data); + tail = `${tail}${chunk}`.slice(-8_000); + onOutput?.(`[test server] ${chunk}`); + }; + child.stdout?.on("data", append); + child.stderr?.on("data", append); + let startError: Error | undefined; + child.once("error", (error) => { + startError = error; + append(error.message); + }); + + let stopPromise: Promise | undefined; + const stop = () => { + stopPromise ??= (async () => { + if (child.pid && child.exitCode === null && child.signalCode === null) { + await killProcess(child); + } + })(); + return stopPromise; + }; + const onAbort = () => void stop(); + signal?.addEventListener("abort", onAbort, { once: true }); + + try { + await waitForReady({ + baseUrl, + process: child, + signal, + outputTail: () => tail, + spawnError: () => startError, + }); + logger.info(`Isolated E2E server ready on port ${port}`); + return { + baseUrl, + process: child, + stop: async () => { + signal?.removeEventListener("abort", onAbort); + await stop(); + }, + }; + } catch (error) { + signal?.removeEventListener("abort", onAbort); + await stop(); + throw error; + } +} + +export async function startE2eTestRuntime( + options: Parameters[0], +): Promise { + let lastError: unknown; + for (let attempt = 0; attempt < 3; attempt += 1) { + try { + return await startE2eTestRuntimeOnce(options); + } catch (error) { + lastError = error; + const message = error instanceof Error ? error.message : String(error); + if (!/EADDRINUSE|address already in use/i.test(message)) throw error; + options.onOutput?.( + "[test server] The selected port was taken; retrying with another port…\n", + ); + } + } + throw lastError; +} diff --git a/src/ipc/services/e2e_test_workspace.test.ts b/src/ipc/services/e2e_test_workspace.test.ts new file mode 100644 index 0000000000..0626becee8 --- /dev/null +++ b/src/ipc/services/e2e_test_workspace.test.ts @@ -0,0 +1,156 @@ +import { promises as fs } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@/paths/paths", () => ({ getUserDataPath: vi.fn() })); + +import { getUserDataPath } from "@/paths/paths"; +import { + createE2eTestWorkspace, + retainE2eTestArtifacts, + rewriteE2eArtifactPath, + shouldCopyE2eWorkspacePath, +} from "./e2e_test_workspace"; + +const roots: string[] = []; + +afterEach(async () => { + await Promise.all( + roots + .splice(0) + .map((root) => fs.rm(root, { recursive: true, force: true })), + ); +}); + +async function tempRoot() { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "dyad-e2e-workspace-")); + roots.push(root); + return root; +} + +describe("E2E test workspace", () => { + it("copies current source while excluding heavyweight roots", async () => { + const root = await tempRoot(); + const appPath = path.join(root, "app"); + const userData = path.join(root, "user-data"); + vi.mocked(getUserDataPath).mockReturnValue(userData); + await fs.mkdir(path.join(appPath, "src"), { recursive: true }); + await fs.mkdir(path.join(appPath, "node_modules", "pkg"), { + recursive: true, + }); + await fs.mkdir(path.join(appPath, ".git"), { recursive: true }); + await fs.writeFile(path.join(appPath, "src", "new.ts"), "uncommitted"); + await fs.writeFile(path.join(appPath, ".env.local"), "REAL=1\n"); + await fs.writeFile(path.join(appPath, "node_modules", "pkg", "x"), "x"); + + const workspace = await createE2eTestWorkspace({ appId: 7, appPath }); + expect( + await fs.readFile( + path.join(workspace.workspacePath, "src", "new.ts"), + "utf8", + ), + ).toBe("uncommitted"); + expect( + await fs.readFile( + path.join(workspace.workspacePath, ".env.local"), + "utf8", + ), + ).toBe("REAL=1\n"); + await fs.writeFile( + path.join(workspace.workspacePath, "src", "new.ts"), + "sandbox-only", + ); + expect(await fs.readFile(path.join(appPath, "src", "new.ts"), "utf8")).toBe( + "uncommitted", + ); + const nodeModulesStat = await fs.lstat( + path.join(workspace.workspacePath, "node_modules"), + ); + expect(nodeModulesStat.isDirectory()).toBe(true); + expect(nodeModulesStat.isSymbolicLink()).toBe(false); + await expect( + fs.stat(path.join(workspace.workspacePath, ".git")), + ).rejects.toThrow(); + + await workspace.dispose(); + await expect(fs.stat(workspace.workspacePath)).rejects.toThrow(); + }); + + it.runIf(process.platform !== "win32")( + "keeps pnpm dependency realpaths inside the sandbox", + async () => { + const root = await tempRoot(); + const appPath = path.join(root, "app"); + vi.mocked(getUserDataPath).mockReturnValue(path.join(root, "user-data")); + const packageStore = path.join( + appPath, + "node_modules", + ".pnpm", + "nitro@3", + "node_modules", + "nitro", + ); + await fs.mkdir(packageStore, { recursive: true }); + await fs.writeFile(path.join(packageStore, "package.json"), "{}"); + await fs.symlink( + path.join(".pnpm", "nitro@3", "node_modules", "nitro"), + path.join(appPath, "node_modules", "nitro"), + "dir", + ); + + const workspace = await createE2eTestWorkspace({ appId: 8, appPath }); + const sandboxNodeModules = path.join( + workspace.workspacePath, + "node_modules", + ); + const nitroRealpath = await fs.realpath( + path.join(sandboxNodeModules, "nitro"), + ); + + expect(path.relative(sandboxNodeModules, nitroRealpath)).not.toMatch( + /^\.\./, + ); + expect(nitroRealpath).not.toContain(path.join(appPath, "node_modules")); + }, + ); + + it("retains and rewrites screenshot artifacts before disposal", async () => { + const root = await tempRoot(); + const workspacePath = path.join(root, "workspace"); + const artifactPath = path.join(root, "artifacts"); + const screenshot = path.join(workspacePath, "test-results", "shot.png"); + await fs.mkdir(path.dirname(screenshot), { recursive: true }); + await fs.writeFile(screenshot, "png"); + + await retainE2eTestArtifacts({ workspacePath, artifactPath }); + expect( + rewriteE2eArtifactPath(screenshot, workspacePath, artifactPath), + ).toBe(path.join(artifactPath, "test-results", "shot.png")); + expect( + await fs.readFile( + path.join(artifactPath, "test-results", "shot.png"), + "utf8", + ), + ).toBe("png"); + }); + + it("uses a root-based exclusion policy", () => { + const appPath = path.resolve("app"); + expect( + shouldCopyE2eWorkspacePath(appPath, path.join(appPath, "src", "a.ts")), + ).toBe(true); + expect( + shouldCopyE2eWorkspacePath( + appPath, + path.join(appPath, "node_modules", "x"), + ), + ).toBe(false); + expect( + shouldCopyE2eWorkspacePath( + appPath, + path.join(appPath, "test-results", "x"), + ), + ).toBe(false); + }); +}); diff --git a/src/ipc/services/e2e_test_workspace.ts b/src/ipc/services/e2e_test_workspace.ts new file mode 100644 index 0000000000..1045f98abc --- /dev/null +++ b/src/ipc/services/e2e_test_workspace.ts @@ -0,0 +1,192 @@ +import { constants as fsConstants, promises as fs } from "node:fs"; +import path from "node:path"; +import { randomUUID } from "node:crypto"; +import log from "electron-log"; + +import { getUserDataPath } from "@/paths/paths"; + +const logger = log.scope("e2e_test_workspace"); + +const EXCLUDED_ROOTS = new Set([ + ".git", + "node_modules", + "dist", + "build", + "out", + ".vite", + ".next", + ".nuxt", + ".svelte-kit", + ".turbo", + ".cache", + "test-results", + "playwright-report", + "coverage", +]); + +export const E2E_TEST_SANDBOX_DIR = "test-sandboxes"; +export const E2E_TEST_ARTIFACT_DIR = "test-artifacts"; + +export interface E2eTestWorkspace { + workspacePath: string; + artifactPath: string; + dispose(): Promise; +} + +export function shouldCopyE2eWorkspacePath( + appPath: string, + candidatePath: string, +): boolean { + const relative = path.relative(appPath, candidatePath); + if (!relative) return true; + const [root] = relative.split(path.sep); + return root !== ".DS_Store" && !EXCLUDED_ROOTS.has(root); +} + +function assertOwnedPath(root: string, candidate: string): void { + const relative = path.relative(path.resolve(root), path.resolve(candidate)); + if (!relative || relative.startsWith("..") || path.isAbsolute(relative)) { + throw new Error(`Refusing to remove path outside the E2E workspace root.`); + } +} + +async function copyNodeModules( + appPath: string, + workspacePath: string, + signal?: AbortSignal, +) { + const source = path.join(appPath, "node_modules"); + try { + const stat = await fs.stat(source); + if (!stat.isDirectory()) throw new Error("not a directory"); + } catch { + throw new Error( + "The app's dependencies are not installed. Start the app successfully before running tests.", + ); + } + + // Do not link the node_modules root to the real app. Vite resolves that root + // symlink before applying its filesystem allowlist; Nitro's server entry then + // appears to live outside the sandbox and fails with ERR_LOAD_URL. A reflink + // keeps package files copy-on-write while preserving pnpm's *relative* links + // inside a sandbox-local dependency tree. + await fs.cp(source, path.join(workspacePath, "node_modules"), { + recursive: true, + verbatimSymlinks: true, + ...(process.platform === "win32" + ? {} + : { mode: fsConstants.COPYFILE_FICLONE }), + filter: () => !signal?.aborted, + }); + if (signal?.aborted) throw new Error("Test run stopped."); +} + +export async function createE2eTestWorkspace({ + appId, + appPath, + signal, + onProgress, +}: { + appId: number; + appPath: string; + signal?: AbortSignal; + onProgress?: (message: string) => void; +}): Promise { + if (signal?.aborted) throw new Error("Test run stopped."); + + const sandboxRoot = path.join(getUserDataPath(), E2E_TEST_SANDBOX_DIR); + const artifactRoot = path.join(getUserDataPath(), E2E_TEST_ARTIFACT_DIR); + await Promise.all([ + fs.mkdir(sandboxRoot, { recursive: true }), + fs.mkdir(artifactRoot, { recursive: true }), + ]); + const oldArtifacts = await fs.readdir(artifactRoot, { withFileTypes: true }); + await Promise.all( + oldArtifacts + .filter( + (entry) => entry.isDirectory() && entry.name.startsWith(`${appId}-`), + ) + .map((entry) => + fs.rm(path.join(artifactRoot, entry.name), { + recursive: true, + force: true, + }), + ), + ); + + const runName = `${appId}-${Date.now()}-${randomUUID()}`; + const workspacePath = path.join(sandboxRoot, runName); + const artifactPath = path.join(artifactRoot, runName); + assertOwnedPath(sandboxRoot, workspacePath); + assertOwnedPath(artifactRoot, artifactPath); + + let disposed = false; + const dispose = async () => { + if (disposed) return; + disposed = true; + assertOwnedPath(sandboxRoot, workspacePath); + await fs.rm(workspacePath, { recursive: true, force: true }); + }; + + try { + await fs.cp(appPath, workspacePath, { + recursive: true, + verbatimSymlinks: true, + ...(process.platform === "win32" + ? {} + : { mode: fsConstants.COPYFILE_FICLONE }), + filter: (candidatePath) => { + if (signal?.aborted) return false; + return shouldCopyE2eWorkspacePath(appPath, candidatePath); + }, + }); + if (signal?.aborted) throw new Error("Test run stopped."); + onProgress?.("Cloning installed dependencies into the test workspace…\n"); + await copyNodeModules(appPath, workspacePath, signal); + return { workspacePath, artifactPath, dispose }; + } catch (error) { + await dispose(); + throw error; + } +} + +export async function retainE2eTestArtifacts({ + workspacePath, + artifactPath, +}: Pick): Promise { + const source = path.join(workspacePath, "test-results"); + try { + if (!(await fs.stat(source)).isDirectory()) return; + } catch { + return; + } + await fs.rm(artifactPath, { recursive: true, force: true }); + await fs.mkdir(artifactPath, { recursive: true }); + await fs.cp(source, path.join(artifactPath, "test-results"), { + recursive: true, + verbatimSymlinks: false, + }); +} + +export function rewriteE2eArtifactPath( + screenshotPath: string | undefined, + workspacePath: string, + artifactPath: string, +): string | undefined { + if (!screenshotPath) return undefined; + const absolute = path.isAbsolute(screenshotPath) + ? path.resolve(screenshotPath) + : path.resolve(workspacePath, screenshotPath); + const relative = path.relative(workspacePath, absolute); + if (relative.startsWith("..") || path.isAbsolute(relative)) return undefined; + return path.join(artifactPath, relative); +} + +export async function reconcileOrphanE2eTestWorkspaces(): Promise { + const sandboxRoot = path.join(getUserDataPath(), E2E_TEST_SANDBOX_DIR); + try { + await fs.rm(sandboxRoot, { recursive: true, force: true }); + } catch (error) { + logger.warn(`Failed to remove abandoned E2E test workspaces: ${error}`); + } +} diff --git a/src/ipc/services/isolated_test_db.test.ts b/src/ipc/services/isolated_test_db.test.ts index 0df5d3b38e..a1de7a0419 100644 --- a/src/ipc/services/isolated_test_db.test.ts +++ b/src/ipc/services/isolated_test_db.test.ts @@ -2,12 +2,14 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; const mocks = vi.hoisted(() => ({ createTempTestBranch: vi.fn(), + markTestBranchCleanupOnly: vi.fn().mockResolvedValue(undefined), // The mark-then-delete tail lives in `neon_test_branch` and is tested there // (`markAndDeleteTempTestBranch`), including the ordering it depends on. What // teardown owes it is the branch it actually created — the app row it holds is // stale by this point — so that is what these tests pin. markAndDeleteTempTestBranch: vi.fn().mockResolvedValue(undefined), createNeonTestAccount: vi.fn(), + ensureNeonAuthTrustedOrigin: vi.fn().mockResolvedValue(null), createTempTestUser: vi.fn(), deleteTempTestUser: vi.fn().mockResolvedValue(undefined), checkRls: vi.fn().mockResolvedValue({ tablesWithoutRls: [] }), @@ -44,11 +46,16 @@ const mocks = vi.hoisted(() => ({ vi.mock("../utils/neon_test_branch", () => ({ createTempTestBranch: mocks.createTempTestBranch, + markTestBranchCleanupOnly: mocks.markTestBranchCleanupOnly, markAndDeleteTempTestBranch: mocks.markAndDeleteTempTestBranch, })); vi.mock("../utils/neon_test_account", () => ({ createNeonTestAccount: mocks.createNeonTestAccount, })); +vi.mock("../utils/neon_utils", async (importOriginal) => ({ + ...(await importOriginal()), + ensureNeonAuthTrustedOrigin: mocks.ensureNeonAuthTrustedOrigin, +})); vi.mock("../../supabase_admin/supabase_context", () => ({ getPublishableKey: mocks.getPublishableKey, })); @@ -206,6 +213,7 @@ describe("prepareIsolatedTestDatabase — Supabase test-user path", () => { DYAD_TEST_SUPABASE_URL: "https://sb-1.supabase.co", }); expect(prepared.infraError).toBeUndefined(); + expect(prepared.authorizeRuntimeOrigin).toBeUndefined(); await prepared.teardown(); expect(mocks.deleteTempTestUser).toHaveBeenCalledWith( @@ -266,6 +274,7 @@ describe("prepareIsolatedTestDatabase — non-Neon paths", () => { }); expect(prepared.isolation).toEqual({ mode: "none" }); expect(prepared.infraError).toBeUndefined(); + expect(prepared.authorizeRuntimeOrigin).toBeUndefined(); }); it("discloses for non-host runtimes on a Neon app (no branch created)", async () => { @@ -281,6 +290,40 @@ describe("prepareIsolatedTestDatabase — non-Neon paths", () => { }); describe("prepareIsolatedTestDatabase — Neon happy path", () => { + it("targets a sandbox without restarting or marking the real env as swapped", async () => { + mocks.createTempTestBranch.mockResolvedValue({ + branchId: "test-br", + databaseUrl: "postgres://temp", + neonAuthBaseUrl: "https://auth", + }); + + const prepared = await prepareIsolatedTestDatabase({ + app: makeApp({ neonProjectId: "proj-1" }), + emit, + runtimeMode: "host", + appPathOverride: "/sandboxes/run-1", + restartApp: false, + }); + + expect(prepared.infraError).toBeUndefined(); + expect(mocks.updateNeonEnvVars).toHaveBeenCalledWith( + expect.objectContaining({ + appPath: "/sandboxes/run-1", + connectionUri: "postgres://temp", + }), + ); + expect(mocks.markTestBranchCleanupOnly).toHaveBeenCalledWith( + expect.objectContaining({ id: 1 }), + "test-br", + ); + expect(mocks.executeApp).not.toHaveBeenCalled(); + await prepared.teardown(); + expect(mocks.markAndDeleteTempTestBranch).toHaveBeenCalledWith( + expect.objectContaining({ id: 1 }), + "test-br", + ); + }); + it("checks the direct dev server instead of the HTML-rewriting proxy", async () => { mocks.createTempTestBranch.mockResolvedValue({ branchId: "test-br", @@ -467,6 +510,12 @@ describe("prepareIsolatedTestDatabase — auth provisioning", () => { email: "neon-test@dyad.test", password: "neon-pw", }); + await prepared.authorizeRuntimeOrigin?.("http://127.0.0.1:49999"); + expect(mocks.ensureNeonAuthTrustedOrigin).toHaveBeenCalledWith({ + projectId: "proj-1", + branchId: "test-br", + origin: "http://127.0.0.1:49999", + }); } finally { fetchSpy.mockRestore(); } @@ -514,6 +563,7 @@ describe("prepareIsolatedTestDatabase — auth provisioning", () => { expect(mocks.createNeonTestAccount).not.toHaveBeenCalled(); expect(prepared.authSetup).toBeUndefined(); + expect(prepared.authorizeRuntimeOrigin).toBeUndefined(); } finally { fetchSpy.mockRestore(); } diff --git a/src/ipc/services/isolated_test_db.ts b/src/ipc/services/isolated_test_db.ts index c3c2bc4bde..749f634fc2 100644 --- a/src/ipc/services/isolated_test_db.ts +++ b/src/ipc/services/isolated_test_db.ts @@ -5,6 +5,7 @@ import { getDyadAppPath } from "../../paths/paths"; import { apps } from "../../db/schema"; import { createTempTestBranch, + markTestBranchCleanupOnly, markAndDeleteTempTestBranch, } from "../utils/neon_test_branch"; import { createNeonTestAccount } from "../utils/neon_test_account"; @@ -22,6 +23,7 @@ import { updateNeonEnvVars, } from "../utils/app_env_var_utils"; import { detectFrameworkType } from "../utils/framework_utils"; +import { ensureNeonAuthTrustedOrigin } from "../utils/neon_utils"; import { runningApps, stopAppByInfo } from "../utils/process_manager"; import { cleanUpPort, executeApp } from "./app_runtime_service"; import { appRunActorService } from "./app_run_actor_service"; @@ -92,6 +94,12 @@ export interface PreparedIsolation { * failed. Never contains privileged keys. */ authSetup?: IsolationAuthSetup; + /** + * Authorize the run-scoped server origin with the isolated auth provider. + * Only Neon Auth isolation supplies this; the E2E runner calls it after the + * server chooses its port and before Playwright sends any requests. + */ + authorizeRuntimeOrigin?: (origin: string) => Promise; teardown: (options?: TeardownOptions) => Promise; } @@ -123,11 +131,17 @@ export async function prepareIsolatedTestDatabase({ emit, runtimeMode, signal, + appPathOverride, + restartApp = true, }: { app: AppRow; emit: EmitOutput; runtimeMode: string; signal?: AbortSignal; + /** E2E-only sandbox path. The recorder deliberately omits this. */ + appPathOverride?: string; + /** E2E sandboxes start their own runtime after isolation is prepared. */ + restartApp?: boolean; }): Promise { // Supabase: isolate via a throwaway, RLS-scoped test user. if (app.supabaseProjectId) { @@ -135,7 +149,8 @@ export async function prepareIsolatedTestDatabase({ } // No Neon project → nothing to isolate. - if (!app.neonProjectId) { + const neonProjectId = app.neonProjectId; + if (!neonProjectId) { return { isolation: { mode: "none" }, teardown: NOOP_TEARDOWN }; } @@ -150,7 +165,7 @@ export async function prepareIsolatedTestDatabase({ }; } - const appPath = getDyadAppPath(app.path); + const appPath = appPathOverride ?? getDyadAppPath(app.path); let envSnapshot: string | null = null; let envModified = false; let branchId: string | undefined; @@ -178,7 +193,7 @@ export async function prepareIsolatedTestDatabase({ "setup", ); } - if (envRestored && !options.skipRestart) { + if (envRestored && restartApp && !options.skipRestart) { try { await restartAppInPlace({ app, appPath }); } catch (error) { @@ -222,6 +237,12 @@ export async function prepareIsolatedTestDatabase({ // 2. Create the throwaway branch (off the preview branch, CoW). const branch = await createTempTestBranch(app); branchId = branch.branchId; + // The E2E sandbox never points the real app env at this branch. Persist the + // cleanup-only form immediately so crash/startup reconciliation cannot + // mistake this run for the recorder's real-env swap. + if (!restartApp) { + await markTestBranchCleanupOnly(app, branchId); + } // 3. Point the app at the throwaway branch. Mark the env as modified before // the write so a partial failure still triggers a restore in teardown. @@ -237,9 +258,11 @@ export async function prepareIsolatedTestDatabase({ // 4. Restart so the dev server reads the throwaway branch, then wait until // it's serving again before Playwright points at it. - emit("Starting the app against the isolated test database…\n", "setup"); - const processId = await restartAppInPlace({ app, appPath }); - await waitForServerReady(app.id, signal, processId); + if (restartApp) { + emit("Starting the app against the isolated test database…\n", "setup"); + const processId = await restartAppInPlace({ app, appPath }); + await waitForServerReady(app.id, signal, processId); + } // 5. If the app uses Neon Auth, provision a throwaway Better Auth account on // the branch so auth-gated recordings/tests can sign in. Best-effort: on @@ -285,6 +308,15 @@ export async function prepareIsolatedTestDatabase({ isolation: { mode: "neon-branch" }, testCredentials, authSetup, + authorizeRuntimeOrigin: branch.neonAuthBaseUrl + ? async (origin) => { + await ensureNeonAuthTrustedOrigin({ + projectId: neonProjectId, + branchId: branch.branchId, + origin, + }); + } + : undefined, teardown, }; } catch (error) { diff --git a/src/ipc/utils/neon_utils.test.ts b/src/ipc/utils/neon_utils.test.ts index f785f62736..61fb1f1dc1 100644 --- a/src/ipc/utils/neon_utils.test.ts +++ b/src/ipc/utils/neon_utils.test.ts @@ -11,6 +11,7 @@ const mocks = vi.hoisted(() => { where, generateCookieSecret: vi.fn(() => "generated".padEnd(64, "0")), readEnvVarsOrEmpty: vi.fn(), + getNeonClient: vi.fn(), }; }); @@ -37,7 +38,7 @@ vi.mock("@/ipc/utils/app_env_var_utils", () => ({ })); vi.mock("@/neon_admin/neon_management_client", () => ({ - getNeonClient: vi.fn(), + getNeonClient: mocks.getNeonClient, })); vi.mock("@/neon_admin/neon_context", () => ({ @@ -56,6 +57,7 @@ vi.mock("electron-log", () => ({ })); import { + ensureNeonAuthTrustedOrigin, getOrCreateNeonAuthCookieSecret, syncActiveNeonAuthCookieSecretFromEnv, } from "@/ipc/utils/neon_utils"; @@ -323,3 +325,65 @@ describe("syncActiveNeonAuthCookieSecretFromEnv", () => { expect(mocks.set).not.toHaveBeenCalled(); }); }); + +describe("ensureNeonAuthTrustedOrigin", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("preserves an HTTP loopback origin exactly", async () => { + const listBranchNeonAuthTrustedDomains = vi.fn().mockResolvedValue({ + data: { domains: [] }, + }); + const addBranchNeonAuthTrustedDomain = vi + .fn() + .mockResolvedValue({ data: undefined }); + mocks.getNeonClient.mockResolvedValue({ + listBranchNeonAuthTrustedDomains, + addBranchNeonAuthTrustedDomain, + }); + + const result = await ensureNeonAuthTrustedOrigin({ + projectId: "proj-1", + branchId: "br-test", + origin: "http://127.0.0.1:35129/path", + }); + + expect(result).toBe("http://127.0.0.1:35129"); + expect(addBranchNeonAuthTrustedDomain).toHaveBeenCalledWith( + "proj-1", + "br-test", + { + domain: "http://127.0.0.1:35129", + auth_provider: "better_auth", + }, + ); + }); + + it("does not add an origin that is already trusted", async () => { + const listBranchNeonAuthTrustedDomains = vi.fn().mockResolvedValue({ + data: { + domains: [ + { + domain: "http://127.0.0.1:35129", + auth_provider: "better_auth", + }, + ], + }, + }); + const addBranchNeonAuthTrustedDomain = vi.fn(); + mocks.getNeonClient.mockResolvedValue({ + listBranchNeonAuthTrustedDomains, + addBranchNeonAuthTrustedDomain, + }); + + const result = await ensureNeonAuthTrustedOrigin({ + projectId: "proj-1", + branchId: "br-test", + origin: "http://127.0.0.1:35129", + }); + + expect(result).toBeNull(); + expect(addBranchNeonAuthTrustedDomain).not.toHaveBeenCalled(); + }); +}); diff --git a/src/ipc/utils/neon_utils.ts b/src/ipc/utils/neon_utils.ts index 03a3e8deeb..993cdd3199 100644 --- a/src/ipc/utils/neon_utils.ts +++ b/src/ipc/utils/neon_utils.ts @@ -397,6 +397,42 @@ export async function ensureNeonAuthTrustedDomain({ return toAdd; } +/** + * Registers an exact HTTP(S) origin without applying deployment-domain + * normalization. Run-scoped E2E servers use HTTP loopback origins, so changing + * their scheme to HTTPS would register a different origin and Better Auth would + * continue rejecting sign-in requests. + */ +export async function ensureNeonAuthTrustedOrigin({ + projectId, + branchId, + origin, +}: { + projectId: string; + branchId: string; + origin: string; +}): Promise { + const trustedOrigin = new URL(origin).origin; + const neonClient = await getNeonClient(); + const existing = await neonClient.listBranchNeonAuthTrustedDomains( + projectId, + branchId, + ); + const alreadyTrusted = (existing.data?.domains ?? []).some(({ domain }) => { + try { + return new URL(domain).origin === trustedOrigin; + } catch { + return domain === trustedOrigin; + } + }); + if (alreadyTrusted) return null; + await neonClient.addBranchNeonAuthTrustedDomain(projectId, branchId, { + domain: trustedOrigin, + auth_provider: NeonAuthSupportedAuthProvider.BetterAuth, + }); + return trustedOrigin; +} + export interface ResolvedNeonBranchEnvVars { databaseUrl: string; neonAuthBaseUrl?: string; diff --git a/src/ipc/utils/playwright_bootstrap.ts b/src/ipc/utils/playwright_bootstrap.ts index e0c49568ad..947f7d0c70 100644 --- a/src/ipc/utils/playwright_bootstrap.ts +++ b/src/ipc/utils/playwright_bootstrap.ts @@ -121,10 +121,9 @@ export function buildPlaywrightConfig(channel: BrowserChannel | null): string { : `// Uses Playwright's bundled Chromium (downloaded on first run).`; return `import { defineConfig } from "@playwright/test"; -// ${DYAD_CONFIG_SENTINEL}. The dev server is started separately by Dyad's -// preview, so we point baseURL at the already-running proxy URL (passed via -// env) rather than using Playwright's \`webServer\` (which would double-start -// the app). +// ${DYAD_CONFIG_SENTINEL}. Dyad starts an isolated test server separately, so +// we point baseURL at its run-scoped URL (passed via env) rather than using +// Playwright's \`webServer\` (which would double-start the app). // ${browserNote} export default defineConfig({ testDir: "./${E2E_TEST_DIR}", diff --git a/src/ipc/utils/test_screenshot.ts b/src/ipc/utils/test_screenshot.ts index d20722a0cb..5413685dd8 100644 --- a/src/ipc/utils/test_screenshot.ts +++ b/src/ipc/utils/test_screenshot.ts @@ -1,6 +1,8 @@ import fs from "node:fs"; import path from "node:path"; import log from "electron-log"; +import { getUserDataPath } from "@/paths/paths"; +import { E2E_TEST_ARTIFACT_DIR } from "@/ipc/services/e2e_test_workspace"; const logger = log.scope("test_screenshot"); @@ -17,7 +19,8 @@ const MAX_SCREENSHOT_BYTES = 5 * 1024 * 1024; * Read a Playwright failure screenshot as a PNG data URL, enforcing the same * containment guards as the `tests:screenshot` IPC handler: PNG-only, resolved * through symlinks, and inside the app's `test-results/` directory. Returns - * null if the path is missing, not a PNG, or escapes the app dir. + * null if the path is missing, not a PNG, or escapes both the app and Dyad's + * retained per-run artifact root. * * Shared by the IPC handler (renderer thumbnails) and the agent's run_tests * tool (attaching a failure screenshot to the model). @@ -25,6 +28,7 @@ const MAX_SCREENSHOT_BYTES = 5 * 1024 * 1024; export async function readTestScreenshotDataUrl( appPath: string, screenshotPath: string, + appId?: number, ): Promise { // Playwright reports absolute paths, but resolve relative ones against the // app dir just in case. @@ -42,12 +46,20 @@ export async function readTestScreenshotDataUrl( // read escapes. Resolve the app path too so ancestor symlinks (e.g. // /var -> /private/var on macOS) don't leave a `..` prefix. let realAppPath: string; + let realArtifactRoot: string | undefined; let realPath: string; try { [realAppPath, realPath] = await Promise.all([ fs.promises.realpath(appPath), fs.promises.realpath(resolved), ]); + try { + realArtifactRoot = await fs.promises.realpath( + path.join(getUserDataPath(), E2E_TEST_ARTIFACT_DIR), + ); + } catch { + // No retained sandbox artifacts yet. + } } catch (error) { logger.warn(`Failed to resolve screenshot path ${resolved}: ${error}`); return null; @@ -57,17 +69,33 @@ export async function readTestScreenshotDataUrl( if (path.extname(realPath).toLowerCase() !== ".png") { return null; } - const rel = path.relative(realAppPath, realPath); + const appRelative = path.relative(realAppPath, realPath); const insideApp = - rel !== "" && !rel.startsWith("..") && !path.isAbsolute(rel); - if (!insideApp) { + appRelative !== "" && + !appRelative.startsWith("..") && + !path.isAbsolute(appRelative); + const artifactRelative = realArtifactRoot + ? path.relative(realArtifactRoot, realPath) + : ""; + const insideArtifacts = + artifactRelative !== "" && + !artifactRelative.startsWith("..") && + !path.isAbsolute(artifactRelative); + if (!insideApp && !insideArtifacts) { return null; } // Only serve screenshots under `test-results/`, not any PNG in the app. Use // split (not a string prefix) so a sibling like `test-results-foo/` can't // slip through. - const [firstSegment] = rel.split(path.sep); - if (firstSegment !== "test-results") { + const segments = (insideApp ? appRelative : artifactRelative).split(path.sep); + const testResultsSegment = insideApp ? segments[0] : segments[1]; + if ( + insideArtifacts && + (appId === undefined || !segments[0].startsWith(`${appId}-`)) + ) { + return null; + } + if (testResultsSegment !== "test-results") { return null; } let handle: fs.promises.FileHandle | undefined; diff --git a/src/main.ts b/src/main.ts index f7b6607e30..129908a31d 100644 --- a/src/main.ts +++ b/src/main.ts @@ -49,6 +49,8 @@ import { apps } from "./db/schema"; import { eq } from "drizzle-orm"; import { reconcileOrphanTestBranches } from "./ipc/utils/neon_test_branch"; import { reconcileOrphanTestUsers } from "./ipc/utils/supabase_test_user"; +import { reconcileOrphanE2eTestWorkspaces } from "./ipc/services/e2e_test_workspace"; +import { stopAllAppTestsSync } from "./ipc/handlers/tests_handlers"; import { UserSettings } from "./lib/schemas"; import { handleNeonOAuthReturn } from "./neon_admin/neon_return_handler"; import { @@ -457,6 +459,7 @@ export async function onReady() { // must not block startup. void reconcileOrphanTestBranches(); void reconcileOrphanTestUsers(); + void reconcileOrphanE2eTestWorkspaces(); // Cleanup old ai_messages_json entries to prevent database bloat cleanupOldAiMessagesJson(); @@ -1682,6 +1685,7 @@ app.on("will-quit", () => { // Synchronously send kill signals to all running apps (fire-and-forget). // We cannot use async/await here because Electron won't wait for it. + stopAllAppTestsSync(); stopAllAppsSync(); // Stop performance monitoring and capture final metrics diff --git a/src/pro/main/ipc/handlers/local_agent/tools/run_tests.ts b/src/pro/main/ipc/handlers/local_agent/tools/run_tests.ts index a736d585f9..f5378bc4bf 100644 --- a/src/pro/main/ipc/handlers/local_agent/tools/run_tests.ts +++ b/src/pro/main/ipc/handlers/local_agent/tools/run_tests.ts @@ -409,6 +409,7 @@ async function attachFailureArtifacts( const dataUrl = await readTestScreenshotDataUrl( ctx.appPath, shot.screenshotPath, + ctx.appId, ); if (dataUrl) { ctx.appendUserMessage([ From 6cad874b60e8c00dd48cdb0fd35dc8f8b014ba8f Mon Sep 17 00:00:00 2001 From: Mohamed Aziz Mejri Date: Sun, 23 Aug 2026 19:42:45 +0000 Subject: [PATCH 2/9] Address PR review comments - Evaluate the testingEnabled and runtime-mode guards before the prepare-e2e-test-workspace stage, so a refused run no longer mutates the user's real project via ensurePlaywrightBootstrap or takes a multi-gigabyte snapshot first. - Restore E2E testing for Docker/cloud runtime: those runtimes fall back to the pre-sandbox path (bootstrap + normal preview) with the missing runtime isolation disclosed on the result. Only Neon apps are refused, because without a sandbox the run would hit the user's real database. - Tree-kill run-scoped children synchronously on quit via a new process registry. Aborting alone routed into the async killProcess/tree-kill path, which will-quit never awaits, leaving the sandbox server holding its port and its cwd under /test-sandboxes. - Scope the startup sandbox sweep: delete run directories individually and skip any run this process still owns, instead of removing the shared root while a freshly started run is mid-copy. - Run custom start commands verbatim instead of appending `-- --port`, and treat a command as custom only when both installCommand and startCommand are set, matching getCommand in app_runtime_service. A custom command that never binds the run-scoped port now gets a {port} hint instead of a bare timeout. - Select the sandbox package manager with getPackageManagerSignal / choosePackageManagerFromSignal so a pnpm lockfile falls back to npm when pnpm is missing or too old, exactly as the normal preview does. - Thread the stage-1 bootstrap's `installed` flag into runAppTestsCore so the e2e_tests_run `first_run` telemetry property stops always reporting false. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JhzboMdTdVq829U81ZCiNb --- src/ipc/handlers/tests_handlers.test.ts | 152 +++++++++++ src/ipc/handlers/tests_handlers.ts | 247 +++++++++++++++--- .../e2e_test_process_registry.test.ts | 63 +++++ src/ipc/services/e2e_test_process_registry.ts | 58 ++++ src/ipc/services/e2e_test_runtime.test.ts | 75 +++++- src/ipc/services/e2e_test_runtime.ts | 76 +++++- src/ipc/services/e2e_test_workspace.test.ts | 27 ++ src/ipc/services/e2e_test_workspace.ts | 45 +++- 8 files changed, 691 insertions(+), 52 deletions(-) create mode 100644 src/ipc/services/e2e_test_process_registry.test.ts create mode 100644 src/ipc/services/e2e_test_process_registry.ts diff --git a/src/ipc/handlers/tests_handlers.test.ts b/src/ipc/handlers/tests_handlers.test.ts index 54c7ed934a..3c93ec21dc 100644 --- a/src/ipc/handlers/tests_handlers.test.ts +++ b/src/ipc/handlers/tests_handlers.test.ts @@ -7,6 +7,8 @@ import { eq } from "drizzle-orm"; import { DyadErrorKind } from "@/errors/dyad_error"; import type { RemoveFileAndCommitResult } from "../services/git_service"; import { apps } from "@/db/schema"; +import { DEFAULT_SETTINGS } from "@/main/settings"; +import { runningApps } from "../utils/process_manager"; import { appOperationCoordinator, type AppOperationRequest, @@ -74,6 +76,8 @@ vi.mock("../services/git_service", () => ({ const queueCloudSandboxSnapshotSyncMock = vi.hoisted(() => vi.fn()); const prepareIsolatedTestDatabaseMock = vi.hoisted(() => vi.fn()); +const readSettingsMock = vi.hoisted(() => vi.fn()); +const sendTelemetryEventMock = vi.hoisted(() => vi.fn()); const ensurePlaywrightBootstrapMock = vi.hoisted(() => vi.fn()); const createE2eTestWorkspaceMock = vi.hoisted(() => vi.fn()); const startE2eTestRuntimeMock = vi.hoisted(() => vi.fn()); @@ -128,6 +132,14 @@ vi.mock("../services/e2e_test_workspace", async (importOriginal) => { vi.mock("../services/e2e_test_runtime", () => ({ startE2eTestRuntime: startE2eTestRuntimeMock, })); +vi.mock("@/main/settings", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, readSettings: readSettingsMock }; +}); +vi.mock("../utils/telemetry", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, sendTelemetryEvent: sendTelemetryEventMock }; +}); vi.mock("../utils/spawn_streaming", () => ({ spawnStreaming: spawnStreamingMock, })); @@ -154,6 +166,11 @@ describe("tests handlers", () => { removeFileAndCommitMock.mockClear(); queueCloudSandboxSnapshotSyncMock.mockClear(); prepareIsolatedTestDatabaseMock.mockReset(); + readSettingsMock.mockReset(); + readSettingsMock.mockImplementation(() => + structuredClone(DEFAULT_SETTINGS), + ); + sendTelemetryEventMock.mockReset(); ensurePlaywrightBootstrapMock.mockReset(); ensurePlaywrightBootstrapMock.mockResolvedValue({ installed: false }); createE2eTestWorkspaceMock.mockReset(); @@ -410,6 +427,141 @@ describe("tests handlers", () => { expect(teardown).toHaveBeenCalledOnce(); expect(dispose).toHaveBeenCalledOnce(); }); + + it("refuses a testing-disabled app before bootstrapping or copying", async () => { + const appId = seedApp("app"); + + const result = await runAppTestsWithIsolation({ + event: { sender: {} } as any, + appId, + source: "panel", + }); + + expect(result.infraError?.message).toMatch(/Testing isn't enabled/i); + // Bootstrap writes into the user's real project and the snapshot copies + // the whole app; a refusal must stay side-effect-free. + expect(ensurePlaywrightBootstrapMock).not.toHaveBeenCalled(); + expect(createE2eTestWorkspaceMock).not.toHaveBeenCalled(); + }); + + it("reports the first run in telemetry when bootstrap installed Playwright", async () => { + const appId = seedApp("app"); + harness.db + .update(apps) + .set({ testingEnabled: true }) + .where(eq(apps.id, appId)) + .run(); + ensurePlaywrightBootstrapMock.mockResolvedValue({ installed: true }); + prepareIsolatedTestDatabaseMock.mockResolvedValue({ + isolation: { mode: "none" }, + teardown: vi.fn().mockResolvedValue({ envRestored: true }), + }); + spawnStreamingMock.mockImplementation( + async ({ cwd }: { cwd: string }) => { + const reportPath = path.join(cwd, "test-results", "results.json"); + fs.mkdirSync(path.dirname(reportPath), { recursive: true }); + fs.writeFileSync( + reportPath, + JSON.stringify({ + suites: [ + { + file: "e2e-tests/a.spec.ts", + specs: [ + { + title: "works", + file: "e2e-tests/a.spec.ts", + line: 1, + tests: [{ status: "expected", results: [{}] }], + }, + ], + }, + ], + }), + ); + return { + code: 0, + stdout: "", + stderr: "", + aborted: false, + timedOut: false, + }; + }, + ); + + const result = await runAppTestsWithIsolation({ + event: { sender: {} } as any, + appId, + source: "panel", + }); + + expect(result.infraError).toBeUndefined(); + expect(sendTelemetryEventMock).toHaveBeenCalledWith( + "e2e_tests_run", + expect.objectContaining({ first_run: true }), + ); + }); + + describe("non-host runtime", () => { + afterEach(() => { + runningApps.clear(); + }); + + function seedRunningApp(name: string): number { + const appId = seedApp(name); + harness.db + .update(apps) + .set({ testingEnabled: true }) + .where(eq(apps.id, appId)) + .run(); + readSettingsMock.mockImplementation(() => ({ + ...structuredClone(DEFAULT_SETTINGS), + runtimeMode2: "docker", + })); + runningApps.set(appId, { proxyUrl: "http://localhost:32100" } as any); + return appId; + } + + it("keeps running against the normal preview and discloses the gap", async () => { + const appId = seedRunningApp("app"); + prepareIsolatedTestDatabaseMock.mockResolvedValue({ + isolation: { mode: "supabase-test-user" }, + teardown: vi.fn().mockResolvedValue({ envRestored: true }), + }); + + const result = await runAppTestsWithIsolation({ + event: { sender: {} } as any, + appId, + source: "panel", + }); + + expect(spawnStreamingMock).toHaveBeenCalled(); + expect(createE2eTestWorkspaceMock).not.toHaveBeenCalled(); + expect(startE2eTestRuntimeMock).not.toHaveBeenCalled(); + expect(result.isolation).toMatchObject({ + mode: "supabase-test-user", + reason: expect.stringMatching(/docker runtime/i), + }); + }); + + it("refuses a Neon app rather than testing against the real database", async () => { + const appId = seedRunningApp("app"); + harness.db + .update(apps) + .set({ neonProjectId: "neon-project" }) + .where(eq(apps.id, appId)) + .run(); + + const result = await runAppTestsWithIsolation({ + event: { sender: {} } as any, + appId, + source: "panel", + }); + + expect(result.infraError?.message).toMatch(/real database/i); + expect(prepareIsolatedTestDatabaseMock).not.toHaveBeenCalled(); + expect(spawnStreamingMock).not.toHaveBeenCalled(); + }); + }); }); describe("stop progress events", () => { diff --git a/src/ipc/handlers/tests_handlers.ts b/src/ipc/handlers/tests_handlers.ts index 41a320e9ff..417092e686 100644 --- a/src/ipc/handlers/tests_handlers.ts +++ b/src/ipc/handlers/tests_handlers.ts @@ -53,8 +53,15 @@ import { parseTestCases } from "../utils/parse_test_cases"; import { getPackageManagerCommandEnv } from "../utils/socket_firewall"; import { queueCloudSandboxSnapshotSync } from "../utils/cloud_sandbox_provider"; import { sendTelemetryEvent } from "../utils/telemetry"; -import type { PreparedIsolation } from "../services/isolated_test_db"; +import { + prepareIsolatedTestDatabase, + type PreparedIsolation, +} from "../services/isolated_test_db"; import { prepareE2eTestDataIsolation } from "../services/e2e_test_data_isolation"; +import { + stopE2eTestProcessesSync, + trackE2eTestProcess, +} from "../services/e2e_test_process_registry"; import { createE2eTestWorkspace, retainE2eTestArtifacts, @@ -203,6 +210,12 @@ export function stopAllAppTestsSync(): void { for (const run of testRunControllers.values()) { run.controller.abort(); } + // Aborting is not enough here. The abort listeners route into `killProcess`, + // which tree-kills asynchronously, and `will-quit` does not await async work. + // Tree-kill the run-scoped children synchronously too, or a sandbox dev + // server survives the quit holding its port and its cwd under + // `/test-sandboxes`. + stopE2eTestProcessesSync(); } export async function endTestsForApp(appId: number): Promise { @@ -260,6 +273,12 @@ export interface RunAppTestsCoreOptions { baseUrl?: string; /** Bootstrap is performed against the real app before sandbox creation. */ skipBootstrap?: boolean; + /** + * Result of that earlier bootstrap. Threaded in with `skipBootstrap` so the + * `first_run` telemetry property keeps meaning "Playwright was installed by + * this run" instead of always reporting false for sandboxed runs. + */ + bootstrapInstalled?: boolean; /** When set, runs a single spec file (relative path); otherwise runs all. */ testFile?: string; /** @@ -312,6 +331,7 @@ export async function runAppTestsCore({ appPath: explicitAppPath, baseUrl: explicitBaseUrl, skipBootstrap = false, + bootstrapInstalled = false, testFile, testLine, grep, @@ -353,7 +373,8 @@ export async function runAppTestsCore({ } // 1. Lazy bootstrap (install Playwright + browser, write config), streamed. - let installed = false; + // Sandboxed runs bootstrap the real app earlier and pass the outcome in. + let installed = bootstrapInstalled; if (!skipBootstrap) { try { const result = await ensurePlaywrightBootstrap({ @@ -443,6 +464,9 @@ export async function runAppTestsCore({ signal, timeoutMs, onOutput: (chunk) => emit(chunk, "running"), + // Quit tree-kills the runner synchronously; the signal path alone would + // leave a headless browser and the sandbox cwd behind. + onProcess: trackE2eTestProcess, }); } catch (error) { // A spawn failure (e.g. npx missing from PATH) rejects rather than exiting @@ -570,6 +594,137 @@ export async function runAppTestsCore({ return { appId, results }; } +/** + * Docker/cloud fallback. The run-scoped sandbox server is host-only for now, so + * these runtimes keep the pre-sandbox path — bootstrap and run against the + * normal preview — with the missing runtime isolation disclosed on the result + * rather than losing E2E testing entirely. Neon apps are the one exception: + * without a sandbox there is no throwaway branch to point the app at, so the + * only way to run would be against the user's real database. + */ +async function runTestsAgainstNormalPreview({ + appId, + runtimeMode, + signal, + emit, + emitProgress, + onIsolationCleanupFailed, + testFile, + testLine, + grep, + headed, + parallel, + timeoutMs, +}: { + appId: number; + runtimeMode: string; + signal: AbortSignal; + emit: (chunk: string, phase: "setup" | "running") => void; + emitProgress: ( + state: "stopping" | "cleaning-up", + isolation?: TestIsolation, + ) => void; + onIsolationCleanupFailed: (failed: boolean) => void; + testFile?: string; + testLine?: number; + grep?: string; + headed?: boolean; + parallel?: boolean; + timeoutMs?: number; +}): Promise { + return appOperationCoordinator.run( + { + appId, + operation: "run-app-tests", + // This path runs Playwright against the user's real working tree and the + // normal preview, so it claims both — unlike the sandboxed path, which + // releases the tree after snapshotting and never touches the preview. + resources: [ + readAppResource("app-path"), + readAppResource("repository-ref"), + "repository-worktree", + "provider", + "runtime", + "runtime-config", + "test-files", + ], + allowCompatibleQueueBypass: true, + refuseWhenRecording: "run tests", + }, + async () => { + const app = await getApp(appId); + // Supabase isolation is provider-side and works in any runtime, so only + // an app whose isolation depends on the Neon branch swap is refused. + if (!app.supabaseProjectId && app.neonProjectId) { + return { + appId, + results: [], + infraError: { + message: `Isolated E2E test servers aren't available in ${runtimeMode} runtime yet, and Dyad won't run Neon tests against your real database. Switch to host runtime to run tests for this app.`, + }, + isolation: { + mode: "none" as const, + reason: `Sandboxed E2E execution is not available in ${runtimeMode} runtime yet.`, + }, + }; + } + + let prepared: PreparedIsolation | undefined; + try { + prepared = await prepareIsolatedTestDatabase({ + app, + emit, + runtimeMode, + signal, + }); + // Disclose the missing runtime sandbox, without overwriting a more + // specific provider reason (e.g. the Supabase publishable-key hint). + const isolation: TestIsolation = { + ...prepared.isolation, + reason: + prepared.isolation.reason ?? + `Tests run against your normal preview because isolated test servers aren't available in ${runtimeMode} runtime yet.`, + }; + if (prepared.infraError) { + return { + appId, + results: [], + infraError: prepared.infraError, + isolation, + }; + } + const result = await runAppTestsCore({ + appId, + testFile, + testLine, + grep, + headed, + parallel, + signal, + timeoutMs, + onOutput: emit, + testEnv: prepared.testCredentials, + }); + return { ...result, isolation }; + } finally { + if (prepared) { + try { + if (prepared.isolation.mode !== "none") { + emitProgress("cleaning-up", prepared.isolation); + } + onIsolationCleanupFailed(true); + onIsolationCleanupFailed(!(await prepared.teardown()).envRestored); + } catch (error) { + logger.error( + `Failed to tear down isolated test environment for app ${appId}: ${error}`, + ); + } + } + } + }, + ); +} + export interface RunTestsWithIsolationOptions { /** * The invoking IPC event. Its `sender` is where `tests:output` and @@ -770,14 +925,53 @@ export async function runAppTestsWithIsolation({ // The database lookup intentionally happens only after this run registered // above. Keeping every await behind registration ensures a rapid second // invocation chains behind this run instead of racing its isolation setup - // and env-file swap. (The resolved app is re-fetched inside the lock below, - // so this call exists only for the ordering barrier.) - await getApp(appId); + // and env-file swap. + const guardApp = await getApp(appId); + + // Decide both refusals BEFORE the workspace stage. `ensurePlaywrightBootstrap` + // is not read-only — it installs `@playwright/test` into the user's real + // project, writes Dyad's config, and can download a browser — and the + // snapshot then copies the whole app plus `node_modules`. Neither may run + // for a run that is about to be turned away. + if (!guardApp.testingEnabled) { + finalResult = { + appId, + results: [], + infraError: { + message: + "Testing isn't enabled for this app. Enable it in the Tests panel before running tests.", + }, + }; + return finalResult; + } + + const runtimeMode = readSettings().runtimeMode2 ?? "host"; + if (runtimeMode !== "host") { + finalResult = withIsolationCleanupWarning( + await runTestsAgainstNormalPreview({ + appId, + runtimeMode, + signal: controller.signal, + emit, + emitProgress, + onIsolationCleanupFailed: (failed) => { + isolationCleanupFailed = failed; + }, + testFile: normalizedTestFile ?? undefined, + testLine, + grep, + headed, + parallel, + timeoutMs, + }), + ); + return finalResult; + } // Bootstrap and snapshot under the real working-tree claim, then release it // before Playwright runs so ordinary app editing can continue against the // normal preview while this run uses its captured filesystem state. - workspace = await appOperationCoordinator.run( + const prepareResult = await appOperationCoordinator.run( { appId, operation: "prepare-e2e-test-workspace", @@ -790,22 +984,26 @@ export async function runAppTestsWithIsolation({ refuseWhenRecording: "run tests", }, async () => { - const app = await getApp(appId); - const realAppPath = getDyadAppPath(app.path); - await ensurePlaywrightBootstrap({ + const claimedApp = await getApp(appId); + const realAppPath = getDyadAppPath(claimedApp.path); + const { installed } = await ensurePlaywrightBootstrap({ appPath: realAppPath, signal: controller.signal, onOutput: (chunk) => emit(chunk, "setup"), }); emit("Copying the app into an isolated test workspace…\n", "setup"); - return createE2eTestWorkspace({ - appId, - appPath: realAppPath, - signal: controller.signal, - onProgress: (message) => emit(message, "setup"), - }); + return { + installed, + workspace: await createE2eTestWorkspace({ + appId, + appPath: realAppPath, + signal: controller.signal, + onProgress: (message) => emit(message, "setup"), + }), + }; }, ); + workspace = prepareResult.workspace; // The live test only owns provider/test inputs. It deliberately does not // claim the normal runtime or runtime-config: its process is run-scoped and @@ -842,6 +1040,8 @@ export async function runAppTestsWithIsolation({ try { const app = await getApp(appId); + // Re-checked under this claim: the two stages take separate claims, + // so testing can be turned off between the snapshot and the run. if (!app.testingEnabled) { return { appId, @@ -853,21 +1053,6 @@ export async function runAppTestsWithIsolation({ }; } - const runtimeMode = readSettings().runtimeMode2 ?? "host"; - if (runtimeMode !== "host") { - return { - appId, - results: [], - infraError: { - message: `Isolated E2E test servers currently require host runtime. Switch from ${runtimeMode} runtime before running tests.`, - }, - isolation: { - mode: "none" as const, - reason: `Sandboxed E2E execution is not available in ${runtimeMode} runtime yet.`, - }, - }; - } - // Set up isolation so the run never mutates the user's real data: // Neon apps get a throwaway copy-on-write branch, Supabase apps get // a throwaway RLS-scoped test user, and no-DB apps run as-is. @@ -892,6 +1077,7 @@ export async function runAppTestsWithIsolation({ emit("Starting the isolated test server…\n", "setup"); testRuntime = await startE2eTestRuntime({ workspacePath: workspace!.workspacePath, + installCommand: app.installCommand, startCommand: app.startCommand, signal: controller.signal, onOutput: (chunk) => emit(chunk, "setup"), @@ -926,6 +1112,7 @@ export async function runAppTestsWithIsolation({ appPath: workspace!.workspacePath, baseUrl: testRuntime.baseUrl, skipBootstrap: true, + bootstrapInstalled: prepareResult.installed, testFile: normalizedTestFile ?? undefined, testLine, grep, diff --git a/src/ipc/services/e2e_test_process_registry.test.ts b/src/ipc/services/e2e_test_process_registry.test.ts new file mode 100644 index 0000000000..260c6a8a5e --- /dev/null +++ b/src/ipc/services/e2e_test_process_registry.test.ts @@ -0,0 +1,63 @@ +// @vitest-environment node + +import { EventEmitter } from "node:events"; +import type { ChildProcess } from "node:child_process"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const killProcessTreeSyncMock = vi.hoisted(() => vi.fn()); +vi.mock("@/ipc/utils/kill_process_tree_sync", () => ({ + killProcessTreeSync: killProcessTreeSyncMock, +})); + +import { + stopE2eTestProcessesSync, + trackE2eTestProcess, + trackedE2eTestProcessCount, +} from "./e2e_test_process_registry"; + +function fakeChild(pid: number | undefined): ChildProcess { + const child = new EventEmitter() as unknown as ChildProcess; + Object.assign(child, { pid, exitCode: null, signalCode: null }); + return child; +} + +describe("e2e test process registry", () => { + beforeEach(() => { + stopE2eTestProcessesSync(); + killProcessTreeSyncMock.mockReset(); + killProcessTreeSyncMock.mockReturnValue(true); + }); + + it("tree-kills tracked children synchronously", () => { + trackE2eTestProcess(fakeChild(111)); + trackE2eTestProcess(fakeChild(222)); + + stopE2eTestProcessesSync(); + + expect(killProcessTreeSyncMock.mock.calls.map(([pid]) => pid)).toEqual([ + 111, 222, + ]); + expect(trackedE2eTestProcessCount()).toBe(0); + }); + + it("forgets a child once it exits", () => { + const child = fakeChild(333); + trackE2eTestProcess(child); + child.emit("exit", 0, null); + + stopE2eTestProcessesSync(); + + expect(killProcessTreeSyncMock).not.toHaveBeenCalled(); + }); + + it("skips children that already terminated or never spawned", () => { + const exited = fakeChild(444); + Object.assign(exited, { exitCode: 0 }); + trackE2eTestProcess(exited); + trackE2eTestProcess(fakeChild(undefined)); + + stopE2eTestProcessesSync(); + + expect(killProcessTreeSyncMock).not.toHaveBeenCalled(); + }); +}); diff --git a/src/ipc/services/e2e_test_process_registry.ts b/src/ipc/services/e2e_test_process_registry.ts new file mode 100644 index 0000000000..ce7bfeaffc --- /dev/null +++ b/src/ipc/services/e2e_test_process_registry.ts @@ -0,0 +1,58 @@ +import type { ChildProcess } from "node:child_process"; +import log from "electron-log"; + +import { killProcessTreeSync } from "@/ipc/utils/kill_process_tree_sync"; + +const logger = log.scope("e2e_test_process_registry"); + +const runScopedProcesses = new Set(); + +/** + * Track a run-scoped child (the sandbox dev server, the Playwright runner) so + * Electron's synchronous quit can terminate it. Returns an unregister callback; + * the child's own exit/error also drops it. + */ +export function trackE2eTestProcess(child: ChildProcess): () => void { + runScopedProcesses.add(child); + const forget = () => { + runScopedProcesses.delete(child); + }; + child.once("exit", forget); + child.once("error", forget); + return forget; +} + +/** Number of tracked children. Exposed for tests. */ +export function trackedE2eTestProcessCount(): number { + return runScopedProcesses.size; +} + +/** + * Tree-kill every tracked child synchronously. + * + * Aborting the run controllers is not enough on quit: their abort path goes + * through `killProcess`/`tree-kill`, which spawns a helper and completes + * asynchronously, and Electron's `will-quit` does not await async work. A + * surviving sandbox server keeps holding its port and its cwd inside + * `/test-sandboxes`, which then makes the next launch's orphan sweep + * fail on Windows. `stopAllAppsSync` uses `killProcessTreeSync` for the same + * reason. + */ +export function stopE2eTestProcessesSync(): void { + const children = Array.from(runScopedProcesses); + runScopedProcesses.clear(); + if (children.length === 0) return; + logger.info( + `Synchronously stopping ${children.length} E2E test process(es) on quit`, + ); + for (const child of children) { + const pid = child.pid; + if (pid === undefined) continue; + if (child.exitCode !== null || child.signalCode !== null) continue; + if (!killProcessTreeSync(pid)) { + logger.warn( + `Failed to synchronously terminate E2E test process (PID ${pid}) during quit`, + ); + } + } +} diff --git a/src/ipc/services/e2e_test_runtime.test.ts b/src/ipc/services/e2e_test_runtime.test.ts index 34089c0479..2c1289ac1f 100644 --- a/src/ipc/services/e2e_test_runtime.test.ts +++ b/src/ipc/services/e2e_test_runtime.test.ts @@ -3,7 +3,17 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const getPnpmMinimumReleaseAgeSupportMock = vi.hoisted(() => vi.fn()); +vi.mock("@/ipc/utils/socket_firewall", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + getPnpmMinimumReleaseAgeSupport: getPnpmMinimumReleaseAgeSupportMock, + }; +}); import { buildE2eTestStartCommand, @@ -11,9 +21,21 @@ import { } from "./e2e_test_runtime"; import { runningApps } from "@/ipc/utils/process_manager"; +function mockPnpmAvailable(available: boolean) { + getPnpmMinimumReleaseAgeSupportMock.mockResolvedValue({ + available, + minimumReleaseAgeSupported: available, + }); +} + describe("buildE2eTestStartCommand", () => { - it("starts npm without reinstalling dependencies", () => { - const command = buildE2eTestStartCommand({ + beforeEach(() => { + getPnpmMinimumReleaseAgeSupportMock.mockReset(); + mockPnpmAvailable(false); + }); + + it("starts npm without reinstalling dependencies", async () => { + const command = await buildE2eTestStartCommand({ workspacePath: path.resolve("app"), port: 45678, }); @@ -22,20 +44,44 @@ describe("buildE2eTestStartCommand", () => { expect(command.env.PORT).toBe("45678"); }); - it("supports an explicit port placeholder in custom commands", () => { - const command = buildE2eTestStartCommand({ + it("supports an explicit port placeholder in custom commands", async () => { + const command = await buildE2eTestStartCommand({ workspacePath: path.resolve("app"), port: 45678, + installCommand: "custom-install", startCommand: "custom-server --listen {port}", }); expect(command.command).toBe("custom-server --listen 45678"); }); - it("uses pnpm when the sandbox contains its lockfile", () => { + it("runs a custom command verbatim instead of appending a port flag", async () => { + const command = await buildE2eTestStartCommand({ + workspacePath: path.resolve("app"), + port: 45678, + installCommand: "pip install -r requirements.txt", + startCommand: "python server.py", + }); + expect(command.command).toBe("python server.py"); + expect(command.env.PORT).toBe("45678"); + }); + + it("ignores a start command that has no matching install command", async () => { + // `getCommand` in app_runtime_service only treats an app as custom when + // both commands are set; the sandbox must agree with the normal preview. + const command = await buildE2eTestStartCommand({ + workspacePath: path.resolve("app"), + port: 45678, + startCommand: "python server.py", + }); + expect(command.command).toBe("npm run dev -- --port 45678"); + }); + + it("uses pnpm when the sandbox contains its lockfile", async () => { + mockPnpmAvailable(true); const root = fs.mkdtempSync(path.join(process.cwd(), ".e2e-runtime-test-")); try { fs.writeFileSync(path.join(root, "pnpm-lock.yaml"), ""); - const command = buildE2eTestStartCommand({ + const command = await buildE2eTestStartCommand({ workspacePath: root, port: 45678, }); @@ -46,6 +92,20 @@ describe("buildE2eTestStartCommand", () => { } }); + it("falls back to npm when the lockfile wants pnpm but pnpm is unusable", async () => { + const root = fs.mkdtempSync(path.join(process.cwd(), ".e2e-runtime-test-")); + try { + fs.writeFileSync(path.join(root, "pnpm-lock.yaml"), ""); + const command = await buildE2eTestStartCommand({ + workspacePath: root, + port: 45678, + }); + expect(command.command).toBe("npm run dev -- --port 45678"); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + it("starts and stops a server without registering the normal app runtime", async () => { const root = fs.mkdtempSync(path.join(os.tmpdir(), "dyad-e2e-runtime-")); fs.writeFileSync( @@ -61,6 +121,7 @@ http.createServer((_request, response) => response.end("sandbox")) try { runtime = await startE2eTestRuntime({ workspacePath: root, + installCommand: "true", startCommand: `"${process.execPath}" server.mjs {port}`, }); await expect( diff --git a/src/ipc/services/e2e_test_runtime.ts b/src/ipc/services/e2e_test_runtime.ts index 61d3446b5a..c90cd5fa50 100644 --- a/src/ipc/services/e2e_test_runtime.ts +++ b/src/ipc/services/e2e_test_runtime.ts @@ -1,12 +1,16 @@ import { spawn, type ChildProcess } from "node:child_process"; -import fs from "node:fs"; import net from "node:net"; -import path from "node:path"; import log from "electron-log"; +import { trackE2eTestProcess } from "@/ipc/services/e2e_test_process_registry"; +import { + choosePackageManagerFromSignal, + getPackageManagerSignal, +} from "@/ipc/utils/package_manager_selection"; import { killProcess } from "@/ipc/utils/process_manager"; import { getPackageManagerCommandEnv, + getPnpmMinimumReleaseAgeSupport, PNPM_PM_ON_FAIL_IGNORE_ARG, } from "@/ipc/utils/socket_firewall"; @@ -38,23 +42,57 @@ export async function allocateE2eTestPort(): Promise { }); } -export function buildE2eTestStartCommand({ +/** + * Whether the app supplies its own commands. Mirrors `getCommand` in + * `app_runtime_service`: a command counts as custom only when BOTH the install + * and the start command are set, so the sandbox and the normal preview never + * disagree about which apps are Dyad-managed. + */ +export function hasCustomE2eStartCommand({ + installCommand, + startCommand, +}: { + installCommand?: string | null; + startCommand?: string | null; +}): boolean { + return Boolean(installCommand?.trim()) && Boolean(startCommand?.trim()); +} + +export async function buildE2eTestStartCommand({ workspacePath, port, + installCommand, startCommand, }: { workspacePath: string; port: number; + installCommand?: string | null; startCommand?: string | null; -}): { command: string; env: NodeJS.ProcessEnv } { - if (startCommand?.trim()) { - const command = startCommand.includes("{port}") - ? startCommand.replaceAll("{port}", String(port)) - : `${startCommand.trim()} -- --port ${port}`; +}): Promise<{ command: string; env: NodeJS.ProcessEnv }> { + if (hasCustomE2eStartCommand({ installCommand, startCommand })) { + // Run the user's command verbatim, exactly as the normal preview does. + // Appending `-- --port` would break every custom server that doesn't accept + // that flag (a Python server, a shell script, a CLI that spells it + // differently) under test only. `{port}` is the explicit opt-in for + // pointing a custom server at the run-scoped port; otherwise PORT is the + // only hint we can safely supply. + const trimmed = startCommand!.trim(); + const command = trimmed.includes("{port}") + ? trimmed.replaceAll("{port}", String(port)) + : trimmed; return { command, env: { ...process.env, PORT: String(port) } }; } - if (fs.existsSync(path.join(workspacePath, "pnpm-lock.yaml"))) { + // Select the package manager the same way the normal preview does. Choosing + // pnpm from the lockfile alone would break sandboxed runs on machines where + // pnpm is missing or too old, even though the normal preview falls back to + // npm there. + const pnpmSupport = await getPnpmMinimumReleaseAgeSupport(); + const packageManager = choosePackageManagerFromSignal({ + signal: getPackageManagerSignal(workspacePath), + pnpmAvailable: pnpmSupport.available, + }); + if (packageManager === "pnpm") { return { command: `pnpm ${PNPM_PM_ON_FAIL_IGNORE_ARG} run dev --port ${port}`, env: { ...getPackageManagerCommandEnv(), PORT: String(port) }, @@ -90,12 +128,14 @@ async function waitForReady({ signal, outputTail, spawnError, + portHint, }: { baseUrl: string; process: ChildProcess; signal?: AbortSignal; outputTail: () => string; spawnError: () => Error | undefined; + portHint: string; }): Promise { const deadline = Date.now() + SERVER_READY_TIMEOUT_MS; while (Date.now() < deadline) { @@ -122,17 +162,19 @@ async function waitForReady({ await delay(SERVER_READY_POLL_MS, signal); } throw new Error( - `The isolated test server did not become ready within 2 minutes.\n${outputTail()}`, + `The isolated test server did not become ready within 2 minutes.${portHint}\n${outputTail()}`, ); } async function startE2eTestRuntimeOnce({ workspacePath, + installCommand, startCommand, signal, onOutput, }: { workspacePath: string; + installCommand?: string | null; startCommand?: string | null; signal?: AbortSignal; onOutput?: (chunk: string) => void; @@ -140,11 +182,20 @@ async function startE2eTestRuntimeOnce({ if (signal?.aborted) throw new Error("Test run stopped."); const port = await allocateE2eTestPort(); const baseUrl = `http://127.0.0.1:${port}`; - const { command, env } = buildE2eTestStartCommand({ + const { command, env } = await buildE2eTestStartCommand({ workspacePath, port, + installCommand, startCommand, }); + // A verbatim custom command can only reach the run-scoped port through + // `{port}` or PORT. If it ignores both it binds elsewhere and never answers + // here, so name the fix instead of leaving a bare timeout. + const portHint = + hasCustomE2eStartCommand({ installCommand, startCommand }) && + !startCommand!.includes("{port}") + ? ` Your custom start command may be ignoring the PORT environment variable — add {port} to it so Dyad can tell it which port to use.` + : ""; const child = spawn(command, [], { cwd: workspacePath, env, @@ -152,6 +203,7 @@ async function startE2eTestRuntimeOnce({ stdio: "pipe", detached: false, }); + const untrack = trackE2eTestProcess(child); let tail = ""; const append = (data: unknown) => { @@ -173,6 +225,7 @@ async function startE2eTestRuntimeOnce({ if (child.pid && child.exitCode === null && child.signalCode === null) { await killProcess(child); } + untrack(); })(); return stopPromise; }; @@ -186,6 +239,7 @@ async function startE2eTestRuntimeOnce({ signal, outputTail: () => tail, spawnError: () => startError, + portHint, }); logger.info(`Isolated E2E server ready on port ${port}`); return { diff --git a/src/ipc/services/e2e_test_workspace.test.ts b/src/ipc/services/e2e_test_workspace.test.ts index 0626becee8..69b81527bf 100644 --- a/src/ipc/services/e2e_test_workspace.test.ts +++ b/src/ipc/services/e2e_test_workspace.test.ts @@ -8,6 +8,8 @@ vi.mock("@/paths/paths", () => ({ getUserDataPath: vi.fn() })); import { getUserDataPath } from "@/paths/paths"; import { createE2eTestWorkspace, + E2E_TEST_SANDBOX_DIR, + reconcileOrphanE2eTestWorkspaces, retainE2eTestArtifacts, rewriteE2eArtifactPath, shouldCopyE2eWorkspacePath, @@ -135,6 +137,31 @@ describe("E2E test workspace", () => { ).toBe("png"); }); + it("sweeps abandoned sandboxes without touching a live run", async () => { + const root = await tempRoot(); + const appPath = path.join(root, "app"); + const userData = path.join(root, "user-data"); + vi.mocked(getUserDataPath).mockReturnValue(userData); + await fs.mkdir(path.join(appPath, "node_modules"), { recursive: true }); + await fs.writeFile(path.join(appPath, "app.ts"), "app"); + + const live = await createE2eTestWorkspace({ appId: 9, appPath }); + const sandboxRoot = path.join(userData, E2E_TEST_SANDBOX_DIR); + const orphan = path.join(sandboxRoot, "9-1-abandoned"); + await fs.mkdir(orphan, { recursive: true }); + + await reconcileOrphanE2eTestWorkspaces(); + + await expect(fs.stat(orphan)).rejects.toThrow(); + expect( + await fs.readFile(path.join(live.workspacePath, "app.ts"), "utf8"), + ).toBe("app"); + + await live.dispose(); + await reconcileOrphanE2eTestWorkspaces(); + await expect(fs.stat(live.workspacePath)).rejects.toThrow(); + }); + it("uses a root-based exclusion policy", () => { const appPath = path.resolve("app"); expect( diff --git a/src/ipc/services/e2e_test_workspace.ts b/src/ipc/services/e2e_test_workspace.ts index 1045f98abc..5a220f3dc5 100644 --- a/src/ipc/services/e2e_test_workspace.ts +++ b/src/ipc/services/e2e_test_workspace.ts @@ -27,6 +27,13 @@ const EXCLUDED_ROOTS = new Set([ export const E2E_TEST_SANDBOX_DIR = "test-sandboxes"; export const E2E_TEST_ARTIFACT_DIR = "test-artifacts"; +/** + * Run directories owned by an in-flight run. The startup orphan sweep skips + * these so it can never delete a sandbox out from under a run that started + * while the sweep was still walking a multi-gigabyte tree. + */ +const activeWorkspaceNames = new Set(); + export interface E2eTestWorkspace { workspacePath: string; artifactPath: string; @@ -119,13 +126,20 @@ export async function createE2eTestWorkspace({ const artifactPath = path.join(artifactRoot, runName); assertOwnedPath(sandboxRoot, workspacePath); assertOwnedPath(artifactRoot, artifactPath); + // Claim the run directory before the first byte is copied so a concurrent + // orphan sweep already sees it as live. + activeWorkspaceNames.add(runName); let disposed = false; const dispose = async () => { if (disposed) return; disposed = true; assertOwnedPath(sandboxRoot, workspacePath); - await fs.rm(workspacePath, { recursive: true, force: true }); + try { + await fs.rm(workspacePath, { recursive: true, force: true }); + } finally { + activeWorkspaceNames.delete(runName); + } }; try { @@ -182,11 +196,34 @@ export function rewriteE2eArtifactPath( return path.join(artifactPath, relative); } +/** + * Remove sandboxes left behind by a crash. Deletes run directories one by one + * and skips any run this process still owns, rather than removing the shared + * root: the sweep is fire-and-forget from startup and removing a multi-gigabyte + * tree is not instantaneous, so a Run pressed right after launch would + * otherwise be deleted mid-copy and surface as an unexplained ENOENT. + */ export async function reconcileOrphanE2eTestWorkspaces(): Promise { const sandboxRoot = path.join(getUserDataPath(), E2E_TEST_SANDBOX_DIR); + let entries; try { - await fs.rm(sandboxRoot, { recursive: true, force: true }); - } catch (error) { - logger.warn(`Failed to remove abandoned E2E test workspaces: ${error}`); + entries = await fs.readdir(sandboxRoot, { withFileTypes: true }); + } catch (error: any) { + if (error?.code !== "ENOENT") { + logger.warn(`Failed to list abandoned E2E test workspaces: ${error}`); + } + return; + } + for (const entry of entries) { + if (activeWorkspaceNames.has(entry.name)) continue; + const runPath = path.join(sandboxRoot, entry.name); + try { + assertOwnedPath(sandboxRoot, runPath); + await fs.rm(runPath, { recursive: true, force: true }); + } catch (error) { + logger.warn( + `Failed to remove abandoned E2E test workspace ${entry.name}: ${error}`, + ); + } } } From 10e04424fca0e9244020c057ffd3560804ee9c31 Mon Sep 17 00:00:00 2001 From: Mohamed Aziz Mejri Date: Sun, 23 Aug 2026 20:21:43 +0000 Subject: [PATCH 3/9] Address second round of PR review comments - Replace the Tests panel / cancellation-banner copy that still promised Dyad restores the preview and database. Nothing is restored on the host path: the run had its own sandbox and its own server. The Neon disclosure now says the preview keeps its real database, and the cancellationRestoringTestApp string is renamed to cancellationRemovingTestDatabase across all five locales. - Drive the "couldn't finish cleaning up" warning off a new TeardownResult.remoteCleanupCompleted instead of envRestored, which on the sandbox path only reported on a workspace file deleted seconds later. markAndDeleteTempTestBranch now returns the delete verdict. - Return a structured "Test run stopped." result when Stop lands during sandbox setup, instead of rejecting the IPC call and recording an internal product exception for an ordinary cancellation. - Announce cleaning-up before disposing the sandbox for every isolation mode, so removing a cloned node_modules tree is not an unlabelled wait with Run/Record/Delete disabled. - Add workspace copy/dispose telemetry (durations, entry counts, whether a reflink was requested; no absolute paths) and a disableSandboxedE2eTests escape hatch with a Settings toggle. Turning it off routes through the same non-sandboxed path, still failing closed for Neon. - Sweep retained test artifacts: startup reconciliation prunes directories whose app no longer exists, and deleting an app drops its artifacts. - Remove the readiness poll's per-iteration abort listener, which reached Node's MaxListenersExceededWarning on any slow sandbox start. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JhzboMdTdVq829U81ZCiNb --- src/components/SandboxedE2eTestsSwitch.tsx | 29 +++ .../chat/CancellationBanner.test.tsx | 19 +- src/components/chat/CancellationBanner.tsx | 12 +- .../preview_panel/TestsPanel.test.tsx | 58 +++-- src/components/preview_panel/TestsPanel.tsx | 40 ++-- src/i18n/locales/en/chat.json | 4 +- src/i18n/locales/es/chat.json | 4 +- src/i18n/locales/ko/chat.json | 4 +- src/i18n/locales/pt-BR/chat.json | 4 +- src/i18n/locales/zh-CN/chat.json | 4 +- src/ipc/handlers/app_handlers.ts | 10 + src/ipc/handlers/tests_handlers.test.ts | 219 ++++++++++++++++-- src/ipc/handlers/tests_handlers.ts | 104 ++++++--- src/ipc/services/e2e_test_runtime.ts | 22 +- src/ipc/services/e2e_test_workspace.test.ts | 54 +++++ src/ipc/services/e2e_test_workspace.ts | 119 ++++++++-- src/ipc/services/isolated_test_db.ts | 28 ++- src/ipc/utils/neon_test_branch.test.ts | 15 +- src/ipc/utils/neon_test_branch.ts | 8 +- src/lib/schemas.ts | 7 + src/lib/settingsSearchIndex.ts | 20 ++ src/main.ts | 11 +- src/pages/settings.tsx | 12 + 23 files changed, 665 insertions(+), 142 deletions(-) create mode 100644 src/components/SandboxedE2eTestsSwitch.tsx diff --git a/src/components/SandboxedE2eTestsSwitch.tsx b/src/components/SandboxedE2eTestsSwitch.tsx new file mode 100644 index 0000000000..b8c2097bd6 --- /dev/null +++ b/src/components/SandboxedE2eTestsSwitch.tsx @@ -0,0 +1,29 @@ +import { useSettings } from "@/hooks/useSettings"; +import { Label } from "@/components/ui/label"; +import { Switch } from "@/components/ui/switch"; + +/** + * Escape hatch for the sandboxed E2E runtime. Stored inverted + * (`disableSandboxedE2eTests`) so the sandbox stays the default for everyone + * who never opens this, and only an explicit opt-out falls back to running + * against the normal preview. + */ +export function SandboxedE2eTestsSwitch() { + const { settings, updateSettings } = useSettings(); + const enabled = !settings?.disableSandboxedE2eTests; + return ( +
+ { + updateSettings({ disableSandboxedE2eTests: !checked }); + }} + /> + +
+ ); +} diff --git a/src/components/chat/CancellationBanner.test.tsx b/src/components/chat/CancellationBanner.test.tsx index 0ec8d60def..cfb882aa29 100644 --- a/src/components/chat/CancellationBanner.test.tsx +++ b/src/components/chat/CancellationBanner.test.tsx @@ -18,10 +18,10 @@ vi.mock("react-i18next", () => ({ ({ stoppingGeneration: "Stopping…", cancellationEndingTestRun: "Ending the test run.", - cancellationRestoringTestApp: - "Restoring your app's database and preview. This can take a while.", + cancellationRemovingTestDatabase: + "Removing the temporary test database. This can take a while.", cancellationCleaningTestData: - "Cleaning up the test data from this run.", + "Cleaning up this run's test sandbox and data.", })[key] ?? key, }), })); @@ -87,21 +87,26 @@ describe("CancellationBanner", () => { }); expect( - screen.getByText(/Restoring your app's database and preview/), + screen.getByText(/Removing the temporary test database/), ).toBeTruthy(); expect(screen.getByText(/can take a while/)).toBeTruthy(); + // The run had its own sandbox and its own server; the user's preview and + // `.env.local` were never touched, so nothing is being restored. + expect(screen.queryByText(/Restoring/i)).toBeNull(); }); it("does not claim a restore on the Supabase path", () => { - // That teardown only deletes the temporary test user — no env swap, no - // dev-server restart, nothing the user sees. + // That teardown only deletes the temporary test user and the run's sandbox + // copy — no env swap, no dev-server restart, nothing the user sees. renderBanner({ phase: "cleaning-up", isolationMode: "supabase-test-user", source: "agent", }); - expect(screen.getByText(/Cleaning up the test data/)).toBeTruthy(); + expect( + screen.getByText(/Cleaning up this run's test sandbox/), + ).toBeTruthy(); expect(screen.queryByText(/Restoring/)).toBeNull(); }); diff --git a/src/components/chat/CancellationBanner.tsx b/src/components/chat/CancellationBanner.tsx index ac699c92db..23db686588 100644 --- a/src/components/chat/CancellationBanner.tsx +++ b/src/components/chat/CancellationBanner.tsx @@ -11,11 +11,11 @@ import { * Pinned above the composer while a stopped turn settles. * * Stopping is not instant. The agent awaits its in-flight tool, and a - * `run_tests` call first kills the Playwright process tree and then runs an - * isolation teardown that accepts no AbortSignal — restoring `.env.local`, - * restarting the dev server and deleting the temporary Neon branch, whose - * delete retries with backoff. That wait can pass a minute, and the composer - * stays locked for all of it. + * `run_tests` call first kills the Playwright process tree and then runs a + * teardown that accepts no AbortSignal — deleting the temporary Neon branch, + * whose delete retries with backoff, and removing the run's sandbox copy of the + * app. That wait can pass a minute, and the composer stays locked for all of + * it. The user's own `.env.local` and preview are never touched. * * The transcript's inline status card scrolls out of view; this stays fused to * the composer the user just clicked Stop in, so the wait is never unexplained. @@ -38,7 +38,7 @@ export function CancellationBanner({ appId }: { appId?: number | null }) { ? null : runState.phase === "cleaning-up" ? runState.isolation?.mode === "neon-branch" - ? t("cancellationRestoringTestApp") + ? t("cancellationRemovingTestDatabase") : t("cancellationCleaningTestData") : runState.phase === "stopping" ? t("cancellationEndingTestRun") diff --git a/src/components/preview_panel/TestsPanel.test.tsx b/src/components/preview_panel/TestsPanel.test.tsx index 03c21a9bf2..7bc831378c 100644 --- a/src/components/preview_panel/TestsPanel.test.tsx +++ b/src/components/preview_panel/TestsPanel.test.tsx @@ -31,6 +31,8 @@ const mocks = vi.hoisted(() => ({ appUrl: "http://localhost:32100" as string | null, previewUrl: "http://localhost:32100/" as string | null, previewUrlSource: "dyad" as "none" | "dyad" | "app", + app: { id: 1, testingEnabled: true } as Record, + settings: {} as Record, })); vi.mock("@/ipc/types", () => ({ @@ -52,11 +54,11 @@ vi.mock("@/lib/toast", () => ({ })); vi.mock("@/hooks/useLoadApp", () => ({ - useLoadApp: () => ({ app: { id: 1, testingEnabled: true } }), + useLoadApp: () => ({ app: mocks.app }), })); vi.mock("@/hooks/useSettings", () => ({ - useSettings: () => ({ settings: {}, updateSettings: vi.fn() }), + useSettings: () => ({ settings: mocks.settings, updateSettings: vi.fn() }), })); vi.mock("@/hooks/useRunApp", () => ({ @@ -131,6 +133,8 @@ describe("TestsPanel", () => { mocks.appUrl = "http://localhost:32100"; mocks.previewUrl = "http://localhost:32100/"; mocks.previewUrlSource = "dyad"; + mocks.app = { id: 1, testingEnabled: true }; + mocks.settings = {}; mocks.listAppTests.mockResolvedValue({ specs: [ { @@ -391,10 +395,32 @@ describe("TestsPanel", () => { expect(mocks.showError).toHaveBeenCalledWith(error); }); - it("names the Neon teardown, which restarts the preview", () => { + it("tells Neon users their preview keeps its real database", async () => { + // The pre-sandbox copy promised a double preview restart. Nothing + // restarts any more, so that disclosure would now be a lie. + mocks.app = { id: 1, testingEnabled: true, neonProjectId: "neon-proj" }; + renderPanel(); + + expect( + await screen.findByText(/Your preview keeps running against your real/), + ).toBeTruthy(); + expect(screen.queryByText(/restart the preview/i)).toBeNull(); + }); + + it("drops the sandbox disclosure when the sandbox is turned off", async () => { + mocks.app = { id: 1, testingEnabled: true, neonProjectId: "neon-proj" }; + mocks.settings = { disableSandboxedE2eTests: true }; + renderPanel(); + + await screen.findByText("signup.spec.ts"); + expect(screen.queryByText(/Your preview keeps running/)).toBeNull(); + }); + + it("names the Neon teardown without promising a restore", () => { // This is the wait that can pass a minute (the branch delete retries with - // backoff), and it visibly reloads the user's preview. Calling it - // "Running…" — as the panel used to — reads as a hang. + // backoff). Calling it "Running…" — as the panel used to — reads as a + // hang, but the run had its own sandbox and its own server, so nothing + // of the user's is being put back either. const { store } = renderPanel(); setPhase(store, { phase: "cleaning-up", @@ -402,31 +428,31 @@ describe("TestsPanel", () => { }); expect( - screen.getByText(/Restoring your database and preview/), + screen.getByText(/Removing the temporary test database/), ).toBeTruthy(); + expect(screen.queryByText(/Restoring/i)).toBeNull(); expect( - screen.getByRole("button", { name: "Restoring your app" }).textContent, - ).toContain("Restoring…"); + screen.getByRole("button", { + name: "Removing the temporary test database", + }).textContent, + ).toContain("Cleaning up…"); }); - it("does not promise a database restore on the Supabase path", () => { - // That teardown only deletes the temporary test user. It never swaps - // `.env.local` and never restarts the app, so the Neon copy would lie. + it("names the sandbox cleanup on the Supabase path", () => { + // That teardown only deletes the temporary test user and the run's + // sandbox copy. It never swaps `.env.local` and never restarts the app. const { store } = renderPanel(); setPhase(store, { phase: "cleaning-up", isolation: { mode: "supabase-test-user" }, }); - expect(screen.getByText(/Cleaning up the test data/)).toBeTruthy(); - expect(screen.queryByText(/Restoring your database/)).toBeNull(); + expect(screen.getByText(/Cleaning up the test sandbox/)).toBeTruthy(); + expect(screen.queryByText(/Restoring/i)).toBeNull(); expect( screen.getByRole("button", { name: "Cleaning up test data" }) .textContent, ).toContain("Cleaning up…"); - expect( - screen.queryByRole("button", { name: "Restoring your app" }), - ).toBeNull(); }); it("does not carry a completed run's stop latch into the next run", () => { diff --git a/src/components/preview_panel/TestsPanel.tsx b/src/components/preview_panel/TestsPanel.tsx index 10af6903be..d6d806822d 100644 --- a/src/components/preview_panel/TestsPanel.tsx +++ b/src/components/preview_panel/TestsPanel.tsx @@ -715,7 +715,11 @@ export function TestsPanel() { (runState.phase === "cleaning-up" && !runState.wasStopped); const isStopping = runState.phase === "stopping"; const isCleaningUp = runState.phase === "cleaning-up"; - const isRestoringApp = + // Nothing about the user's app is restored any more: the run had its own + // sandbox copy and its own server, so cleanup is deleting that sandbox and + // the temporary database it was pointed at. The real `.env.local` and the + // preview were never touched. + const isRemovingTestDatabase = isCleaningUp && runState.isolation?.mode === "neon-branch"; const specsQuery = useQuery({ queryKey: queryKeys.tests.list({ appId: selectedAppId }), @@ -746,10 +750,15 @@ export function TestsPanel() { }); const loadingSpecs = specsQuery.isLoading && specs.length === 0; - const showNeonRestartDisclosure = + // Host runs get the sandbox: a throwaway copy of the app, its own server, and + // a temporary Neon branch that only that copy points at. Worth saying, since + // the alternative a user would assume is "my tests hit my real database" — + // but it must not promise the preview restart the old env-swap path did. + const showNeonSandboxDisclosure = specs.length > 0 && !!app?.neonProjectId && - (settings?.runtimeMode2 ?? "host") === "host"; + (settings?.runtimeMode2 ?? "host") === "host" && + !settings?.disableSandboxedE2eTests; // Pop the output drawer when a run starts for the app being viewed. Keyed // off the global atom's phase transition — not the raw IPC event — so it @@ -1377,8 +1386,8 @@ export function TestsPanel() { disabled={showStopping || isCleaningUp} aria-label={ isCleaningUp - ? isRestoringApp - ? "Restoring your app" + ? isRemovingTestDatabase + ? "Removing the temporary test database" : "Cleaning up test data" : showStopping ? "Stopping tests" @@ -1397,9 +1406,7 @@ export function TestsPanel() { )} {isCleaningUp - ? isRestoringApp - ? "Restoring…" - : "Cleaning up…" + ? "Cleaning up…" : showStopping ? "Stopping…" : "Stop"} @@ -1448,12 +1455,13 @@ export function TestsPanel() { )} > {isCleaningUp - ? // The Neon teardown restarts the dev server, so the - // preview visibly reloads and the copy has to account for - // it. The Supabase teardown only deletes the test user. + ? // The Neon teardown deletes the throwaway branch on + // Neon's side, which retries with backoff and is the + // slowest case worth naming. Everything else is the local + // sandbox and, for Supabase, the temporary test user. runState.isolation?.mode === "neon-branch" - ? "Restoring your database and preview… " - : "Cleaning up the test data… " + ? "Removing the temporary test database… " + : "Cleaning up the test sandbox… " : showStopping ? "Stopping the tests… " : runState.phase === "setup" @@ -1541,12 +1549,12 @@ export function TestsPanel() { )} - {!isRunning && showNeonRestartDisclosure && ( + {!isRunning && showNeonSandboxDisclosure && (
- Neon test runs restart the preview to switch to a temporary - database, then restart it again afterward. + Neon test runs use a copy of your app and a temporary database. + Your preview keeps running against your real one.
)} diff --git a/src/i18n/locales/en/chat.json b/src/i18n/locales/en/chat.json index 885e8d4ba0..cc4fc95c3e 100644 --- a/src/i18n/locales/en/chat.json +++ b/src/i18n/locales/en/chat.json @@ -80,8 +80,8 @@ "cancelGeneration": "Cancel generation", "stoppingGeneration": "Stopping…", "cancellationEndingTestRun": "Ending the test run.", - "cancellationRestoringTestApp": "Restoring your app's database and preview. This can take a while.", - "cancellationCleaningTestData": "Cleaning up the test data from this run.", + "cancellationRemovingTestDatabase": "Removing the temporary test database. This can take a while.", + "cancellationCleaningTestData": "Cleaning up this run's test sandbox and data.", "sendMessage": "Send message", "loadingProposal": "Loading proposal...", "errorLoadingProposal": "Error loading proposal: {{message}}", diff --git a/src/i18n/locales/es/chat.json b/src/i18n/locales/es/chat.json index fa376c9da6..8c28664c1b 100644 --- a/src/i18n/locales/es/chat.json +++ b/src/i18n/locales/es/chat.json @@ -80,8 +80,8 @@ "cancelGeneration": "Cancelar generación", "stoppingGeneration": "Deteniendo…", "cancellationEndingTestRun": "Finalizando la ejecución de pruebas.", - "cancellationRestoringTestApp": "Restaurando la base de datos y la vista previa de tu aplicación. Esto puede tardar un poco.", - "cancellationCleaningTestData": "Limpiando los datos de prueba de esta ejecución.", + "cancellationRemovingTestDatabase": "Eliminando la base de datos de prueba temporal. Esto puede tardar un poco.", + "cancellationCleaningTestData": "Limpiando el entorno aislado y los datos de prueba de esta ejecución.", "sendMessage": "Enviar mensaje", "loadingProposal": "Cargando propuesta...", "errorLoadingProposal": "Error al cargar la propuesta: {{message}}", diff --git a/src/i18n/locales/ko/chat.json b/src/i18n/locales/ko/chat.json index f72f4f64aa..5c3bd5e1ae 100644 --- a/src/i18n/locales/ko/chat.json +++ b/src/i18n/locales/ko/chat.json @@ -80,8 +80,8 @@ "cancelGeneration": "생성 취소", "stoppingGeneration": "중지하는 중…", "cancellationEndingTestRun": "테스트 실행을 종료하는 중입니다.", - "cancellationRestoringTestApp": "앱의 데이터베이스와 미리보기를 복원하는 중입니다. 시간이 걸릴 수 있습니다.", - "cancellationCleaningTestData": "이 테스트 실행의 데이터를 정리하는 중입니다.", + "cancellationRemovingTestDatabase": "임시 테스트 데이터베이스를 제거하는 중입니다. 시간이 걸릴 수 있습니다.", + "cancellationCleaningTestData": "이번 실행의 테스트 샌드박스와 데이터를 정리하는 중입니다.", "sendMessage": "메시지 보내기", "loadingProposal": "제안을 불러오는 중...", "errorLoadingProposal": "제안 불러오기 오류: {{message}}", diff --git a/src/i18n/locales/pt-BR/chat.json b/src/i18n/locales/pt-BR/chat.json index 19b6a52530..b1afb113d5 100644 --- a/src/i18n/locales/pt-BR/chat.json +++ b/src/i18n/locales/pt-BR/chat.json @@ -80,8 +80,8 @@ "cancelGeneration": "Cancelar geração", "stoppingGeneration": "Parando…", "cancellationEndingTestRun": "Encerrando a execução dos testes.", - "cancellationRestoringTestApp": "Restaurando o banco de dados e a pré-visualização do seu app. Isso pode levar algum tempo.", - "cancellationCleaningTestData": "Limpando os dados de teste desta execução.", + "cancellationRemovingTestDatabase": "Removendo o banco de dados de teste temporário. Isso pode levar algum tempo.", + "cancellationCleaningTestData": "Limpando o ambiente isolado e os dados de teste desta execução.", "sendMessage": "Enviar mensagem", "loadingProposal": "Carregando proposta...", "errorLoadingProposal": "Erro ao carregar proposta: {{message}}", diff --git a/src/i18n/locales/zh-CN/chat.json b/src/i18n/locales/zh-CN/chat.json index 0a3a52a518..eec6531cc1 100644 --- a/src/i18n/locales/zh-CN/chat.json +++ b/src/i18n/locales/zh-CN/chat.json @@ -80,8 +80,8 @@ "cancelGeneration": "取消生成", "stoppingGeneration": "正在停止…", "cancellationEndingTestRun": "正在结束测试运行。", - "cancellationRestoringTestApp": "正在恢复应用的数据库和预览。这可能需要一些时间。", - "cancellationCleaningTestData": "正在清理本次运行的测试数据。", + "cancellationRemovingTestDatabase": "正在删除临时测试数据库。这可能需要一些时间。", + "cancellationCleaningTestData": "正在清理本次运行的测试沙箱和数据。", "sendMessage": "发送消息", "loadingProposal": "正在加载提案...", "errorLoadingProposal": "加载提案出错：{{message}}", diff --git a/src/ipc/handlers/app_handlers.ts b/src/ipc/handlers/app_handlers.ts index c42fdeabef..04c655048a 100644 --- a/src/ipc/handlers/app_handlers.ts +++ b/src/ipc/handlers/app_handlers.ts @@ -147,6 +147,7 @@ import { } from "../utils/neon_test_branch"; import type { AppSearchResult } from "@/lib/schemas"; import { endTestsForApp } from "./tests_handlers"; +import { removeE2eTestArtifactsForApp } from "../services/e2e_test_workspace"; import { getRgExecutablePath, @@ -539,6 +540,15 @@ async function deleteAppById( // recording the user can still save. forgetAppRecordedDrafts(appId); + // Retained screenshots/traces live outside the app directory, under + // /test-artifacts, and are otherwise only replaced by the next run + // of this same app — which will never happen now. + await removeE2eTestArtifactsForApp(appId).catch((error) => + logger.warn( + `App ${appId} was deleted but its retained test artifacts could not be removed: ${error}`, + ), + ); + // Only after the deletion has committed — the throw above skips this. Doing // it earlier means a deletion that then fails leaves a live app pointed at a // database that no longer exists, which is worse than the leak it prevents. diff --git a/src/ipc/handlers/tests_handlers.test.ts b/src/ipc/handlers/tests_handlers.test.ts index 3c93ec21dc..8aa37a354c 100644 --- a/src/ipc/handlers/tests_handlers.test.ts +++ b/src/ipc/handlers/tests_handlers.test.ts @@ -230,7 +230,10 @@ describe("tests handlers", () => { prepareIsolatedTestDatabaseMock.mockResolvedValue({ isolation: { mode: "none" }, infraError: { message: "setup stopped" }, - teardown: vi.fn().mockResolvedValue({ envRestored: true }), + teardown: vi.fn().mockResolvedValue({ + envRestored: true, + remoteCleanupCompleted: true, + }), }); const requests: AppOperationRequest[] = []; const originalRun = appOperationCoordinator.run.bind( @@ -315,7 +318,7 @@ describe("tests handlers", () => { } }); - it("reports an unfinished isolated-database cleanup", async () => { + it("reports a leaked test branch, not an unrestored sandbox env", async () => { const appId = seedApp("app"); harness.db .update(apps) @@ -327,7 +330,10 @@ describe("tests handlers", () => { infraError: { message: "Isolation setup stopped before running tests.", }, - teardown: vi.fn().mockResolvedValue({ envRestored: false }), + teardown: vi.fn().mockResolvedValue({ + envRestored: true, + remoteCleanupCompleted: false, + }), }); const result = await runAppTestsWithIsolation({ @@ -340,6 +346,37 @@ describe("tests handlers", () => { expect(result.infraError?.message).toMatch(/settings were not changed/i); }); + it("stays quiet when only the sandbox env was left unrestored", async () => { + // The sandbox `.env.local` is deleted with the workspace seconds later, + // so `envRestored` says nothing the user needs to hear. + const appId = seedApp("app"); + harness.db + .update(apps) + .set({ testingEnabled: true }) + .where(eq(apps.id, appId)) + .run(); + prepareIsolatedTestDatabaseMock.mockResolvedValue({ + isolation: { mode: "neon-branch" }, + infraError: { + message: "Isolation setup stopped before running tests.", + }, + teardown: vi.fn().mockResolvedValue({ + envRestored: false, + remoteCleanupCompleted: true, + }), + }); + + const result = await runAppTestsWithIsolation({ + event: { sender: {} } as any, + appId, + source: "panel", + }); + + expect(result.infraError?.message).toBe( + "Isolation setup stopped before running tests.", + ); + }); + it("authorizes the isolated server origin before Playwright starts", async () => { const appId = seedApp("app"); harness.db @@ -354,7 +391,10 @@ describe("tests handlers", () => { prepareIsolatedTestDatabaseMock.mockResolvedValue({ isolation: { mode: "neon-branch" }, authorizeRuntimeOrigin, - teardown: vi.fn().mockResolvedValue({ envRestored: true }), + teardown: vi.fn().mockResolvedValue({ + envRestored: true, + remoteCleanupCompleted: true, + }), }); startE2eTestRuntimeMock.mockImplementation(async () => { events.push("server"); @@ -395,7 +435,9 @@ describe("tests handlers", () => { .where(eq(apps.id, appId)) .run(); const stop = vi.fn().mockResolvedValue(undefined); - const teardown = vi.fn().mockResolvedValue({ envRestored: true }); + const teardown = vi + .fn() + .mockResolvedValue({ envRestored: true, remoteCleanupCompleted: true }); const dispose = vi.fn().mockResolvedValue(undefined); createE2eTestWorkspaceMock.mockResolvedValue({ workspacePath: path.join(TEMP_BASE, "app"), @@ -454,7 +496,10 @@ describe("tests handlers", () => { ensurePlaywrightBootstrapMock.mockResolvedValue({ installed: true }); prepareIsolatedTestDatabaseMock.mockResolvedValue({ isolation: { mode: "none" }, - teardown: vi.fn().mockResolvedValue({ envRestored: true }), + teardown: vi.fn().mockResolvedValue({ + envRestored: true, + remoteCleanupCompleted: true, + }), }); spawnStreamingMock.mockImplementation( async ({ cwd }: { cwd: string }) => { @@ -501,6 +546,70 @@ describe("tests handlers", () => { ); }); + it("returns cleanly when Stop lands during the sandbox copy", async () => { + // The workspace copy and the test-server start both signal cancellation + // by throwing. Letting that escape rejects the IPC call and records an + // internal product exception for an ordinary user cancellation. + const appId = seedApp("app"); + harness.db + .update(apps) + .set({ testingEnabled: true }) + .where(eq(apps.id, appId)) + .run(); + const stop = new AbortController(); + createE2eTestWorkspaceMock.mockImplementation(async () => { + stop.abort(); + throw new Error("Test run stopped."); + }); + + const result = await runAppTestsWithIsolation({ + event: { sender: {} } as any, + appId, + source: "panel", + externalSignal: stop.signal, + }); + + expect(result.infraError?.message).toBe("Test run stopped."); + expect(startE2eTestRuntimeMock).not.toHaveBeenCalled(); + }); + + it("routes around the sandbox when the user turned it off", async () => { + const appId = seedApp("app"); + harness.db + .update(apps) + .set({ testingEnabled: true }) + .where(eq(apps.id, appId)) + .run(); + readSettingsMock.mockImplementation(() => ({ + ...structuredClone(DEFAULT_SETTINGS), + disableSandboxedE2eTests: true, + })); + runningApps.set(appId, { proxyUrl: "http://localhost:32100" } as any); + prepareIsolatedTestDatabaseMock.mockResolvedValue({ + isolation: { mode: "none" }, + teardown: vi.fn().mockResolvedValue({ + envRestored: true, + remoteCleanupCompleted: true, + }), + }); + + let result; + try { + result = await runAppTestsWithIsolation({ + event: { sender: {} } as any, + appId, + source: "panel", + }); + } finally { + runningApps.clear(); + } + + expect(createE2eTestWorkspaceMock).not.toHaveBeenCalled(); + expect(startE2eTestRuntimeMock).not.toHaveBeenCalled(); + expect(spawnStreamingMock).toHaveBeenCalled(); + expect(result.isolation?.reason).toMatch(/turned off in Settings/i); + }); + describe("non-host runtime", () => { afterEach(() => { runningApps.clear(); @@ -525,7 +634,10 @@ describe("tests handlers", () => { const appId = seedRunningApp("app"); prepareIsolatedTestDatabaseMock.mockResolvedValue({ isolation: { mode: "supabase-test-user" }, - teardown: vi.fn().mockResolvedValue({ envRestored: true }), + teardown: vi.fn().mockResolvedValue({ + envRestored: true, + remoteCleanupCompleted: true, + }), }); const result = await runAppTestsWithIsolation({ @@ -601,7 +713,7 @@ describe("tests handlers", () => { isolation: { mode: "neon-branch" }, teardown: vi.fn().mockImplementation(async () => { statesWhenTeardownRan = runStates(); - return { envRestored: true }; + return { envRestored: true, remoteCleanupCompleted: true }; }), }); @@ -617,13 +729,17 @@ describe("tests handlers", () => { expect(runStates()).toContain("finished"); }); - it("stays quiet when there is no isolation to tear down", async () => { - // `NOOP_TEARDOWN` returns immediately, so a `cleaning-up` label would - // flash for a frame and read as a glitch. + it("announces the sandbox deletion even with no isolation to tear down", async () => { + // `NOOP_TEARDOWN` returns immediately, but removing the cloned + // node_modules tree does not, and the panel keeps Run/Record/Delete + // disabled for all of it. An unlabelled wait reads as a hang. const appId = seedTestableApp("app"); prepareIsolatedTestDatabaseMock.mockResolvedValue({ isolation: { mode: "none" }, - teardown: vi.fn().mockResolvedValue({ envRestored: true }), + teardown: vi.fn().mockResolvedValue({ + envRestored: true, + remoteCleanupCompleted: true, + }), }); await runAppTestsWithIsolation({ @@ -632,6 +748,37 @@ describe("tests handlers", () => { source: "panel", }); + expect(runStates()).toContain("cleaning-up"); + }); + + it("stays quiet when no sandbox was taken and there is nothing to tear down", async () => { + // Without a sandbox to delete, `NOOP_TEARDOWN` returns immediately and a + // `cleaning-up` label would flash for a frame and read as a glitch. + const appId = seedTestableApp("app"); + readSettingsMock.mockImplementation(() => ({ + ...structuredClone(DEFAULT_SETTINGS), + disableSandboxedE2eTests: true, + })); + runningApps.set(appId, { proxyUrl: "http://localhost:32100" } as any); + prepareIsolatedTestDatabaseMock.mockResolvedValue({ + isolation: { mode: "none" }, + teardown: vi.fn().mockResolvedValue({ + envRestored: true, + remoteCleanupCompleted: true, + }), + }); + + try { + await runAppTestsWithIsolation({ + event: { sender: {} } as any, + appId, + source: "panel", + }); + } finally { + runningApps.clear(); + } + + expect(createE2eTestWorkspaceMock).not.toHaveBeenCalled(); expect(runStates()).not.toContain("cleaning-up"); }); @@ -641,7 +788,10 @@ describe("tests handlers", () => { const appId = seedTestableApp("app"); prepareIsolatedTestDatabaseMock.mockResolvedValue({ isolation: { mode: "none" }, - teardown: vi.fn().mockResolvedValue({ envRestored: true }), + teardown: vi.fn().mockResolvedValue({ + envRestored: true, + remoteCleanupCompleted: true, + }), }); await runAppTestsWithIsolation({ @@ -664,7 +814,10 @@ describe("tests handlers", () => { let resolveFirstPrepare!: (value: { isolation: { mode: "neon-branch" }; infraError: { message: string }; - teardown: () => Promise<{ envRestored: boolean }>; + teardown: () => Promise<{ + envRestored: boolean; + remoteCleanupCompleted: boolean; + }>; }) => void; prepareIsolatedTestDatabaseMock .mockReturnValueOnce( @@ -675,7 +828,10 @@ describe("tests handlers", () => { .mockResolvedValueOnce({ isolation: { mode: "none" }, infraError: { message: "second run stopped before execution" }, - teardown: vi.fn().mockResolvedValue({ envRestored: true }), + teardown: vi.fn().mockResolvedValue({ + envRestored: true, + remoteCleanupCompleted: true, + }), }); const firstRun = runAppTestsWithIsolation({ @@ -695,13 +851,25 @@ describe("tests handlers", () => { resolveFirstPrepare({ isolation: { mode: "neon-branch" }, infraError: { message: "first run stopped before execution" }, - teardown: vi.fn().mockResolvedValue({ envRestored: true }), + teardown: vi.fn().mockResolvedValue({ + envRestored: true, + remoteCleanupCompleted: true, + }), }); await Promise.all([firstRun, secondRun]); - expect(runStates()).not.toContain("stopping"); - expect(runStates()).not.toContain("cleaning-up"); + // The superseded run must contribute no progress at all. The replacement + // legitimately announces its own sandbox deletion, so the assertion is + // scoped to the first run's generation rather than to the whole stream. + const supersededRunId = Math.min( + ...runStatePayloads().map((payload) => payload.runId), + ); + const supersededStates = runStatePayloads() + .filter((payload) => payload.runId === supersededRunId) + .map((payload) => payload.state); + expect(supersededStates).not.toContain("stopping"); + expect(supersededStates).not.toContain("cleaning-up"); }); it("attributes a queued run's stop to its own generation", async () => { @@ -709,7 +877,10 @@ describe("tests handlers", () => { let resolveFirstPrepare!: (value: { isolation: { mode: "neon-branch" }; infraError: { message: string }; - teardown: () => Promise<{ envRestored: boolean }>; + teardown: () => Promise<{ + envRestored: boolean; + remoteCleanupCompleted: boolean; + }>; }) => void; prepareIsolatedTestDatabaseMock .mockReturnValueOnce( @@ -720,7 +891,10 @@ describe("tests handlers", () => { .mockResolvedValueOnce({ isolation: { mode: "none" }, infraError: { message: "queued run stopped before execution" }, - teardown: vi.fn().mockResolvedValue({ envRestored: true }), + teardown: vi.fn().mockResolvedValue({ + envRestored: true, + remoteCleanupCompleted: true, + }), }); const firstRun = runAppTestsWithIsolation({ @@ -755,7 +929,10 @@ describe("tests handlers", () => { resolveFirstPrepare({ isolation: { mode: "neon-branch" }, infraError: { message: "first run superseded" }, - teardown: vi.fn().mockResolvedValue({ envRestored: true }), + teardown: vi.fn().mockResolvedValue({ + envRestored: true, + remoteCleanupCompleted: true, + }), }); await Promise.all([firstRun, secondRun]); }); diff --git a/src/ipc/handlers/tests_handlers.ts b/src/ipc/handlers/tests_handlers.ts index 417092e686..029cf2c8e1 100644 --- a/src/ipc/handlers/tests_handlers.ts +++ b/src/ipc/handlers/tests_handlers.ts @@ -595,16 +595,18 @@ export async function runAppTestsCore({ } /** - * Docker/cloud fallback. The run-scoped sandbox server is host-only for now, so - * these runtimes keep the pre-sandbox path — bootstrap and run against the - * normal preview — with the missing runtime isolation disclosed on the result - * rather than losing E2E testing entirely. Neon apps are the one exception: - * without a sandbox there is no throwaway branch to point the app at, so the - * only way to run would be against the user's real database. + * The non-sandboxed path, taken when the sandbox isn't available (Docker/cloud + * runtime) or the user opted out of it. Keeps the pre-sandbox behavior — + * bootstrap and run against the normal preview — with the missing runtime + * isolation disclosed on the result rather than losing E2E testing entirely. + * Neon apps are the one exception: without a sandbox there is no throwaway + * branch to point the app at, so the only way to run would be against the + * user's real database, and this fails closed instead. */ async function runTestsAgainstNormalPreview({ appId, - runtimeMode, + disclosure, + neonRefusal, signal, emit, emitProgress, @@ -617,7 +619,10 @@ async function runTestsAgainstNormalPreview({ timeoutMs, }: { appId: number; - runtimeMode: string; + /** Why this run isn't sandboxed, shown on the result's isolation badge. */ + disclosure: string; + /** Why a Neon app can't run at all here, and what to change. */ + neonRefusal: string; signal: AbortSignal; emit: (chunk: string, phase: "setup" | "running") => void; emitProgress: ( @@ -659,13 +664,8 @@ async function runTestsAgainstNormalPreview({ return { appId, results: [], - infraError: { - message: `Isolated E2E test servers aren't available in ${runtimeMode} runtime yet, and Dyad won't run Neon tests against your real database. Switch to host runtime to run tests for this app.`, - }, - isolation: { - mode: "none" as const, - reason: `Sandboxed E2E execution is not available in ${runtimeMode} runtime yet.`, - }, + infraError: { message: neonRefusal }, + isolation: { mode: "none" as const, reason: disclosure }, }; } @@ -674,16 +674,16 @@ async function runTestsAgainstNormalPreview({ prepared = await prepareIsolatedTestDatabase({ app, emit, - runtimeMode, + // Nothing here depends on the sandbox, and the Neon branch path — + // the only branch that reads this — was refused above. + runtimeMode: "host", signal, }); // Disclose the missing runtime sandbox, without overwriting a more // specific provider reason (e.g. the Supabase publishable-key hint). const isolation: TestIsolation = { ...prepared.isolation, - reason: - prepared.isolation.reason ?? - `Tests run against your normal preview because isolated test servers aren't available in ${runtimeMode} runtime yet.`, + reason: prepared.isolation.reason ?? disclosure, }; if (prepared.infraError) { return { @@ -713,7 +713,9 @@ async function runTestsAgainstNormalPreview({ emitProgress("cleaning-up", prepared.isolation); } onIsolationCleanupFailed(true); - onIsolationCleanupFailed(!(await prepared.teardown()).envRestored); + onIsolationCleanupFailed( + !(await prepared.teardown()).remoteCleanupCompleted, + ); } catch (error) { logger.error( `Failed to tear down isolated test environment for app ${appId}: ${error}`, @@ -945,12 +947,32 @@ export async function runAppTestsWithIsolation({ return finalResult; } - const runtimeMode = readSettings().runtimeMode2 ?? "host"; - if (runtimeMode !== "host") { + // The sandbox is host-only for now, and snapshotting the app plus its + // node_modules is a real copy on filesystems without reflink support — so + // the user gets an explicit opt-out. Both routes take the same + // non-sandboxed path, and both fail closed for Neon rather than running + // against the user's real database. + const settings = readSettings(); + const runtimeMode = settings.runtimeMode2 ?? "host"; + const sandboxUnavailable = + runtimeMode !== "host" + ? { + disclosure: `Tests run against your normal preview because isolated test servers aren't available in ${runtimeMode} runtime yet.`, + neonRefusal: `Isolated E2E test servers aren't available in ${runtimeMode} runtime yet, and Dyad won't run Neon tests against your real database. Switch to host runtime to run tests for this app.`, + } + : settings.disableSandboxedE2eTests + ? { + disclosure: + "Tests run against your normal preview because isolated test servers are turned off in Settings.", + neonRefusal: + "Isolated test servers are turned off in Settings, and Dyad won't run Neon tests against your real database. Turn them back on to run tests for this app.", + } + : null; + if (sandboxUnavailable) { finalResult = withIsolationCleanupWarning( await runTestsAgainstNormalPreview({ appId, - runtimeMode, + ...sandboxUnavailable, signal: controller.signal, emit, emitProgress, @@ -1145,15 +1167,21 @@ export async function runAppTestsWithIsolation({ // still require their guaranteed teardown. if (prepared) { try { - // Announce the teardown before it starts. It restores - // the temporary branch/user, takes no AbortSignal, and may - // outlast the process kill because Neon deletion retries with - // backoff. Skipped for `none`, whose teardown is a NOOP. + // Announce the teardown before it starts. It removes the + // temporary branch/user, takes no AbortSignal, and may outlast + // the process kill because Neon deletion retries with backoff. + // Skipped for `none`, whose teardown is a NOOP — the sandbox + // disposal below announces that case instead. if (prepared.isolation.mode !== "none") { emitProgress("cleaning-up", prepared.isolation); } isolationCleanupFailed = true; - isolationCleanupFailed = !(await prepared.teardown()).envRestored; + // NOT `envRestored`: the sandbox path never rewrites the real + // `.env.local`, so that flag only reports on a workspace file + // that is deleted seconds later. A leaked remote branch is the + // thing this warning actually describes. + isolationCleanupFailed = !(await prepared.teardown()) + .remoteCleanupCompleted; } catch (error) { logger.error( `Failed to tear down isolated test environment for app ${appId}: ${error}`, @@ -1166,6 +1194,19 @@ export async function runAppTestsWithIsolation({ finalResult = withIsolationCleanupWarning(finalResult); return finalResult; } catch (error) { + // A Stop pressed during sandbox setup escapes as a throw — the workspace + // copy and the test-server start both signal cancellation that way. That's + // an ordinary user cancellation, not an infrastructure failure: return the + // same structured result the in-run Stop path produces instead of rejecting + // the IPC call and recording an internal product exception for it. + if (controller.signal.aborted) { + finalResult = withIsolationCleanupWarning({ + appId, + results: [], + infraError: { message: "Test run stopped." }, + }); + return finalResult; + } // Surface an unexpected failure as an infra error on the run-state event so // the panel leaves its spinner state, then rethrow for the caller. finalResult = withIsolationCleanupWarning({ @@ -1185,6 +1226,13 @@ export async function runAppTestsWithIsolation({ }); } finally { if (workspace) { + // Deleting a cloned node_modules tree is tens of thousands of unlinks — + // slowest on Windows, where the copy was a real one. The results are + // already computed but the panel still has Run/Record/Delete disabled + // until `finished`, so label the wait for every isolation mode instead of + // leaving it unexplained (the provider teardown above only announces + // itself when there was provider state to remove). + emitProgress("cleaning-up", finalResult.isolation); try { await workspace.dispose(); } catch (error) { diff --git a/src/ipc/services/e2e_test_runtime.ts b/src/ipc/services/e2e_test_runtime.ts index c90cd5fa50..c72883b696 100644 --- a/src/ipc/services/e2e_test_runtime.ts +++ b/src/ipc/services/e2e_test_runtime.ts @@ -110,15 +110,19 @@ function delay(ms: number, signal?: AbortSignal): Promise { reject(new Error("Test run stopped.")); return; } - const timer = setTimeout(resolve, ms); - signal?.addEventListener( - "abort", - () => { - clearTimeout(timer); - reject(new Error("Test run stopped.")); - }, - { once: true }, - ); + // `{ once: true }` only removes the listener when it FIRES. The readiness + // poll calls this up to ~480 times per run, so without an explicit removal + // on the normal path every poll leaves a listener (and its timer closure) + // on the run's signal, and Node logs MaxListenersExceededWarning past 10. + const onAbort = () => { + clearTimeout(timer); + reject(new Error("Test run stopped.")); + }; + const timer = setTimeout(() => { + signal?.removeEventListener("abort", onAbort); + resolve(); + }, ms); + signal?.addEventListener("abort", onAbort, { once: true }); }); } diff --git a/src/ipc/services/e2e_test_workspace.test.ts b/src/ipc/services/e2e_test_workspace.test.ts index 69b81527bf..eee04c0723 100644 --- a/src/ipc/services/e2e_test_workspace.test.ts +++ b/src/ipc/services/e2e_test_workspace.test.ts @@ -8,8 +8,10 @@ vi.mock("@/paths/paths", () => ({ getUserDataPath: vi.fn() })); import { getUserDataPath } from "@/paths/paths"; import { createE2eTestWorkspace, + E2E_TEST_ARTIFACT_DIR, E2E_TEST_SANDBOX_DIR, reconcileOrphanE2eTestWorkspaces, + removeE2eTestArtifactsForApp, retainE2eTestArtifacts, rewriteE2eArtifactPath, shouldCopyE2eWorkspacePath, @@ -162,6 +164,58 @@ describe("E2E test workspace", () => { await expect(fs.stat(live.workspacePath)).rejects.toThrow(); }); + it("prunes artifacts for apps that no longer exist", async () => { + // Nothing else ever removes these: they're replaced only by the next run + // of the same app, which never comes once the app is deleted. + const root = await tempRoot(); + const userData = path.join(root, "user-data"); + vi.mocked(getUserDataPath).mockReturnValue(userData); + const artifactRoot = path.join(userData, E2E_TEST_ARTIFACT_DIR); + const kept = path.join(artifactRoot, "3-1-kept"); + const orphaned = path.join(artifactRoot, "9-1-orphaned"); + const unparseable = path.join(artifactRoot, "not-a-run"); + for (const dir of [kept, orphaned, unparseable]) { + await fs.mkdir(dir, { recursive: true }); + } + + await reconcileOrphanE2eTestWorkspaces({ knownAppIds: new Set([3]) }); + + expect((await fs.stat(kept)).isDirectory()).toBe(true); + await expect(fs.stat(orphaned)).rejects.toThrow(); + // Not ours to interpret, so it is left alone rather than guessed at. + expect((await fs.stat(unparseable)).isDirectory()).toBe(true); + }); + + it("leaves artifacts alone when the caller can't say which apps exist", async () => { + const root = await tempRoot(); + const userData = path.join(root, "user-data"); + vi.mocked(getUserDataPath).mockReturnValue(userData); + const artifact = path.join(userData, E2E_TEST_ARTIFACT_DIR, "9-1-run"); + await fs.mkdir(artifact, { recursive: true }); + + await reconcileOrphanE2eTestWorkspaces(); + + expect((await fs.stat(artifact)).isDirectory()).toBe(true); + }); + + it("drops one app's artifacts without touching another's", async () => { + const root = await tempRoot(); + const userData = path.join(root, "user-data"); + vi.mocked(getUserDataPath).mockReturnValue(userData); + const artifactRoot = path.join(userData, E2E_TEST_ARTIFACT_DIR); + const deleted = path.join(artifactRoot, "9-1-run"); + const other = path.join(artifactRoot, "10-1-run"); + for (const dir of [deleted, other]) { + await fs.mkdir(dir, { recursive: true }); + } + + await removeE2eTestArtifactsForApp(9); + + await expect(fs.stat(deleted)).rejects.toThrow(); + // A prefix match, not a substring match: "10-" must survive removing 9. + expect((await fs.stat(other)).isDirectory()).toBe(true); + }); + it("uses a root-based exclusion policy", () => { const appPath = path.resolve("app"); expect( diff --git a/src/ipc/services/e2e_test_workspace.ts b/src/ipc/services/e2e_test_workspace.ts index 5a220f3dc5..f8f5c1c656 100644 --- a/src/ipc/services/e2e_test_workspace.ts +++ b/src/ipc/services/e2e_test_workspace.ts @@ -4,9 +4,17 @@ import { randomUUID } from "node:crypto"; import log from "electron-log"; import { getUserDataPath } from "@/paths/paths"; +import { sendTelemetryEvent } from "@/ipc/utils/telemetry"; const logger = log.scope("e2e_test_workspace"); +// COPYFILE_FICLONE only clones on reflink-capable filesystems (APFS, btrfs, +// XFS); on ext4 and on Windows — where the mode isn't passed at all — this +// degrades to a full byte-for-byte copy of the app and its dependency tree on +// every run. Report timings and entry counts (never absolute paths) so the cost +// on non-reflink filesystems is measurable in the field. +const REFLINK_REQUESTED = process.platform !== "win32"; + const EXCLUDED_ROOTS = new Set([ ".git", "node_modules", @@ -61,6 +69,7 @@ async function copyNodeModules( appPath: string, workspacePath: string, signal?: AbortSignal, + countEntry?: () => void, ) { const source = path.join(appPath, "node_modules"); try { @@ -83,7 +92,11 @@ async function copyNodeModules( ...(process.platform === "win32" ? {} : { mode: fsConstants.COPYFILE_FICLONE }), - filter: () => !signal?.aborted, + filter: () => { + if (signal?.aborted) return false; + countEntry?.(); + return true; + }, }); if (signal?.aborted) throw new Error("Test run stopped."); } @@ -135,13 +148,21 @@ export async function createE2eTestWorkspace({ if (disposed) return; disposed = true; assertOwnedPath(sandboxRoot, workspacePath); + const startedAt = Date.now(); try { await fs.rm(workspacePath, { recursive: true, force: true }); + sendTelemetryEvent("e2e_test_workspace_disposed", { + duration_ms: Date.now() - startedAt, + platform: process.platform, + }); } finally { activeWorkspaceNames.delete(runName); } }; + let sourceEntries = 0; + let dependencyEntries = 0; + const startedAt = Date.now(); try { await fs.cp(appPath, workspacePath, { recursive: true, @@ -151,12 +172,26 @@ export async function createE2eTestWorkspace({ : { mode: fsConstants.COPYFILE_FICLONE }), filter: (candidatePath) => { if (signal?.aborted) return false; - return shouldCopyE2eWorkspacePath(appPath, candidatePath); + if (!shouldCopyE2eWorkspacePath(appPath, candidatePath)) return false; + sourceEntries += 1; + return true; }, }); if (signal?.aborted) throw new Error("Test run stopped."); + const sourceMs = Date.now() - startedAt; onProgress?.("Cloning installed dependencies into the test workspace…\n"); - await copyNodeModules(appPath, workspacePath, signal); + await copyNodeModules(appPath, workspacePath, signal, () => { + dependencyEntries += 1; + }); + sendTelemetryEvent("e2e_test_workspace_created", { + duration_ms: Date.now() - startedAt, + source_ms: sourceMs, + dependencies_ms: Date.now() - startedAt - sourceMs, + source_entries: sourceEntries, + dependency_entries: dependencyEntries, + reflink_requested: REFLINK_REQUESTED, + platform: process.platform, + }); return { workspacePath, artifactPath, dispose }; } catch (error) { await dispose(); @@ -196,34 +231,82 @@ export function rewriteE2eArtifactPath( return path.join(artifactPath, relative); } -/** - * Remove sandboxes left behind by a crash. Deletes run directories one by one - * and skips any run this process still owns, rather than removing the shared - * root: the sweep is fire-and-forget from startup and removing a multi-gigabyte - * tree is not instantaneous, so a Run pressed right after launch would - * otherwise be deleted mid-copy and surface as an unexplained ENOENT. - */ -export async function reconcileOrphanE2eTestWorkspaces(): Promise { - const sandboxRoot = path.join(getUserDataPath(), E2E_TEST_SANDBOX_DIR); +/** Run directory names start with `-`; recover the id from one. */ +function runDirectoryAppId(name: string): number | null { + const [prefix] = name.split("-"); + const appId = Number(prefix); + return prefix !== "" && Number.isInteger(appId) ? appId : null; +} + +async function removeRunDirectories( + root: string, + shouldRemove: (name: string) => boolean, + label: string, +): Promise { let entries; try { - entries = await fs.readdir(sandboxRoot, { withFileTypes: true }); + entries = await fs.readdir(root, { withFileTypes: true }); } catch (error: any) { if (error?.code !== "ENOENT") { - logger.warn(`Failed to list abandoned E2E test workspaces: ${error}`); + logger.warn(`Failed to list abandoned E2E ${label}: ${error}`); } return; } for (const entry of entries) { - if (activeWorkspaceNames.has(entry.name)) continue; - const runPath = path.join(sandboxRoot, entry.name); + if (!shouldRemove(entry.name)) continue; + const runPath = path.join(root, entry.name); try { - assertOwnedPath(sandboxRoot, runPath); + assertOwnedPath(root, runPath); await fs.rm(runPath, { recursive: true, force: true }); } catch (error) { logger.warn( - `Failed to remove abandoned E2E test workspace ${entry.name}: ${error}`, + `Failed to remove abandoned E2E ${label} ${entry.name}: ${error}`, ); } } } + +/** Drop every retained artifact directory belonging to one app. */ +export async function removeE2eTestArtifactsForApp( + appId: number, +): Promise { + await removeRunDirectories( + path.join(getUserDataPath(), E2E_TEST_ARTIFACT_DIR), + (name) => runDirectoryAppId(name) === appId, + "test artifacts", + ); +} + +/** + * Remove sandboxes and artifacts left behind by a crash or a deleted app. + * + * Sandboxes are deleted one run directory at a time, skipping any run this + * process still owns, rather than by removing the shared root: the sweep is + * fire-and-forget from startup and removing a multi-gigabyte tree is not + * instantaneous, so a Run pressed right after launch would otherwise be deleted + * mid-copy and surface as an unexplained ENOENT. + * + * Artifacts are otherwise only replaced by the next run of the same app, so + * without `knownAppIds` a deleted app's screenshots and traces would sit in + * user data forever with no surface that shows they exist. + */ +export async function reconcileOrphanE2eTestWorkspaces({ + knownAppIds, +}: { knownAppIds?: ReadonlySet } = {}): Promise { + const userDataPath = getUserDataPath(); + await removeRunDirectories( + path.join(userDataPath, E2E_TEST_SANDBOX_DIR), + (name) => !activeWorkspaceNames.has(name), + "test workspaces", + ); + if (!knownAppIds) return; + await removeRunDirectories( + path.join(userDataPath, E2E_TEST_ARTIFACT_DIR), + (name) => { + const appId = runDirectoryAppId(name); + // An unparseable name isn't ours to interpret; leave it alone. + return appId !== null && !knownAppIds.has(appId); + }, + "test artifacts", + ); +} diff --git a/src/ipc/services/isolated_test_db.ts b/src/ipc/services/isolated_test_db.ts index 749f634fc2..363ea4d74b 100644 --- a/src/ipc/services/isolated_test_db.ts +++ b/src/ipc/services/isolated_test_db.ts @@ -76,6 +76,14 @@ export interface TeardownResult { * rather than quietly starting the user's app against isolated data. */ envRestored: boolean; + /** + * False when a remote resource this run created is still out there — today, + * a temporary Neon branch whose delete failed and stays tracked for the + * startup sweep to retry. The E2E sandbox path never modifies the real env, + * so `envRestored` says nothing there; this is the flag that means "the user + * has something left over". + */ + remoteCleanupCompleted: boolean; } export interface PreparedIsolation { @@ -106,8 +114,8 @@ export interface PreparedIsolation { type EmitOutput = (chunk: string, phase: "setup" | "running") => void; const NOOP_TEARDOWN = async () => { - // No isolation was set up, so there is nothing to restore. - return { envRestored: true }; + // No isolation was set up, so there is nothing to restore or delete. + return { envRestored: true, remoteCleanupCompleted: true }; }; /** @@ -211,15 +219,19 @@ export async function prepareIsolatedTestDatabase({ // it, and the row's id is what the startup sweep reconciles from. App // deletion — the one case where that row is about to disappear — handles the // branch itself, after the deletion commits. + let remoteCleanupCompleted = true; if (branchId && envRestored) { // Shared with the recovery path in `neon_test_branch`: the cleanup-only // marker is written before the fallible remote delete, so a crash in // between leaves a row that says the env is real and only the branch is // outstanding. Both callers must encode that ordering identically or // teardown and recovery drift apart. - await markAndDeleteTempTestBranch(app, branchId); + remoteCleanupCompleted = await markAndDeleteTempTestBranch(app, branchId); + } else if (branchId) { + // Deliberately kept, but still outstanding from the user's perspective. + remoteCleanupCompleted = false; } - return { envRestored }; + return { envRestored, remoteCleanupCompleted }; }; try { @@ -330,8 +342,9 @@ export async function prepareIsolatedTestDatabase({ // `NOOP_TEARDOWN` — a no-op answers "restored" and would let the app be // relaunched against the temporary branch. let envRestored = false; + let remoteCleanupCompleted = false; try { - envRestored = (await teardown()).envRestored; + ({ envRestored, remoteCleanupCompleted } = await teardown()); } catch (teardownError) { logger.error( `Teardown failed during error recovery for app ${app.id}: ${teardownError}`, @@ -340,6 +353,7 @@ export async function prepareIsolatedTestDatabase({ // Already torn down; this only carries the verdict to whoever asks later. const settledTeardown = async (): Promise => ({ envRestored, + remoteCleanupCompleted, }); // A user Stop surfaces here too (waitForServerReady & co. throw on abort). // That's a deliberate cancellation, not an infra failure — don't show the @@ -401,6 +415,7 @@ async function prepareSupabaseTestUserIsolation({ // Nothing here touches `.env.local` — the Supabase path isolates by test user, // not by swapping the app's database — so the environment is never at risk. const teardown = async (): Promise => { + let remoteCleanupCompleted = true; if (testUser) { try { await deleteTempTestUser({ @@ -408,12 +423,13 @@ async function prepareSupabaseTestUserIsolation({ supabaseTestUserId: testUser.userId, }); } catch (error) { + remoteCleanupCompleted = false; logger.error( `Failed to delete isolated Supabase test user ${testUser.userId} for app ${app.id}: ${error}`, ); } } - return { envRestored: true }; + return { envRestored: true, remoteCleanupCompleted }; }; try { diff --git a/src/ipc/utils/neon_test_branch.test.ts b/src/ipc/utils/neon_test_branch.test.ts index 82cee98190..dcff4616eb 100644 --- a/src/ipc/utils/neon_test_branch.test.ts +++ b/src/ipc/utils/neon_test_branch.test.ts @@ -375,7 +375,9 @@ describe("markAndDeleteTempTestBranch", () => { expect(mocks.deleteProjectBranch).toHaveBeenCalledWith("proj-1", "test-br"); }); - it("does not throw when the remote delete fails", async () => { + it("reports the failure without throwing when the remote delete fails", async () => { + // Callers promise the user "Dyad will retry remote cleanup on next + // startup", so they need the verdict — but a teardown must never throw. mocks.deleteProjectBranch.mockRejectedValueOnce({ response: { status: 500 }, }); @@ -384,7 +386,16 @@ describe("markAndDeleteTempTestBranch", () => { makeApp({ neonTestBranchId: "test-br" }), "test-br", ), - ).resolves.toBeUndefined(); + ).resolves.toBe(false); + }); + + it("reports success when the branch is gone", async () => { + await expect( + markAndDeleteTempTestBranch( + makeApp({ neonTestBranchId: "test-br" }), + "test-br", + ), + ).resolves.toBe(true); }); }); diff --git a/src/ipc/utils/neon_test_branch.ts b/src/ipc/utils/neon_test_branch.ts index a7f7f83d40..7ba3b32593 100644 --- a/src/ipc/utils/neon_test_branch.ts +++ b/src/ipc/utils/neon_test_branch.ts @@ -352,7 +352,7 @@ export async function deleteTempTestBranch(appData: AppRow): Promise { export async function markAndDeleteTempTestBranch( appData: AppRow, branchId: string, -): Promise { +): Promise { // `deleteTempTestBranch` reads the marker off the row it is given, and the // caller's copy is stale by now, so carry the branch we actually created. let cleanupApp: AppRow = { ...appData, neonTestBranchId: branchId }; @@ -364,11 +364,15 @@ export async function markAndDeleteTempTestBranch( ); } try { - await deleteTempTestBranch(cleanupApp); + // Still best-effort — never throws — but the verdict is reported now. + // Callers that promise the user "Dyad will retry remote cleanup on next + // startup" need to know whether the branch actually leaked. + return await deleteTempTestBranch(cleanupApp); } catch (error) { logger.error( `Failed to delete temporary test branch ${trackedBranchId(branchId)} for app ${appData.id}: ${error}`, ); + return false; } } diff --git a/src/lib/schemas.ts b/src/lib/schemas.ts index f69c291f0f..28949b51c2 100644 --- a/src/lib/schemas.ts +++ b/src/lib/schemas.ts @@ -435,6 +435,13 @@ const BaseUserSettingsFields = { // preference. Default (unset) is headless + serial. testHeaded: z.boolean().optional(), testParallel: z.boolean().optional(), + // Escape hatch for the sandboxed E2E runtime, which snapshots the app plus + // its node_modules per run. On filesystems without reflink support (ext4, + // Windows) that snapshot is a real copy and can be slow. Off by default — + // the sandbox is the intended path — and turning it on runs the tests + // against the normal preview with the missing isolation disclosed. Neon apps + // are refused rather than run against the real database either way. + disableSandboxedE2eTests: z.boolean().optional(), autoExpandPreviewPanel: z.boolean().optional(), enableChatEventNotifications: z.boolean().optional(), blockUnsafeNpmPackages: z.boolean().optional(), diff --git a/src/lib/settingsSearchIndex.ts b/src/lib/settingsSearchIndex.ts index 4feac26a7e..09148d68e6 100644 --- a/src/lib/settingsSearchIndex.ts +++ b/src/lib/settingsSearchIndex.ts @@ -26,6 +26,7 @@ export const SETTING_IDS = { keepPreviewsRunning: "setting-keep-previews-running", appBlueprint: "setting-app-blueprint", testingForNewApps: "setting-testing-for-new-apps", + sandboxedE2eTests: "setting-sandboxed-e2e-tests", chatEventNotification: "setting-chat-event-notification", maxChatTurns: "setting-max-chat-turns", maxToolCallSteps: "setting-max-tool-call-steps", @@ -185,6 +186,25 @@ export const SETTINGS_SEARCH_INDEX: SearchableSettingItem[] = [ sectionId: SECTION_IDS.workflow, sectionLabel: "Workflow", }, + { + id: SETTING_IDS.sandboxedE2eTests, + label: "Run E2E Tests in an Isolated Sandbox", + description: + "Copy the app into a throwaway workspace with its own server so test runs never touch your preview or real data", + keywords: [ + "testing", + "tests", + "e2e", + "sandbox", + "isolated", + "copy", + "node_modules", + "slow", + "workflow", + ], + sectionId: SECTION_IDS.workflow, + sectionLabel: "Workflow", + }, { id: SETTING_IDS.autoExpandPreview, label: "Auto Expand Preview", diff --git a/src/main.ts b/src/main.ts index 129908a31d..abd7870e4a 100644 --- a/src/main.ts +++ b/src/main.ts @@ -459,7 +459,16 @@ export async function onReady() { // must not block startup. void reconcileOrphanTestBranches(); void reconcileOrphanTestUsers(); - void reconcileOrphanE2eTestWorkspaces(); + // Also prunes retained test artifacts whose app no longer exists — nothing + // else ever removes them, and the user has no surface that shows they exist. + void (async () => { + const rows = await db.query.apps.findMany({ columns: { id: true } }); + await reconcileOrphanE2eTestWorkspaces({ + knownAppIds: new Set(rows.map((row) => row.id)), + }); + })().catch((error) => + logger.error("Failed to reconcile abandoned E2E test workspaces", error), + ); // Cleanup old ai_messages_json entries to prevent database bloat cleanupOldAiMessagesJson(); diff --git a/src/pages/settings.tsx b/src/pages/settings.tsx index bf7522136c..f36b30eb89 100644 --- a/src/pages/settings.tsx +++ b/src/pages/settings.tsx @@ -21,6 +21,7 @@ import { Button } from "@/components/ui/button"; import { cn } from "@/lib/utils"; import { AppBlueprintSwitch } from "@/components/AppBlueprintSwitch"; import { TestingForNewAppsSwitch } from "@/components/TestingForNewAppsSwitch"; +import { SandboxedE2eTestsSwitch } from "@/components/SandboxedE2eTestsSwitch"; import { AutoExpandPreviewSwitch } from "@/components/AutoExpandPreviewSwitch"; import { KeepPreviewsRunningSwitch } from "@/components/KeepPreviewsRunningSwitch"; import { ChatEventNotificationSwitch } from "@/components/ChatEventNotificationSwitch"; @@ -526,6 +527,17 @@ export function WorkflowSettings() {

+
+ +

+ Run each E2E test in a throwaway copy of your app with its own server, + so tests never touch your preview or your real database. Turn this off + if copying your dependencies makes runs slow — tests then run against + your normal preview, and apps using Neon won't run at all rather than + test against your real data. +

+
+

From f6f274ada3ed8495f84d66231db5cf38e6f59d42 Mon Sep 17 00:00:00 2001 From: Mohamed Aziz Mejri Date: Sun, 23 Aug 2026 21:24:15 +0000 Subject: [PATCH 4/9] Address third round of PR review comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Stop handing the agent an unreadable artifact path. A sandboxed run retains error-context.md under /test-artifacts, which read_file's safeJoin refuses, so the model's first diagnostic step always failed. The page snapshot is now inlined (bounded, same containment guards as the screenshot reader) and the traversal path is no longer printed at all. - Make the dev-server gate conditional on the non-sandboxed path. A sandboxed run serves its own copy of the app on its own port, so requiring the user's preview blocked the feature's main benefit and contradicted the panel's own "your preview keeps running" disclosure. The panel banner, Run/Retry state and the agent's guardDevServerRunning now share one helper, usesSandboxedE2eTests. - Thread `sandboxed` through the run-state payload so cleanup copy only claims a sandbox when one was taken. The fallback path creates no workspace, and "Cleaning up the test sandbox…" there was the same inaccuracy the previous round removed. Adds a separate cancellationCleaningTestSandbox locale key. - Throw DyadError with DyadErrorKind.Precondition for expected setup failures (dependencies not installed, server never became ready), so ordinary user-fixable problems stop being reported to PostHog as product exceptions. - Add allowCompatibleQueueBypass to the prepare-e2e-test-workspace claim, matching the run stage: work conflicting only on test-files should not queue behind a test run that is itself blocked on something unrelated. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01JhzboMdTdVq829U81ZCiNb --- src/atoms/testRuntimeAtoms.ts | 8 ++ src/components/chat/CancellationBanner.tsx | 4 +- .../preview_panel/TestsPanel.test.tsx | 53 +++++++ src/components/preview_panel/TestsPanel.tsx | 31 ++-- src/hooks/useTestRunEvents.ts | 1 + src/i18n/locales/en/chat.json | 3 +- src/i18n/locales/es/chat.json | 3 +- src/i18n/locales/ko/chat.json | 3 +- src/i18n/locales/pt-BR/chat.json | 3 +- src/i18n/locales/zh-CN/chat.json | 3 +- src/ipc/handlers/tests_handlers.test.ts | 4 + src/ipc/handlers/tests_handlers.ts | 38 +++-- src/ipc/services/e2e_test_runtime.ts | 13 +- src/ipc/services/e2e_test_workspace.ts | 6 +- src/ipc/types/tests.ts | 7 + src/ipc/utils/test_screenshot.ts | 136 ++++++++++++++---- src/lib/e2eSandbox.ts | 22 +++ .../local_agent/tools/run_tests.spec.ts | 73 +++++++++- .../handlers/local_agent/tools/run_tests.ts | 69 ++++++--- 19 files changed, 404 insertions(+), 76 deletions(-) create mode 100644 src/lib/e2eSandbox.ts diff --git a/src/atoms/testRuntimeAtoms.ts b/src/atoms/testRuntimeAtoms.ts index e4fb06a206..3500a39182 100644 --- a/src/atoms/testRuntimeAtoms.ts +++ b/src/atoms/testRuntimeAtoms.ts @@ -72,6 +72,12 @@ export interface TestRunState { * a run completes. */ isolation?: TestIsolation; + /** + * Whether the run executed in an isolated sandbox. Drives the cleanup copy: + * the fallback path (Docker/cloud runtime, or the user's opt-out) creates no + * workspace, so there is no sandbox to claim Dyad is deleting. + */ + sandboxed?: boolean; startedAt?: number; } @@ -275,6 +281,8 @@ export const applyTestRunStartedAtom = atom( ), runError: undefined, isolation: undefined, + // Reported by the main process once the run has picked its path. + sandboxed: undefined, startedAt: startedAt ?? Date.now(), }), }); diff --git a/src/components/chat/CancellationBanner.tsx b/src/components/chat/CancellationBanner.tsx index 23db686588..ff20323dd5 100644 --- a/src/components/chat/CancellationBanner.tsx +++ b/src/components/chat/CancellationBanner.tsx @@ -39,7 +39,9 @@ export function CancellationBanner({ appId }: { appId?: number | null }) { : runState.phase === "cleaning-up" ? runState.isolation?.mode === "neon-branch" ? t("cancellationRemovingTestDatabase") - : t("cancellationCleaningTestData") + : runState.sandboxed + ? t("cancellationCleaningTestSandbox") + : t("cancellationCleaningTestData") : runState.phase === "stopping" ? t("cancellationEndingTestRun") : null; diff --git a/src/components/preview_panel/TestsPanel.test.tsx b/src/components/preview_panel/TestsPanel.test.tsx index 7bc831378c..09ff14abee 100644 --- a/src/components/preview_panel/TestsPanel.test.tsx +++ b/src/components/preview_panel/TestsPanel.test.tsx @@ -318,6 +318,43 @@ describe("TestsPanel", () => { ).toBe(true); }); + it("runs sandboxed tests without the preview being up", async () => { + // A sandboxed run serves its own copy of the app on its own port. Requiring + // the user's preview would block the whole point of the feature — and would + // contradict the panel's own "your preview keeps running" disclosure. + mocks.appUrl = null; + + renderPanel(); + + await screen.findByText("signup.spec.ts"); + expect(screen.queryByText("Start the app to run tests.")).toBeNull(); + expect( + ( + screen.getByRole("button", { + name: "Run all tests", + }) as HTMLButtonElement + ).disabled, + ).toBe(false); + }); + + it("still requires the preview when the run is not sandboxed", async () => { + // The fallback path runs Playwright against the user's preview, so the + // gate is still correct there. + mocks.appUrl = null; + mocks.settings = { disableSandboxedE2eTests: true }; + + renderPanel(); + + expect(await screen.findByText("Start the app to run tests.")).toBeTruthy(); + expect( + ( + screen.getByRole("button", { + name: "Run all tests", + }) as HTMLButtonElement + ).disabled, + ).toBe(true); + }); + describe("stopping a run", () => { /** Put the panel's app into `phase` as if a run had reached it. */ function setPhase( @@ -445,6 +482,7 @@ describe("TestsPanel", () => { setPhase(store, { phase: "cleaning-up", isolation: { mode: "supabase-test-user" }, + sandboxed: true, }); expect(screen.getByText(/Cleaning up the test sandbox/)).toBeTruthy(); @@ -455,6 +493,21 @@ describe("TestsPanel", () => { ).toContain("Cleaning up…"); }); + it("claims no sandbox when the run never took one", () => { + // The fallback path (docker/cloud runtime, or the opt-out) creates no + // workspace, so naming one would be the same inaccurate cleanup copy the + // Neon "restoring your preview" wording was. + const { store } = renderPanel(); + setPhase(store, { + phase: "cleaning-up", + isolation: { mode: "supabase-test-user" }, + sandboxed: false, + }); + + expect(screen.getByText(/Cleaning up the test data/)).toBeTruthy(); + expect(screen.queryByText(/test sandbox/)).toBeNull(); + }); + it("does not carry a completed run's stop latch into the next run", () => { const { store } = renderPanel(); setPhase(store, { phase: "running", startedAt: 1000 }); diff --git a/src/components/preview_panel/TestsPanel.tsx b/src/components/preview_panel/TestsPanel.tsx index d6d806822d..dc864f2b59 100644 --- a/src/components/preview_panel/TestsPanel.tsx +++ b/src/components/preview_panel/TestsPanel.tsx @@ -78,6 +78,7 @@ import { queryKeys } from "@/lib/queryKeys"; import { cn } from "@/lib/utils"; import { showError, showInfo, showSuccess } from "@/lib/toast"; import { findCaseResult, statusLabel, testKey } from "@/lib/testResultUtils"; +import { usesSandboxedE2eTests } from "@/lib/e2eSandbox"; import { usePreviewIframeController } from "@/preview_iframe/usePreviewIframe"; import { sameOriginStartPath } from "./previewAddressPath"; @@ -702,6 +703,13 @@ export function TestsPanel() { const parallel = settings?.testParallel ?? false; const devServerRunning = appUrl.appUrl !== null; + // A sandboxed run serves the app itself, on its own port, from its own copy — + // the user's preview is not involved, so requiring it would block the whole + // point of the feature. The fallback path (Docker/cloud runtime, or the + // opt-out) still runs Playwright against the preview and still needs it up. + // Recording is unaffected either way: it drives the live preview. + const testsNeedDevServer = !usesSandboxedE2eTests(settings); + const testRunBlocked = testsNeedDevServer && !devServerRunning; // Owns the run's whole lifecycle, teardown included. Gates every action that // must not interleave with it (Run, Record, Delete), because the per-app lock // is still held during `cleaning-up`. @@ -1416,13 +1424,13 @@ export function TestsPanel() { specs.length > 0 && (

)} - {/* Dev-server gate banner */} - {!devServerRunning && specs.length > 0 && ( + {/* Dev-server gate banner — only when the run actually needs it. */} + {testRunBlocked && specs.length > 0 && (
Start the app to run tests. @@ -1585,10 +1596,10 @@ export function TestsPanel() {