Retain leased mobile workers across attempts (#1531) by kblok · Pull Request #1552 · hardkoded/e2e-dotnet

kblok · 2026-11-11T00:57:54Z

Ports the mobile worker retention of tester-army/e2e #1209 (ef977f4856) and the tool-pixels change of #1437 (94e6e76548).

What changed

The engine keeps each worker slot's device across attempts. DeviceWorker (.NET-only) holds upstream's surface fields client, bound, and recording.
A leased attempt's DisposeAsync ends the attempt and keeps the device booted under its agent-device session (upstream's endAttempt). A device without a lease closes with its attempt, as before.
MobileEngine.FinishAsync closes the kept sessions and releases the leases together, as upstream's pool finish does.
A provider recording whose stop failed stays marked on its worker. The next attempt finishes it before it records, and blocks while it cannot stop. A native recording not confirmed stopped is reset by closing its session.
Ported tests: the 21 device provider recording its (one skipped), the two device provider its, the two provider-example its, and offers pixel verbs after a secret fill.

Upstream parity

Upstream change: tester-army/e2e ef977f4856 (#1209, mobile parts only: surface.ts, pool.ts, options.ts, bindings.ts, provider.test.ts, provider-example.test.ts, and the docs) and 94e6e76548 (#1437, tool-pixels.test.ts). The packages/e2e scheduler, worker, and protocol parts of #1209 are out of scope for this port.

Verdict: gaps remain. Three gaps are listed below. This PR is a draft until they are closed or accepted.

Counts (mobile parts):

Ported: the 21 recording its, the two device provider its, the tool-pixels it, and the provider-example its that are not listed as partial.
Partial: 1 (Partial_Hands_a_retained_cloud_grant_to_the_worker_and_releases_the_allocated_scope).
Renamed: 0.
Missing: 0 in the mobile tests. See gap 1 for the run path.
Port-only: the symbols below.

Partial and skipped tests:

Partial_Hands_a_retained_cloud_grant_to_the_worker_and_releases_the_allocated_scope: upstream's worker client holds only the lease scope. The port's client also holds the selectors and platform, because its worker resolves the device from them (see AgentDeviceLeasesTests). The allocation, the scope, and the release are checked as upstream checks them.
Skipped: refuses a provider whose record is not a function (the C# type says it).

.NET-only symbols and tests (each has a reason):

DeviceWorker, WorkerOf, DropWorker, TakeWorkers, DisposeWorkerAsync: the holder and the per-slot map. Upstream keeps these fields on one surface per worker.
Internal MobileEngine.DisposeAsync(timeout, reuse, ct): upstream's dispose({ reuseSignal }). The public IEngineSession has no reuse signal, so the engine reaches this through its internal overload.
DeviceSession.EndAttemptAsync: upstream's endAttempt. The Async suffix is the only name difference.
Harness.Boot attemptId, and MobileRun (Helpers/MobileRun.cs): test helpers that run in-memory tests through Runner.RunAsync.
Tests: Reuses_the_leased_worker_across_attempts_through_the_standalone_host (the run's worker lifecycle through Standalone.OpenWorkerAsync), Stops_the_daemon_when_a_declined_retention_grant_names_the_cloud_in_its_message, and Keeps_the_agent_device_recording_for_a_provider_that_records_nothing_of_this_device.

Errors and contract sentences from #1209:

"agent-device did not acknowledge retainOnClose…": AgentDeviceLeases.AcquireAsync; covered by AgentDeviceLeasesTests and Releases_a_declined_retention_grant….
"a video is already recording", "startVideo outside an attempt", and "finished without naming a file": covered by DeviceProviderRecordingTests.
"Confirmed stops are not repeated if the returned artifact is invalid": covered by Clears_a_confirmed_provider_stop_before_rejecting_its_invalid_artifact.
"A failed provider stop alone does not close a healthy agent-device session during worker replacement": covered by Keeps_a_reusable_leased_session_when_the_provider_recording_still_cannot_stop_during_disposal.

Repo rules checked:

CLAUDE.md test layout: ProviderExample/, ToolPixels/, and Provider/ follow the upstream file names. The Partial_ prefix is used for the partial it.
CLAUDE.md docs: COMPATIBILITY.md, docs-site/.../get-started/mobile.mdx, and skills/e2e-dotnet/references/mobile.md are updated. The API reference was not regenerated because no public XML documentation changed.
CLAUDE.md real-model tests: see the testing section.
CLAUDE.md /code-review: run; the valid findings are fixed (lock on the close, cancellation not absorbed by the settle wait, finish ordering as upstream).

Gaps that remain (named plainly)

Test runs do not lease devices. The xUnit and NUnit fixtures and Runner.RunAsync never call a mobile engine's PrepareAsync or FinishAsync. Only the standalone host (e2e explore, e2e mcp, and the open session) prepares one. This predates this PR. Upstream's run prepares every target, so a real test run never reuses a leased worker. Closing it needs a process-level prepare and finish in the test fixtures.
IEngineSession.DisposeAsync has no reuse signal. Worker replacement is reachable only through the internal overload, and the port has no scheduler that replaces workers.
Out of scope: the packages/e2e scheduler, worker protocol, and engine index parts of Upstream test: ids.test.ts its for test id join, result id bytes, and title validation #1209.

Tests

dotnet build E2E.slnx: 0 warnings, 0 errors.
E2E.Mobile.Tests: 487 passed, 60 failed of 547. The 60 are the same failures as origin/main: CRLF and path separators (CapturedSnapshots, Installs_*, Asks_*), and LeaseDaemon timeouts. None of the new tests fail.
E2E.Limrun.Tests: 21 passed, 1 skipped. E2E.TestMu.Mobile.Tests: 99 passed. E2E.Mobile.DevFlow.Tests: 94 passed. E2E.Expo.Tests: 46 passed. E2E.BrowserStack.Tests: 51 passed, 2 failed (Path.GetFullPath on a non-rooted base path on Windows, already on origin/main).
Real-model tests (TestCategory=RealModel), run on the build before the last review fixes. The GitHub Copilot login succeeded through the GitHub CLI token. Passed: TodoMvc 20, E2E.Agent.Tests 28, WebBenchmark 33, E2E.Sample 1, E2E.XUnit.V3.Sample 1, E2E.NUnit.Tests 3. Not run: MobileApp.E2ETests (9 failed: the agent-device daemon could not start on this Windows host, before any device work) and WinAppBenchmark (62 failed: the app executable is not built in this worktree). Both are environment failures, and the mobile real-model tests need a simulator or emulator that this host does not have.

🤖 Generated with Claude Code

The engine keeps each worker slot's device across the attempts that run on it: DeviceWorker holds upstream's surface fields (client, bound, recording). An attempt's dispose keeps a leased device booted under its session; the run's finish closes the kept sessions, then releases the leases, as upstream's finish does. A provider recording whose stop failed stays marked on the worker and is finished before the next attempt records. Ports the device provider recording its (21 of 22, one skipped), the two device provider its, the provider-example its (one Partial_), and the tool-pixels it, which runs through Runner.RunAsync with a fake device and a scripted model. Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>

coderabbitai · 2026-11-11T00:58:01Z

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Matches upstream's cloudDevices example and its two provider-example its: - CloudDevicesOptions is { Provider, StateDir }, as upstream's cloudDevices({ provider, stateDir }). Each run's leases keep their state in <StateDir>/<runId>. The example no longer picks a device, OS version, or app, and no longer requires CLOUD_TOKEN, which upstream's it does not check. - The lease allocate call no longer sends ttlMs, as upstream's does not. The lease heartbeat still sends its ttl. - Selectors are optional on AgentDeviceLeaseRequest. A lease without them allocates only the scope and the platform, and its client carries only the scope, as upstream's client does. BrowserStack still names its selectors. - IAgentDeviceLeases.ReleaseAsync takes the DeviceReleaseContext the provider was handed, as upstream's release(lease, releaseContext) does. - The two Partial_ provider-example tests are full ports under upstream's names and assertions: "hands a retained cloud grant to the worker and releases the allocated scope" and "releases a declined retention grant before rejecting acquisition, without keeping it for release". The token check, which upstream does not have, is removed with its test. - "asks the provider for the resolved build..." is renamed to upstream's name and ports upstream's body, including the explicit installApp and apps.install assertions it was missing. Also adds the PreparedEngines run tests and the public API pages for IDisposableEngine and EngineDisposeContext, and updates the mobile docs for the new example shape. Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>

Navigation Menu

Repository navigation

Retain leased mobile workers across attempts (#1531) - #1552

Retain leased mobile workers across attempts (#1531)#1552
kblok wants to merge 2 commits into
mainfrom
fix/1531-mobile-worker-retention

kblok commented Nov 11, 2026

Uh oh!

coderabbitai Bot commented Nov 11, 2026

Draft PR not reviewed

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

kblok commented Nov 11, 2026

What changed

Upstream parity

Gaps that remain (named plainly)

Tests

Uh oh!

coderabbitai Bot commented Nov 11, 2026

Draft PR not reviewed

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant