Skip to content

feat: add optional Doublespeed provider plugin - #2112

Open
hassantsyed wants to merge 13 commits into
callstack:mainfrom
hassantsyed:feat/doublespeed-provider
Open

hassantsyed wants to merge 13 commits into
callstack:mainfrom
hassantsyed:feat/doublespeed-provider

Conversation

@hassantsyed

@hassantsyed hassantsyed commented Aug 28, 2026 •

Copy link
Copy Markdown

Summary

Ship Doublespeed as the optional @agent-device/doublespeed package, installed under AGENT_DEVICE_HOME. Core no longer bundles or activates it from credentials alone.

agent-device plugins add @agent-device/doublespeed
export DOUBLESPEED_API_KEY=...
agent-device connect doublespeed --platform ios

Preserves the existing simulator lifecycle, app deployment, interactions, snapshots and app-log runtime. Adds provider-owned connection callbacks, scoped Apple host helpers and a lazy shared WebDriver adapter for plugins. Plugins ship bundled ESM with no runtime or peer dependency on agent-device; host imports are type-only. The published plugin exports only its factory.

96 files including the original provider feature; migration also touches shared loading, SDK, packaging and help/docs. Independent Claude review led to removing loader hooks and duplicate core dependencies. Existing startup budgets remain unchanged; the base plugin declarations are 705 bytes.

Validation

  • Tested ca1a7c722e77d896f5e6b18c28e075696c40262e: pnpm check:affected --run passed all runnable checks.
  • Isolated packed npm installs: minimal exports, no second core installation, typed authentication failure and successful CLI connect against a local API fixture.
  • Error-brand regression fails without the fix; import-budget and test-size checks pass.
  • CI, coverage and current live-provider verification remain outstanding. Live credentials are unavailable here; the earlier live run at 8dad6500 predates this migration.

Adds `agent-device connect doublespeed`, a direct iOS-simulator provider
backed by the Doublespeed Mac fleet. The provider mirrors the Limrun
package: lease lifecycle with label-selector recovery, a session-API
interactor, content-addressed app deployment, and an ADR-0019 platform
runtime owner with durable app-log recovery. No new external dependency;
the kernel snapshot provenance table gains the `doublespeed-ios-tree`
producer.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@thymikee

Copy link
Copy Markdown
Member

Not ready at 8dad6500a3.

  • P1 — prevent paid simulator leaks during allocation failure. DoublespeedApiClient.createSimulator() creates the simulator and then waits for readiness, while DoublespeedRuntime.allocate() only enters its rollback block after createSimulator() returns. A failed status, timeout, or abort during readiness therefore loses the simulator id and never issues DELETE. Split creation from readiness or make the client own rollback/finally. Plant failed-status, timeout, and cancellation tests proving DELETE occurs and the primary error remains authoritative.
  • P1 — do not advertise semantics the provider does not implement. Facts mark snapshot custom actions available, but ios.ts ignores SnapshotOptions; double-tap is two independent HTTPS taps, and fill taps then types without clearing. Mark these unsupported unless Doublespeed exposes owning primitives, or implement the actual semantics with planted route tests and exact-head live evidence.
  • P2 — stream app uploads. uploadAsset() readFiles the entire zipped .app before PUT, doubling memory for archives that can be hundreds of MB. Stream it with bounded cancellation and prove abort/large-body behavior.
  • Design/size — materially reduce the duplicated provider stack before landing. This is ~3,081 net production lines and explicitly mirrors ~590 Limrun lines across nine modules. This PR itself creates the second consumer, so ADR-0019s condition for extracting honest shared ownership is met now; a later cleanup is not a sufficient smaller-design rejection. Extract focused provider-neutral session/app-log machinery into its owning package (or otherwise shrink this slice), itemize justified residual growth, and remove implementation/test-tour comments that narrate the code or justify workarounds.
  • Evidence — complete exact-head gates and failure-path proof. GitHub currently has no check rollup. The happy-path device run is useful, but it does not cover allocation cleanup failures/cancellation, durable log reattach after daemon restart, or the claimed custom-action/fill/double-tap semantics. Add the smallest practical exact-head evidence for those paths.

@thymikee

Copy link
Copy Markdown
Member

Current exact head 8dad650 is unchanged and now CONFLICTING with current main. The direct Doublespeed route and prior live evidence are worth preserving, but confirmed blockers remain: allocation failures before runtime.ts:193 bypass DELETE cleanup and can leak paid simulators; advertised snapshot options are ignored, doubleTap is two independent taps, and fill does not clear; uploads buffer the full archive; and ~3,081 net production lines duplicate Limrun seams instead of extracting the now-proven provider-neutral owner. With nine current-main conflicts, no linked issue, and old-base CI, do not rebase this 5k-line single commit wholesale. Build a fresh current-main stack: shared provider-neutral session/app-log owner, rollback-safe streamed Doublespeed lifecycle, then only accurately supported capabilities with exact failure-path evidence. Keep this PR as reference and close it once replacement exists.

@thymikee

thymikee commented Sep 4, 2026

Copy link
Copy Markdown
Member

Current-head re-review at 8dad6500a31178e4f11ec32fe981ce069aa75041: blocked beyond the conflict. Allocation readiness failures can leak a created, billable Simulator because rollback begins only after createSimulator() returns, while awaitReady() can fail, time out, or cancel after creation without retaining the id for DELETE. Split create/wait ownership or clean up inside the client while preserving the primary error, with failed-status, timeout, and cancellation tests. Runtime facts also overclaim custom-action snapshots, fill replacement, and atomic double-tap semantics that the adapter does not implement; mark them unsupported or implement them through the owning shared contracts with live proof. The branch also needs current-main conflict resolution and fresh exact-head checks.

@thymikee

thymikee commented Oct 2, 2026 •

Copy link
Copy Markdown
Member

We’re moving new integrations to managed npm provider plugins. The infrastructure is in #3121; Doublespeed and TestMu will be the first consumers, ahead of extracting the bundled providers.

For this PR, please target a Doublespeed-owned npm package with agentDevicePlugin: { apiVersion: 1, provider: "doublespeed", entry: "./dist/plugin.js" }. Type its default factory parameter as the experimental ProviderPluginHost from agent-device/plugins; infer the { runtime, platformModule } return from the provider implementation. The SDK exports only factory context, keeping core runtime and platform implementation types out of the public declarations. Construct lazily from the host environment/options, use the injected createError, and preserve shutdown and exact-owner recovery. Core manages npm installation, version constraints, immutable directories, compatibility checks, and activation; no root dependency or hardcoded runtime import is needed.

The remaining shared prerequisite is connection-provider registration: profile construction, verification, capabilities, and consumed profile fields must come from one provider declaration so connect doublespeed needs no new core switch cases. The foundation deliberately does not expose that seam yet. Any required host zip/install helpers should have narrow owning SDK ports rather than importing private workspace packages.

Please retain the shared session/app-log extraction and the earlier failure-path/capability corrections when reshaping this work. The plugin boundary does not change the allocation rollback, streaming upload, truthful facts, cancellation, or exact-head evidence requirements. Build the adapter against the infrastructure head, with a clean npm install → connect → install/open → interaction → close/recovery run before landing the adapter.

…lugin

* origin/main: (628 commits)
  fix(ios): pin runner build roots under derived data (callstack#3158)
  feat: add managed provider plugin infrastructure (callstack#3121)
  fix(ios): report keyboard focus from the AX bridge's is-editing trait (callstack#3163)
  feat(devices): report model and osVersion (callstack#3119)
  fix: guard alert deadline before native tap synthesis (callstack#3113)
  feat(install-source): accept archive URLs from any public host (callstack#3110)
  test: keep uptime responsive behind busy runner work (callstack#3114)
  fix(apple): read the launch confirmation whenever the open cannot see the app (callstack#3115)
  fix(host-kit): keep extracted directories owner-accessible (callstack#3111)
  fix(daemon): run Apple tools with the requesting client's DEVELOPER_DIR (callstack#3109)
  fix(daemon): fence daemon.json removal to its owning process (callstack#3102)
  fix(snapshot): stop sibling-sized chrome containers from covering their own region (callstack#2996) (callstack#3097)
  fix(ios): stop reading windows past the one the runner resolved (callstack#3103)
  refactor(daemon): apply one dispatch-disclosure rule to returned and thrown failures (callstack#3099)
  chore: drop unused production exports and suppress dynamic consumers (callstack#3100)
  docs(help): document wait readiness and restart exhaustion (callstack#3098)
  docs: simplify Host to fresh Simlock devices and lease recovery (callstack#3095)
  feat(capture): report the display rotation a screenshot was rendered in (callstack#3088)
  refactor(snapshot): preserve normalized node attributes through presentation (callstack#3092)
  refactor(help): colocate fold guidance and extract workflows (callstack#3093)
  ...

# Conflicts:
#	README.md
#	package.json
#	packages/kernel/src/snapshot.ts
#	src/__tests__/eager-closure-budgets.ts
#	src/cli/commands/connection-presentation.ts
#	src/commands/schema/cli-help.ts
#	src/commands/schema/command-overrides.ts
@thymikee thymikee changed the title feat: add direct Doublespeed provider runtime feat: add optional Doublespeed provider plugin Oct 3, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

28 issues found across 96 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/provider-doublespeed/src/session-client.ts">

<violation number="1" location="packages/provider-doublespeed/src/session-client.ts:151">
P2: `longPress(..., 0)` drops the explicit duration and lets the session API use its default. Include `ms` whenever it is not `undefined` so zero-duration requests are preserved.</violation>
</file>

<file name="packages/provider-doublespeed/src/runtime.fixtures.ts">

<violation number="1" location="packages/provider-doublespeed/src/runtime.fixtures.ts:121">
P3: `scriptedFetch` throws before the client sees scripted 204/205/304 responses because `Response` forbids bodies for those statuses. Pass `null` for body-forbidden statuses so tests can cover successful no-content responses.</violation>
</file>

<file name="packages/provider-doublespeed/src/app-log-poller.ts">

<violation number="1" location="packages/provider-doublespeed/src/app-log-poller.ts:73">
P2: A rejected `output.write` is caught as recoverable, but `previous` has already advanced, so the next poll skips the unwritten delta. Move the assignment after the awaited write.</violation>
</file>

<file name="packages/provider-doublespeed/src/plugin.ts">

<violation number="1" location="packages/provider-doublespeed/src/plugin.ts:51">
P3: When `DOUBLESPEED_DEVICE` is set, connect still reports “Provider-selected iOS simulator” instead of the selected model. Pass the configured device to verification so readiness matches the simulator that will be allocated.</violation>
</file>

<file name="packages/provider-doublespeed/src/runtime.ts">

<violation number="1" location="packages/provider-doublespeed/src/runtime.ts:95">
P2: This drops the allocation context, so caller cancellation and the lease deadline never reach Doublespeed’s create and readiness requests; a timed-out lease can still allocate a simulator for up to 10 minutes. Pass the context through and bound creation and polling by its signal and deadline.</violation>
</file>

<file name="packages/provider-doublespeed/src/runtime-instance.ts">

<violation number="1" location="packages/provider-doublespeed/src/runtime-instance.ts:34">
P2: Lowercasing the full URL aliases distinct case-sensitive base paths, even though the API client sends requests to those paths with their original casing. Normalize the scheme and hostname only so endpoints such as `/TenantA` and `/tenanta` retain separate runtime owners.</violation>
</file>

<file name="src/commands/schema/cli-help.ts">

<violation number="1" location="src/commands/schema/cli-help.ts:653">
P2: This sequence omits the required daemon restart: an already-running daemon will not load the newly added provider, so subsequent install/open commands fail. Tell users to close active sessions and restart the daemon before continuing when it is already running.</violation>
</file>

<file name="packages/provider-doublespeed/src/api-client.ts">

<violation number="1" location="packages/provider-doublespeed/src/api-client.ts:92">
P1: A readiness timeout or poll error throws after creation, but this path never deletes `created.id`. `DoublespeedRuntime.allocate` only starts its cleanup after this call returns, so delete the simulator in a catch before rethrowing to avoid billable orphans.</violation>

<violation number="2" location="packages/provider-doublespeed/src/api-client.ts:151">
P3: `uploadAsset` reads the entire archive into memory with `fs.promises.readFile` before upload. `.app` bundles are zipped (often 100 MB+ for a packaged app) and payloaded once per org, so a single install can transiently double memory usage. Stream the file instead: pass `Readable.toWeb(fs.createReadStream(filePath))` as the fetch body (undici's fetch consumes web ReadableStreams and streams them) to keep memory O(chunk).</violation>
</file>

<file name="packages/provider-doublespeed/src/ios.ts">

<violation number="1" location="packages/provider-doublespeed/src/ios.ts:218">
P2: `doubleTap` sends two separate HTTP `tap` requests, with a full cloud round trip between the two device-side events. The Limrun sibling documents exactly this pattern as failing the double-tap recognizer ('Two separate tap requests put a network round trip between the taps, which exceeds the double-tap recognition window and registers as two slow single taps') and therefore batches both taps in one on-device `performActions` call. On a remote Mac fleet the RTT gap is unconstrained, so a `doubleTap` here is likely to register as two slow taps. If the session API exposes no composite gesture, at least document this limitation at the call site; otherwise prefer an endpoint that paces the two taps on the device.</violation>

<violation number="2" location="packages/provider-doublespeed/src/ios.ts:242">
P1: `fill` must replace the field, but this only taps and types; existing text is appended and an empty fill reports success without clearing. Implement a replacement/clear path and verify the target took focus, or reject fills the session cannot perform safely.</violation>
</file>

<file name="packages/kernel/src/snapshot.ts">

<violation number="1" location="packages/kernel/src/snapshot.ts:466">
P2: R74 still omits `doublespeed-ios-tree` from `IOS_PROVENANCE_LITERALS`, so daemon assembly can branch on this newly valid producer without the guard rejecting it. Add the producer to the guarded set.</violation>
</file>

<file name="src/plugins/load.ts">

<violation number="1" location="src/plugins/load.ts:85">
P3: A factory returning a non-null primitive throws a raw `TypeError` here instead of the loader’s `INVALID_ARGS` error. Check that `result` is a non-null object before using `in`.</violation>
</file>

<file name="src/__tests__/provider-device-runtimes.test.ts">

<violation number="1" location="src/__tests__/provider-device-runtimes.test.ts:83">
P3: This check is always false because `doublespeed` was just asserted `undefined`, while each registration has an object `runtime`; it cannot detect a Doublespeed module. Check `module.owner.provider === 'doublespeed'` instead.</violation>
</file>

<file name="packages/provider-doublespeed/src/device.ts">

<violation number="1" location="packages/provider-doublespeed/src/device.ts:27">
P2: `parseDoublespeedDeviceId` accepts trailing ID segments, so malformed IDs such as `doublespeed:ios:lease-a:other` resolve to the `lease-a` session. Reject IDs unless they contain exactly three fields.</violation>
</file>

<file name="website/docs/docs/doublespeed.md">

<violation number="1" location="website/docs/docs/doublespeed.md:49">
P3: The orientation claim omits a supported-value limitation: `portrait-upside-down` is rejected by the provider. Specify that only portrait and landscape orientations are supported.</violation>
</file>

<file name="packages/provider-doublespeed/src/connection-verification.test.ts">

<violation number="1" location="packages/provider-doublespeed/src/connection-verification.test.ts:47">
P2: `JSON.stringify(error)` omits the non-enumerable `Error.message`, so this check passes if verification echoes `dsx_bad_key` in its message. Check the error string as well as its serialized fields.</violation>
</file>

<file name="packages/provider-doublespeed/src/app-log-reconnect.test.ts">

<violation number="1" location="packages/provider-doublespeed/src/app-log-reconnect.test.ts:31">
P2: This test does not verify that log polling uses the simulator's session URL; the scripted fetch returns the same payload for any URL, so a reconnect that ignores `api_url` still passes. Assert the recorded request URL and path to cover the session-capability routing this test claims to exercise.</violation>
</file>

<file name="website/docs/docs/plugins.md">

<violation number="1" location="website/docs/docs/plugins.md:42">
P2: This example puts the connection capabilities at the package-manifest root, so copying it does not declare `agentDevicePlugin.connection`. Nest the object under `agentDevicePlugin.connection`.</violation>
</file>

<file name="packages/provider-doublespeed/src/app-log-reconnect.ts">

<violation number="1" location="packages/provider-doublespeed/src/app-log-reconnect.ts:29">
P2: `DoublespeedApiClient` treats `running` as live and polls unready simulators in `awaitReady()`, but this check immediately reports one as `missing`. During recovery, durable app-log reattachment then fails with `resource-missing` while the simulator still exists; retry live states before returning `missing`.</violation>
</file>

<file name="packages/contracts/src/ios-snapshot.ts">

<violation number="1" location="packages/contracts/src/ios-snapshot.ts:8">
P3: This addition makes the adjacent contract comment inaccurate: it omits Doublespeed from the provider list and says truncation covers four producers, though the union now has five. Update the comment to include Doublespeed and the new count.</violation>
</file>

<file name="packages/provider-doublespeed/src/api-client.test.ts">

<violation number="1" location="packages/provider-doublespeed/src/api-client.test.ts:78">
P2: This serialization omits `Error.message`, so a regression that echoes the API key in the user-visible error message would still pass. Check `String(error)` as well as the serialized details.</violation>
</file>

<file name="src/plugins/host.ts">

<violation number="1" location="src/plugins/host.ts:18">
P3: Set `allowFailure: true` here or catch the rejection; otherwise `runCmd` rejects nonzero `zip` exits before this branch can emit the provider-specific packaging error.</violation>
</file>

<file name="scripts/layering/package-boundaries.ts">

<violation number="1" location="scripts/layering/package-boundaries.ts:118">
P2: This now accepts workspace packages in `devDependencies`, but the violation still directs authors to `package.json dependencies`. For bundled imports, that needlessly makes a build-only dependency a runtime dependency; update the diagnostic to name both fields.</violation>
</file>

<file name="scripts/check-provider-plugin.mjs">

<violation number="1" location="scripts/check-provider-plugin.mjs:26">
P3: The successful fixture sends `[]`, but `listSimulators` expects `{ simulators: [...] }`; verification ignores the result, so this smoke test passes with an invalid API response. Return `{ simulators: [] }` for the success case.</violation>
</file>

<file name="packages/provider-doublespeed/src/facts-runtime.ts">

<violation number="1" location="packages/provider-doublespeed/src/facts-runtime.ts:173">
P3: `setFoldPose` is marked unavailable with the `viewportUnavailable` hint that says Doublespeed does not expose viewport resizing, which is a different capability. Give fold pose its own unavailability cell (e.g. hint 'Doublespeed does not expose fold pose controls.').</violation>

<violation number="2" location="packages/provider-doublespeed/src/facts-runtime.ts:188">
P3: The perf facts (frames, memory sample/snapshot, native capture, profile report) are marked unavailable with the `elementTextUnavailable` hint about reading element text from the captured tree, which is misleading for perf operations. Add a dedicated `perfUnavailable` cell with a hint such as 'Doublespeed does not expose performance capture or sampling' and use it for all five perf cells.</violation>
</file>

<file name="packages/provider-doublespeed/src/connection-verification.ts">

<violation number="1" location="packages/provider-doublespeed/src/connection-verification.ts:38">
P2: The generic catch here swallows Doublespeed's classified credit failure. `DoublespeedApiClient` maps HTTP 402 to `COMMAND_FAILED` with the billing hint ('Add credits at https://mac.doublespeed.ai/dashboard/billing…'), but `verifyDoublespeedConnection` only re-throws `UNAUTHORIZED` and wraps everything else — including that 402 — into a new `COMMAND_FAILED` whose hint says only 'Check Doublespeed service access…'. The credit message and billing hint survive only as `cause`, so a user hitting a no-credits 402 during `agent-device connect` sees neither. Preserve the classified error instead of re-wrapping it.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

},
signal,
);
return await this.awaitReady(created, signal);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: A readiness timeout or poll error throws after creation, but this path never deletes created.id. DoublespeedRuntime.allocate only starts its cleanup after this call returns, so delete the simulator in a catch before rethrowing to avoid billable orphans.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/provider-doublespeed/src/api-client.ts, line 92:

<comment>A readiness timeout or poll error throws after creation, but this path never deletes `created.id`. `DoublespeedRuntime.allocate` only starts its cleanup after this call returns, so delete the simulator in a catch before rethrowing to avoid billable orphans.</comment>

<file context>
@@ -0,0 +1,245 @@
+      },
+      signal,
+    );
+    return await this.awaitReady(created, signal);
+  }
+
</file context>
Suggested change
return await this.awaitReady(created, signal);
try {
return await this.awaitReady(created, signal);
} catch (error) {
await this.deleteSimulator(created.id).catch(() => {});
throw error;
}

await this.session.client.typeText(text);
}

async fill(x: number, y: number, text: string): Promise<void> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: fill must replace the field, but this only taps and types; existing text is appended and an empty fill reports success without clearing. Implement a replacement/clear path and verify the target took focus, or reject fills the session cannot perform safely.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/provider-doublespeed/src/ios.ts, line 242:

<comment>`fill` must replace the field, but this only taps and types; existing text is appended and an empty fill reports success without clearing. Implement a replacement/clear path and verify the target took focus, or reject fills the session cannot perform safely.</comment>

<file context>
@@ -0,0 +1,434 @@
+    await this.session.client.typeText(text);
+  }
+
+  async fill(x: number, y: number, text: string): Promise<void> {
+    await this.tap(x, y);
+    await this.session.client.typeText(text);
</file context>

openUrl: async (url, signal) => await post('/open-url', { url }, signal),
tap: async (x, y, signal) => await post('/tap', { x, y }, signal),
longPress: async (x, y, ms, signal) =>
await post('/long-press', { x, y, ...(ms ? { ms } : {}) }, signal),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: longPress(..., 0) drops the explicit duration and lets the session API use its default. Include ms whenever it is not undefined so zero-duration requests are preserved.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/provider-doublespeed/src/session-client.ts, line 151:

<comment>`longPress(..., 0)` drops the explicit duration and lets the session API use its default. Include `ms` whenever it is not `undefined` so zero-duration requests are preserved.</comment>

<file context>
@@ -0,0 +1,200 @@
+    openUrl: async (url, signal) => await post('/open-url', { url }, signal),
+    tap: async (x, y, signal) => await post('/tap', { x, y }, signal),
+    longPress: async (x, y, ms, signal) =>
+      await post('/long-press', { x, y, ...(ms ? { ms } : {}) }, signal),
+    tapElement: async (selector, signal) => await post('/tap-element', { selector }, signal),
+    typeText: async (text, signal) => await post('/type', { text }, signal),
</file context>

Comment on lines +73 to +74
previous = read.text;
if (delta) await output.write(delta.endsWith('\n') ? delta : `${delta}\n`);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: A rejected output.write is caught as recoverable, but previous has already advanced, so the next poll skips the unwritten delta. Move the assignment after the awaited write.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/provider-doublespeed/src/app-log-poller.ts, line 73:

<comment>A rejected `output.write` is caught as recoverable, but `previous` has already advanced, so the next poll skips the unwritten delta. Move the assignment after the awaited write.</comment>

<file context>
@@ -0,0 +1,206 @@
+        }
+        if (stopped) return;
+        const delta = appendedTail(previous, read.text);
+        previous = read.text;
+        if (delta) await output.write(delta.endsWith('\n') ? delta : `${delta}\n`);
+        state = 'active';
</file context>
Suggested change
previous = read.text;
if (delta) await output.write(delta.endsWith('\n') ? delta : `${delta}\n`);
if (delta) await output.write(delta.endsWith('\n') ? delta : `${delta}\n`);
previous = read.text;

readonly provider = DOUBLESPEED_PROVIDER;

readonly leaseLifecycle: LeaseLifecycleProvider = {
allocate: async (lease) => await this.allocate(lease),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: This drops the allocation context, so caller cancellation and the lease deadline never reach Doublespeed’s create and readiness requests; a timed-out lease can still allocate a simulator for up to 10 minutes. Pass the context through and bound creation and polling by its signal and deadline.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/provider-doublespeed/src/runtime.ts, line 95:

<comment>This drops the allocation context, so caller cancellation and the lease deadline never reach Doublespeed’s create and readiness requests; a timed-out lease can still allocate a simulator for up to 10 minutes. Pass the context through and bound creation and polling by its signal and deadline.</comment>

<file context>
@@ -0,0 +1,334 @@
+  readonly provider = DOUBLESPEED_PROVIDER;
+
+  readonly leaseLifecycle: LeaseLifecycleProvider = {
+    allocate: async (lease) => await this.allocate(lease),
+    release: async (lease) => await this.release(lease),
+  };
</file context>

| 'appium-source'
| 'limrun-ios-tree';
| 'limrun-ios-tree'
| 'doublespeed-ios-tree';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: This addition makes the adjacent contract comment inaccurate: it omits Doublespeed from the provider list and says truncation covers four producers, though the union now has five. Update the comment to include Doublespeed and the new count.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/contracts/src/ios-snapshot.ts, line 8:

<comment>This addition makes the adjacent contract comment inaccurate: it omits Doublespeed from the provider list and says truncation covers four producers, though the union now has five. Update the comment to include Doublespeed and the new count.</comment>

<file context>
@@ -4,12 +4,13 @@ export type IosSnapshotProducer =
   | 'appium-source'
-  | 'limrun-ios-tree';
+  | 'limrun-ios-tree'
+  | 'doublespeed-ios-tree';
 
 export type IosAcquisitionProducer = Exclude<IosSnapshotProducer, 'apple-runner'>;
</file context>

Comment thread src/plugins/host.ts
apple: Object.freeze({
archiveDirectory: async ({ sourceDirectory, entryName, archivePath }) => {
const args = ['-qr', archivePath, entryName];
const result = await runCmd('zip', args, { cwd: sourceDirectory, timeoutMs: 120_000 });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Set allowFailure: true here or catch the rejection; otherwise runCmd rejects nonzero zip exits before this branch can emit the provider-specific packaging error.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At src/plugins/host.ts, line 18:

<comment>Set `allowFailure: true` here or catch the rejection; otherwise `runCmd` rejects nonzero `zip` exits before this branch can emit the provider-specific packaging error.</comment>

<file context>
@@ -0,0 +1,36 @@
+    apple: Object.freeze({
+      archiveDirectory: async ({ sourceDirectory, entryName, archivePath }) => {
+        const args = ['-qr', archivePath, entryName];
+        const result = await runCmd('zip', args, { cwd: sourceDirectory, timeoutMs: 120_000 });
+        if (result.exitCode !== 0) {
+          throw new AppError('COMMAND_FAILED', 'Failed to package iOS app for provider install', {
</file context>
Suggested change
const result = await runCmd('zip', args, { cwd: sourceDirectory, timeoutMs: 120_000 });
const result = await runCmd('zip', args, {
cwd: sourceDirectory,
timeoutMs: 120_000,
allowFailure: true,
});

requests.push(request.url);
const result = fixture.respond(request.url, rejectCredentials);
response.writeHead(result.status ?? 200, { 'content-type': 'application/json' });
response.end(JSON.stringify(result.body));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The successful fixture sends [], but listSimulators expects { simulators: [...] }; verification ignores the result, so this smoke test passes with an invalid API response. Return { simulators: [] } for the success case.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/check-provider-plugin.mjs, line 26:

<comment>The successful fixture sends `[]`, but `listSimulators` expects `{ simulators: [...] }`; verification ignores the result, so this smoke test passes with an invalid API response. Return `{ simulators: [] }` for the success case.</comment>

<file context>
@@ -0,0 +1,141 @@
+  requests.push(request.url);
+  const result = fixture.respond(request.url, rejectCredentials);
+  response.writeHead(result.status ?? 200, { 'content-type': 'application/json' });
+  response.end(JSON.stringify(result.body));
+});
+await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve));
</file context>

query: audioProbeUnavailable,
}),
...perfRuntimeOperationFacts({
frames: elementTextUnavailable,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The perf facts (frames, memory sample/snapshot, native capture, profile report) are marked unavailable with the elementTextUnavailable hint about reading element text from the captured tree, which is misleading for perf operations. Add a dedicated perfUnavailable cell with a hint such as 'Doublespeed does not expose performance capture or sampling' and use it for all five perf cells.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/provider-doublespeed/src/facts-runtime.ts, line 188:

<comment>The perf facts (frames, memory sample/snapshot, native capture, profile report) are marked unavailable with the `elementTextUnavailable` hint about reading element text from the captured tree, which is misleading for perf operations. Add a dedicated `perfUnavailable` cell with a hint such as 'Doublespeed does not expose performance capture or sampling' and use it for all five perf cells.</comment>

<file context>
@@ -0,0 +1,254 @@
+        query: audioProbeUnavailable,
+      }),
+      ...perfRuntimeOperationFacts({
+        frames: elementTextUnavailable,
+        memorySample: elementTextUnavailable,
+        memorySnapshot: elementTextUnavailable,
</file context>

findText: observationUnavailable,
}),
...viewportRuntimeOperationFacts({ setViewport: viewportUnavailable }),
setFoldPose: viewportUnavailable,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: setFoldPose is marked unavailable with the viewportUnavailable hint that says Doublespeed does not expose viewport resizing, which is a different capability. Give fold pose its own unavailability cell (e.g. hint 'Doublespeed does not expose fold pose controls.').

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At packages/provider-doublespeed/src/facts-runtime.ts, line 173:

<comment>`setFoldPose` is marked unavailable with the `viewportUnavailable` hint that says Doublespeed does not expose viewport resizing, which is a different capability. Give fold pose its own unavailability cell (e.g. hint 'Doublespeed does not expose fold pose controls.').</comment>

<file context>
@@ -0,0 +1,254 @@
+        findText: observationUnavailable,
+      }),
+      ...viewportRuntimeOperationFacts({ setViewport: viewportUnavailable }),
+      setFoldPose: viewportUnavailable,
+      ...doublespeedInteractionOperationFacts(device),
+      ...elementTextRuntimeOperationFacts({ readTextAtPoint: elementTextUnavailable }),
</file context>

@thymikee

thymikee commented Oct 3, 2026

Copy link
Copy Markdown
Member

I reviewed ca1a7c7 and found three problems to fix before this can merge. No conflicts. The one check that reported passed, but the unit, layering, typecheck and packed-plugin smoke lanes did not report on this fork PR, so green covers only that one check. The two open inline threads on the app-log write order and the 402 hint, along with the other open inline threads from the earlier review, still stand.

The Doublespeed app-log and runtime modules are a renamed copy of the Limrun ones. For example, app-log-poller.ts is 206 lines with 43 changed, and app-log-runtime.ts, app-log-descriptor.ts, deployment-runtime.ts and facts-runtime.ts follow the same pattern. That is about 600 duplicated lines, and a fix in one provider will not reach the other. The write-before-advance bug in the poller already exists in both copies. The earlier review and the 2026-10-02 comment asked to keep the shared extraction. The rule: one provider-neutral owner for app-log polling, the descriptor envelope and reattach, parameterized by a reader port. Capture-kit already owns the durable envelope, so could it live there, with provider-limrun and provider-doublespeed both consuming it and both local copies deleted?

The facts overclaim from the earlier review is still there. facts-runtime.ts marks customActions available, but DoublespeedIosInteractor.snapshot ignores SnapshotOptions and never returns actions. A snapshot that asks for custom actions is admitted and returns a tree with none, so an agent reads that as "no actions exist" instead of getting a typed unsupported-provider-mode refusal. The rule: every fact cell the provider marks available must have an implementation that honors that operation's options. Please enumerate the cells from doublespeedRuntimeFacts plus doublespeedInteractionOperationFacts, mark customActions unsupported-provider-mode as Limrun does, apply the same rule to fill and doubleTap, and add a facts test that asserts the refusal.

The PR body says live credentials were unavailable and the only live run is at 8dad650, before the plugin migration. This head changes the activation route through the plugin loader, connection callbacks, host.apple helpers and the bundled AppError copy, and none of it has run against the real service. Please run a packed @agent-device/doublespeed with plugins add into a fresh AGENT_DEVICE_HOME, stop the daemon, then run against the real API: connect doublespeed --platform ios, install <bundle> <App.app>, open --relaunch, snapshot -i, click, logs showing an app-log line, close and disconnect. Then show a label-filtered simulator list with zero remaining sims. Also kill the daemon mid-session and show that recoverExpiredLease deletes the simulator on the next start.

Cubic's open threads that still apply: the P1 allocation leak on a failed create (thread) and the P1 fill without clearing (thread). Another 23 lower-priority threads from the same review also still apply, and none of them is resolved by this head.

Not blocking, and you can take or leave these: the { webDriver } loader branch, the public agent-device/plugins/webdriver subpath and about 270 lines of hub helpers have no consumer here except BrowserStack, and that refactor changes bs:// validation, case folding, directory handling and the session-details timeout with no CHANGELOG entry, so these could move to the TestMu PR; the producer doublespeed-ios-tree is named in kernel, contracts and capture-kit, so a provider-neutral producer would avoid core edits per plugin; the plugin bundles a copy of kernel errors instead of using host.createError, and also bundles contracts and capture-kit runtime code that can drift from the host; and in provider-policy.ts a non-builtin provider whose plugin is no longer installed falls back to remote-provider with requiresRemoteDaemon: true where a typed error naming the missing plugin would be clearer.

Production code is +3999/-278 against the ~700 threshold, so could a smaller design work? Drop the WebDriver adapter and hub helpers, replace the five Limrun-mirrored modules with one shared owner, use a provider-neutral iOS tree producer, and use host.createError plus a narrow descriptor port. That leaves the Doublespeed client, interactor, connection callbacks and the loader's connection seam. Would it help to extract the app-log owner out of provider-limrun into capture-kit in its own PR first, and decide whether plugin snapshot producers use a generic literal or a registration-declared capability?

Before merge, the shared app-log owner must replace the Limrun copies, the allocation leak and the overclaimed facts must be fixed, and the exact-head live run above must be shown. I could not run tests or gates here, and I could not check real-service behavior without credentials.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants