Repository navigation
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
24 issues found across 120 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="test/integration/provider-scenarios/limrun-ios-appstate.test.ts">
<violation number="1" location="test/integration/provider-scenarios/limrun-ios-appstate.test.ts:109">
P2: This assertion allows duplicate launches to pass, so a regression that opens the app twice is no longer detected. Filter the `simctl` calls and assert the expected launch call list exactly once.</violation>
</file>
<file name="src/core/interactors/limrun-ios-tools.ts">
<violation number="1" location="src/core/interactors/limrun-ios-tools.ts:76">
P1: The normal simulator inventory command is `simctl list -j`, so this `devices` check prevents UDID normalization and leaves the remote UDID in the shared inventory. Match the JSON list form (`-j`/`--json`, while retaining the existing `list devices` form) so consumers can resolve the leased Limrun device.</violation>
<violation number="2" location="src/core/interactors/limrun-ios-tools.ts:120">
P2: This provider makes every local plist read return `null`, which causes the shared macOS XCTest product resolver to treat valid `.xctestrun` files as unusable. Implement the provider's plist read against the local host (with the operation signal and timeout) instead of unconditionally returning `null`.</violation>
</file>
<file name="packages/command-surface/src/management/app.ts">
<violation number="1" location="packages/command-surface/src/management/app.ts:56">
P2: `--launch-console` still rejects every provider-owned open, so this metadata now advertises an unsupported Limrun invocation. Remove Limrun from both this description and the duplicate CLI detail, or admit provider launch-console support before documenting it.</violation>
</file>
<file name="website/docs/docs/client-api.md">
<violation number="1" location="website/docs/docs/client-api.md:67">
P2: This now documents Limrun `launchConsole` support, but Limrun requests are still rejected by the provider lifecycle binding before launch. Keep the example local-only until the provider binding admits Limrun launch-console output, or update the runtime validation.</violation>
</file>
<file name="src/core/interactors/limrun-ios-runner-parity.test.ts">
<violation number="1" location="src/core/interactors/limrun-ios-runner-parity.test.ts:96">
P2: This test does not verify the airplane setting it claims to cover, so an airplane-specific regression can pass unnoticed. Assert the complete `status_bar ... override --dataNetwork hide --wifiMode failed --wifiBars 0 --cellularMode failed --cellularBars 0 --operatorName ''` call separately.</violation>
</file>
<file name="packages/provider-limrun/src/interaction-operations.test.ts">
<violation number="1" location="packages/provider-limrun/src/interaction-operations.test.ts:63">
P3: This test now checks clipboard availability only for the default iOS device, so an Android clipboard regression would pass unnoticed. Assert the same read/write facts for both `iosDevice` and `androidMobileDevice`.</violation>
</file>
<file name="packages/provider-limrun/src/deployment-runtime.test.ts">
<violation number="1" location="packages/provider-limrun/src/deployment-runtime.test.ts:71">
P2: This Apple case advertises push as available without supplying or exercising the push callback. Add a `sendPushNotification` mock to the binding and invoke it in the Apple case so the test verifies the operation behind the new availability assertion.</violation>
</file>
<file name="src/core/limrun-download-file.test.ts">
<violation number="1" location="src/core/limrun-download-file.test.ts:166">
P2: This test can pass without creating a partial destination file, so it does not verify the partial-download cleanup it claims to cover. Wait until `destinationPath` contains the streamed bytes before aborting.</violation>
</file>
<file name="packages/provider-revyl/src/interactor.test.ts">
<violation number="1" location="packages/provider-revyl/src/interactor.test.ts:615">
P2: This replacement never tests cancellation while the local write is in flight, so a regression in the `writeFile` abort/rejection cleanup path would pass. Keep or add a deterministic blocked-write case that aborts before releasing the write, alongside this post-write case.</violation>
</file>
<file name="website/docs/docs/limrun.md">
<violation number="1" location="website/docs/docs/limrun.md:50">
P2: This overstates the credentials needed for attached Android sessions: the install sync itself uses HTTP, but connecting the attached Android provider still requires `LIM_ANDROID_INSTANCE_ADB_URL`. Distinguish iOS URL/token-only access from Android’s required URL, token, and ADB URL.</violation>
</file>
<file name="packages/provider-limrun/src/app-log-runtime.test.ts">
<violation number="1" location="packages/provider-limrun/src/app-log-runtime.test.ts:332">
P2: The iOS branch no longer executes `appState`, so this parameterized runtime test cannot catch a broken remote read or fallback result. Invoke it and assert the expected `{}` response as before.</violation>
</file>
<file name="packages/host-kit/src/internal/request-cancel.ts">
<violation number="1" location="packages/host-kit/src/internal/request-cancel.ts:259">
P2: `awaitWithSignal` leaks its abort listener when called with an already-aborted signal because the manual `aborted()` call does not consume the `{ once: true }` registration. Remove the listener inside `aborted()` so canceled waits do not retain callbacks until an indefinitely pending operation settles.</violation>
</file>
<file name="packages/provider-limrun/src/runtime-dependencies.fixtures.ts">
<violation number="1" location="packages/provider-limrun/src/runtime-dependencies.fixtures.ts:66">
P2: This fixture ignores the session device and reuses a runner bound to `limrun:ios:lease-a`. Create a runner for the supplied `session.device`; otherwise default-fixture iOS tests can silently exercise the wrong simulator.</violation>
</file>
<file name="website/docs/docs/commands.md">
<violation number="1" location="website/docs/docs/commands.md:446">
P2: This paragraph overclaims Limrun’s localized alert support: Limrun remains English-only, so localized permission or launch-confirmation buttons are not recognized or automatically handled as this text promises. Restrict the preferred-language and `launchConfirmationPrompt` claims to local iOS simulators.</violation>
</file>
<file name="src/core/interactors/limrun-ios-runner-products.ts">
<violation number="1" location="src/core/interactors/limrun-ios-runner-products.ts:71">
P2: Cancellation cannot interrupt the archive checksum read. Pass `input.signal` to `fs.readFile` so a canceled Limrun preparation stops before reading and hashing the full archive.</violation>
</file>
<file name="src/core/limrun-download-file.ts">
<violation number="1" location="src/core/limrun-download-file.ts:39">
P2: This branch can replace an already-created HTTP or filesystem `AppError` with `request_canceled` when the caller aborts during cleanup. Preserve an existing `AppError` before consulting the signal, otherwise typed failures such as the 404 used by `readLimrunAppInfo` are misclassified.</violation>
</file>
<file name="src/core/interactors/limrun-ios-files.ts">
<violation number="1" location="src/core/interactors/limrun-ios-files.ts:165">
P2: This cache shares the first caller’s timeout and abort signal with every concurrent read for the bundle. Cache only completed metadata, or give each caller an independently cancellable read so one short or canceled lookup cannot fail a live caller.</violation>
</file>
<file name="packages/provider-limrun/src/android.ts">
<violation number="1" location="packages/provider-limrun/src/android.ts:129">
P2: Pass the request signal into `awaitLimrunDeploymentOperation` here. Without it, aborting an in-flight Android sync does not reject the install until Limrun finishes, delaying artifact cleanup and request teardown.</violation>
</file>
<file name="packages/provider-limrun/src/ios.ts">
<violation number="1" location="packages/provider-limrun/src/ios.ts:95">
P2: The iOS sync omits the caller signal, so cancellation is not observed until `syncApp` settles. Pass `signal` to `awaitLimrunDeploymentOperation`; the SDK request cannot be stopped, but the command should end when it answers, times out, or the connection drops.
(Based on your team's feedback about in-flight Limrun request cancellation.) [871bed26-c669-418e-817d-3b63af6e9aec]</violation>
<violation number="2" location="packages/provider-limrun/src/ios.ts:226">
P2: This wrapper ignores the request-scoped `RunnerContext.appBundleId` when resolving `open`. Use the runner context target as the current target before falling back to `session.appBundleId`, so per-request operations do not resolve against a stale app.</violation>
</file>
<file name="packages/provider-limrun/src/deployment-runtime.ts">
<violation number="1" location="packages/provider-limrun/src/deployment-runtime.ts:47">
P1: This advertises the local Apple uninstall implementation for Limrun devices, but that implementation cannot address the remote `limrun:ios:...` target. Uninstall and reinstall therefore fail instead of removing data through the Limrun session; bind a provider-backed removal operation or keep this capability unavailable.</violation>
<violation number="2" location="packages/provider-limrun/src/deployment-runtime.ts:50">
P1: This marks push available for Limrun iOS while forwarding it to native Apple tooling, which cannot target the remote Limrun instance. Keep Limrun push unavailable or provide an SDK-backed push operation.</violation>
<violation number="3" location="packages/provider-limrun/src/deployment-runtime.ts:104">
P2: `deployMaterializedApp` does not pass its promise through the same cancellation wrapper as `deployApp`, so a canceled request waits for the Limrun SDK sync to answer. Wrap this call with `awaitLimrunDeploymentOperation(operationDrain, ..., signal)` while retaining the drain for the uncancelable provider operation.</violation>
</file>
Reply with feedback, questions, or to request a fix.
View guided diff | Re-trigger cubic
| }); | ||
| }); | ||
| let stdout = result.stdout; | ||
| if (args[0] === 'list' && args.includes('devices') && result.code === 0) { |
There was a problem hiding this comment.
P1: The normal simulator inventory command is simctl list -j, so this devices check prevents UDID normalization and leaves the remote UDID in the shared inventory. Match the JSON list form (-j/--json, while retaining the existing list devices form) so consumers can resolve the leased Limrun device.
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/core/interactors/limrun-ios-tools.ts, line 76:
<comment>The normal simulator inventory command is `simctl list -j`, so this `devices` check prevents UDID normalization and leaves the remote UDID in the shared inventory. Match the JSON list form (`-j`/`--json`, while retaining the existing `list devices` form) so consumers can resolve the leased Limrun device.</comment>
<file context>
@@ -0,0 +1,154 @@
+ });
+ });
+ let stdout = result.stdout;
+ if (args[0] === 'list' && args.includes('devices') && result.code === 0) {
+ try {
+ stdout = normalizeDeviceInventory(stdout, session);
</file context>
| uninstallApp: deployment, | ||
| sendPushNotification: | ||
| device.platform === 'apple' | ||
| ? deployment |
There was a problem hiding this comment.
P1: This marks push available for Limrun iOS while forwarding it to native Apple tooling, which cannot target the remote Limrun instance. Keep Limrun push unavailable or provide an SDK-backed push operation.
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-limrun/src/deployment-runtime.ts, line 50:
<comment>This marks push available for Limrun iOS while forwarding it to native Apple tooling, which cannot target the remote Limrun instance. Keep Limrun push unavailable or provide an SDK-backed push operation.</comment>
<file context>
@@ -28,51 +20,37 @@ const pushUnavailable = unavailableFact(
+ uninstallApp: deployment,
+ sendPushNotification:
+ device.platform === 'apple'
+ ? deployment
+ : sessionActive
+ ? pushUnavailable
</file context>
| uninstallApp: isActiveLimrunRuntimeSession(options, device) | ||
| ? uninstallUnavailable | ||
| : deploymentUnavailable, | ||
| uninstallApp: deployment, |
There was a problem hiding this comment.
P1: This advertises the local Apple uninstall implementation for Limrun devices, but that implementation cannot address the remote limrun:ios:... target. Uninstall and reinstall therefore fail instead of removing data through the Limrun session; bind a provider-backed removal operation or keep this capability unavailable.
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-limrun/src/deployment-runtime.ts, line 47:
<comment>This advertises the local Apple uninstall implementation for Limrun devices, but that implementation cannot address the remote `limrun:ios:...` target. Uninstall and reinstall therefore fail instead of removing data through the Limrun session; bind a provider-backed removal operation or keep this capability unavailable.</comment>
<file context>
@@ -28,51 +20,37 @@ const pushUnavailable = unavailableFact(
- uninstallApp: isActiveLimrunRuntimeSession(options, device)
- ? uninstallUnavailable
- : deploymentUnavailable,
+ uninstallApp: deployment,
+ sendPushNotification:
+ device.platform === 'apple'
</file context>
| assertRpcOk(await daemon.callCommand('open', [BUNDLE_ID], flags, { meta })); | ||
| assert.equal(limrunApiState.launchApp.mock.calls.length, 1); | ||
| assert.ok( | ||
| limrunApiState.simctl.mock.calls.some( |
There was a problem hiding this comment.
P2: This assertion allows duplicate launches to pass, so a regression that opens the app twice is no longer detected. Filter the simctl calls and assert the expected launch call list exactly once.
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 test/integration/provider-scenarios/limrun-ios-appstate.test.ts, line 109:
<comment>This assertion allows duplicate launches to pass, so a regression that opens the app twice is no longer detected. Filter the `simctl` calls and assert the expected launch call list exactly once.</comment>
<file context>
@@ -82,7 +105,11 @@ test('Limrun iOS appstate stops answering from the session once the instance is
assertRpcOk(await daemon.callCommand('open', [BUNDLE_ID], flags, { meta }));
- assert.equal(limrunApiState.launchApp.mock.calls.length, 1);
+ assert.ok(
+ limrunApiState.simctl.mock.calls.some(
+ ([args]) => args[0] === 'launch' && args.includes(BUNDLE_ID),
+ ),
</file context>
| return await unavailable(); | ||
| }, | ||
| whichCommand: async (command) => command === 'xcrun', | ||
| plist: { readJson: async () => null }, |
There was a problem hiding this comment.
P2: This provider makes every local plist read return null, which causes the shared macOS XCTest product resolver to treat valid .xctestrun files as unusable. Implement the provider's plist read against the local host (with the operation signal and timeout) instead of unconditionally returning null.
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/core/interactors/limrun-ios-tools.ts, line 120:
<comment>This provider makes every local plist read return `null`, which causes the shared macOS XCTest product resolver to treat valid `.xctestrun` files as unusable. Implement the provider's plist read against the local host (with the operation signal and timeout) instead of unconditionally returning `null`.</comment>
<file context>
@@ -0,0 +1,154 @@
+ return await unavailable();
+ },
+ whichCommand: async (command) => command === 'xcrun',
+ plist: { readJson: async () => null },
+ simulatorScreenshotBackend: 'runner',
+ simulatorFiles,
</file context>
| install: true, | ||
| watch: false, | ||
| launchMode: options?.relaunch ? 'RelaunchIfRunning' : 'None', | ||
| }), |
There was a problem hiding this comment.
P2: Pass the request signal into awaitLimrunDeploymentOperation here. Without it, aborting an in-flight Android sync does not reject the install until Limrun finishes, delaying artifact cleanup and request teardown.
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-limrun/src/android.ts, line 129:
<comment>Pass the request signal into `awaitLimrunDeploymentOperation` here. Without it, aborting an in-flight Android sync does not reject the install until Limrun finishes, delaying artifact cleanup and request teardown.</comment>
<file context>
@@ -114,39 +112,26 @@ export function createLimrunAndroidInteractor(session: LimrunAndroidSession): In
+ install: true,
+ watch: false,
+ launchMode: options?.relaunch ? 'RelaunchIfRunning' : 'None',
+ }),
);
signal?.throwIfAborted();
</file context>
| }), | |
| }), | |
| signal, |
| ( | ||
| await this.session.runner.applicationTools.resolveOpenTarget(this.session.device, { | ||
| target: app, | ||
| currentAppBundleId: this.session.appBundleId, |
There was a problem hiding this comment.
P2: This wrapper ignores the request-scoped RunnerContext.appBundleId when resolving open. Use the runner context target as the current target before falling back to session.appBundleId, so per-request operations do not resolve against a stale app.
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-limrun/src/ios.ts, line 226:
<comment>This wrapper ignores the request-scoped `RunnerContext.appBundleId` when resolving `open`. Use the runner context target as the current target before falling back to `session.appBundleId`, so per-request operations do not resolve against a stale app.</comment>
<file context>
@@ -175,234 +184,68 @@ export async function installLimrunIosRemoteApp(
+ (
+ await this.session.runner.applicationTools.resolveOpenTarget(this.session.device, {
+ target: app,
+ currentAppBundleId: this.session.appBundleId,
+ surface: 'app',
+ })
</file context>
| deployApp: async (input) => | ||
| awaitLimrunDeploymentOperation(operationDrain, (await load()).deployApp(input), signal), | ||
| materializeAppSource: async (input) => (await load()).materializeAppSource(input), | ||
| deployMaterializedApp: async (input) => (await load()).deployMaterializedApp(input), |
There was a problem hiding this comment.
P2: deployMaterializedApp does not pass its promise through the same cancellation wrapper as deployApp, so a canceled request waits for the Limrun SDK sync to answer. Wrap this call with awaitLimrunDeploymentOperation(operationDrain, ..., signal) while retaining the drain for the uncancelable provider operation.
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-limrun/src/deployment-runtime.ts, line 104:
<comment>`deployMaterializedApp` does not pass its promise through the same cancellation wrapper as `deployApp`, so a canceled request waits for the Limrun SDK sync to answer. Wrap this call with `awaitLimrunDeploymentOperation(operationDrain, ..., signal)` while retaining the drain for the uncancelable provider operation.</comment>
<file context>
@@ -82,61 +60,48 @@ export function createLimrunAppDeploymentOperations(
+ deployApp: async (input) =>
+ awaitLimrunDeploymentOperation(operationDrain, (await load()).deployApp(input), signal),
+ materializeAppSource: async (input) => (await load()).materializeAppSource(input),
+ deployMaterializedApp: async (input) => (await load()).deployMaterializedApp(input),
+ uninstallApp: async (input) => (await load()).uninstallApp(input),
+ };
</file context>
| deployMaterializedApp: async (input) => (await load()).deployMaterializedApp(input), | |
| deployMaterializedApp: async (input) => | |
| awaitLimrunDeploymentOperation(operationDrain, (await load()).deployMaterializedApp(input), signal), |
| const synced = await awaitLimrunDeploymentOperation( | ||
| operationDrain, | ||
| session.client.syncApp(installablePath, { | ||
| install: true, | ||
| watch: false, | ||
| launchMode: options?.relaunch ? 'RelaunchIfRunning' : 'None', | ||
| }), | ||
| ); |
There was a problem hiding this comment.
P2: The iOS sync omits the caller signal, so cancellation is not observed until syncApp settles. Pass signal to awaitLimrunDeploymentOperation; the SDK request cannot be stopped, but the command should end when it answers, times out, or the connection drops.
(Based on your team's feedback about in-flight Limrun request cancellation.) [871bed26-c669-418e-817d-3b63af6e9aec]
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-limrun/src/ios.ts, line 95:
<comment>The iOS sync omits the caller signal, so cancellation is not observed until `syncApp` settles. Pass `signal` to `awaitLimrunDeploymentOperation`; the SDK request cannot be stopped, but the command should end when it answers, times out, or the connection drops.
(Based on your team's feedback about in-flight Limrun request cancellation.) [871bed26-c669-418e-817d-3b63af6e9aec]</comment>
<file context>
@@ -67,60 +65,53 @@ export async function createLimrunIosSession(
+ async () => {
+ signal?.throwIfAborted();
+ // The shared deployment owner must retain the artifact until sync finishes reading it.
+ const synced = await awaitLimrunDeploymentOperation(
+ operationDrain,
+ session.client.syncApp(installablePath, {
</file context>
| const synced = await awaitLimrunDeploymentOperation( | |
| operationDrain, | |
| session.client.syncApp(installablePath, { | |
| install: true, | |
| watch: false, | |
| launchMode: options?.relaunch ? 'RelaunchIfRunning' : 'None', | |
| }), | |
| ); | |
| const synced = await awaitLimrunDeploymentOperation( | |
| operationDrain, | |
| session.client.syncApp(installablePath, { | |
| install: true, | |
| watch: false, | |
| launchMode: options?.relaunch ? 'RelaunchIfRunning' : 'None', | |
| }), | |
| signal, | |
| ); |
|
|
||
| test('a live session reads and writes the clipboard on both legs', () => { | ||
| const live = limrunClipboardOperationFacts(); | ||
| const live = factsFor(); |
There was a problem hiding this comment.
P3: This test now checks clipboard availability only for the default iOS device, so an Android clipboard regression would pass unnoticed. Assert the same read/write facts for both iosDevice and androidMobileDevice.
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-limrun/src/interaction-operations.test.ts, line 63:
<comment>This test now checks clipboard availability only for the default iOS device, so an Android clipboard regression would pass unnoticed. Assert the same read/write facts for both `iosDevice` and `androidMobileDevice`.</comment>
<file context>
@@ -57,39 +55,35 @@ test('the Android leg admits back/orientation and gates tv-remote on a real TV t
test('a live session reads and writes the clipboard on both legs', () => {
- const live = limrunClipboardOperationFacts();
+ const live = factsFor();
expect(live.readClipboard).toEqual({ available: true });
expect(live.writeClipboard).toEqual({ available: true });
</file context>
|
At a95049b, the install change looks right, but the published bundle will likely fail when a changed app is installed again. All 17 checks pass. The bundle still aliases Your live run uses the built Not blocking, take or leave: the versioned help at cli-help.ts:631 still says install needs On the open inline threads: the limrun.md:50 thread still applies, since an attached Android session also needs the ADB URL. Most of the other threads are about #4224 files this commit does not change, so they belong there. The three about leaving the abort signal out of |
Summary
Use Limrun app sync for local iOS and Android installs and reinstalls. This avoids full artifact uploads through asset installation and APK installation over the ADB tunnel. Install-only sync leaves apps stopped, matching local agent-device behavior; reinstall removes old app data. Asset URL installation remains supported. Launch mode remains a Limrun SDK option.
This follows the native XCTest replacement #4224. Its own change touches 21 files and uses released
@limrun/api0.69.1. GitHub requires this upstream PR to targetmain, so the displayed 120-file diff includes its parent; review the isolated sync diff. Merge the native-runner parent first, then rebase this PR. CLI syntax is unchanged:agent-device install ./MyApp.apporagent-device install ./app.apkin a Limrun session.Validation
Tested head
a95049b9a1a86413d2703c91a6cc7f08ff136f65. Receipts and live evidence record final affected checks and fresh iOS/Android install, stopped-state, open, reinstall, data-removal, and uninstall checks. Independent review found no issues. Required GitHub CI remains pending.Open in [code]smith · Need help on this PR? Tag
@codesmith-botwith what you need.