Skip to content

feat(driver-mobilenext): allocate devices via the sessions REST API#228

Merged
gmegidish merged 3 commits into
mainfrom
feat/driver-mobilenext-sessions-api
Jul 14, 2026
Merged

feat(driver-mobilenext): allocate devices via the sessions REST API#228
gmegidish merged 3 commits into
mainfrom
feat/driver-mobilenext-sessions-api

Conversation

@gmegidish

Copy link
Copy Markdown
Member

What

Moves cloud device allocation off the fleet.allocate WebSocket RPC and onto the mobilefleet sessions REST API, reflecting the server-side change where a single session can hold multiple device allocations.

Flow

  • POST /api/v1/sessions → one session per test run (created lazily on first allocation, shared across parallel workers).
  • POST /api/v1/sessions/{sessionId}/devices { filters } → returns an allocationId (the device allocation id, distinct from the session id).
    • Pre-booted device: 201 { allocationId, device: { id: <serial>, … } } — ready inline.
    • On-demand device: 202 { allocationId, state: "allocating" } — provisions asynchronously.
  • Poll GET /api/v1/sessions/{sessionId}/devices, matching the list entry by id === allocationId, until status: "in_use" with an info.serial.
  • Drive/release the device by its serial (POST .../devices/{serial}/release).

Matching by allocationId (not "the first in_use device") is required because a shared session lists several devices at once — it keeps parallel workers from picking up each other's devices.

Changes

  • driver-mobilenext
    • New FleetApiClient (REST): createSession, allocateDevice (with provisioning poll), releaseDevice. Bearer mob_ auth, base https://api.mobilenext.ai. Debug-gated per-call body logging under mw:driver-mobilenext:fleet-api.
    • MobileNextDriver no longer allocates — connect() only opens the RPC channel to drive a pre-allocated serial (dead fleet.allocate/waitForAllocation/fleet.release path removed).
  • mobilewright device-pool
    • MobileNextAllocator rewritten to use FleetApiClient (session per run, allocation per allocate(), release by serial).
    • allocator-factory / launchers updated to the new options shape; allocationTimeout now correctly reaches the poller (previously silently dropped in the pool path).

Test plan

  • npm run lint (tsc + eslint) passes
  • New fleet-api.test.ts: session create, pre-booted allocate, provisioning poll-by-allocationId, release-by-serial, API error surfacing
  • Existing device-pool + driver tests pass
  • Verified end-to-end against a live iOS run (npx mobilewright test)

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 44 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cd903096-afb0-4c8e-b1a4-af0c2fc58db5

📥 Commits

Reviewing files that changed from the base of the PR and between cc28e47 and f3e1c2f.

📒 Files selected for processing (2)
  • packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.test.ts
  • packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts

Walkthrough

The change adds a fleet REST client for session creation, device allocation, polling, release, authentication, cancellation, timeouts, and error handling. MobileNextAllocator now manages shared fleet sessions and device ownership directly. MobileNextDriver connects only to an already allocated device through RPC. Package exports and allocator construction are updated, and tests cover fleet requests, polling, release, cancellation, timeouts, API errors, platform validation, and session retries.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: moving MobileNext device allocation to the sessions REST API.
Description check ✅ Passed The description is directly related to the changeset and accurately describes the REST API allocation flow and updated driver/device-pool behavior.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/driver-mobilenext-sessions-api

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (2)
packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts (1)

9-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated toDeviceType/VALID_DEVICE_TYPES logic with driver.ts.

The same VALID_DEVICE_TYPES set and toDeviceType function exist in packages/driver-mobilenext/src/driver.ts (lines 11-13 per the related graph context). Since driver-mobilenext already exports several symbols from its root (fleet-api.js exports were just added in index.ts), consider exporting this helper once and importing it here instead of re-implementing it.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts`
around lines 9 - 13, Remove the local VALID_DEVICE_TYPES and toDeviceType
definitions from the mobilenext allocator, and reuse the exported helper from
driver-mobilenext. Export toDeviceType from the driver-mobilenext package entry
point, then import it in mobilenext-allocator.ts while preserving its current
DeviceType conversion behavior.
packages/driver-mobilenext/src/fleet-api.ts (1)

8-8: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Hardcoded POLL_INTERVAL isn't configurable like allocationTimeout/fetchFn.

Unlike allocationTimeout and fetchFn, the 5s poll interval is a fixed module constant. This limits production tuning and, downstream, forces fleet-api.test.ts's polling test to wait ~10s of real time per poll cycle since it can't be injected.

Also applies to: 142-146

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/driver-mobilenext/src/fleet-api.ts` at line 8, The module-level
POLL_INTERVAL constant is not injectable, preventing production tuning and fast
polling tests. Update the fleet API configuration and polling flow to accept a
configurable poll interval alongside allocationTimeout and fetchFn, use that
value wherever polling delays are scheduled, and provide the existing 5-second
value as the default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/driver-mobilenext/src/fleet-api.ts`:
- Around line 164-187: Update the private request method to enforce a
per-request timeout using an AbortController, passing its signal to fetchFn and
ensuring the timer is cleared when the request settles. Preserve existing
response handling while allowing stalled calls to abort so waitForDevice and
other callers remain bounded.

In `@packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts`:
- Around line 15-18: Update buildFilters so an undefined criteria.platform does
not silently become an iOS constraint; omit the platform filter when no platform
is provided, while preserving the existing EQUALS filter for explicitly
specified platforms.
- Around line 86-92: Update getSession() so a rejected createSession promise
clears sessionPromise before propagating the error, while preserving promise
caching for concurrent successful allocations. Ensure subsequent allocate()
calls can retry session creation after a failure.
- Around line 15-23: The buildFilters flow currently sends deviceNamePattern as
a literal CONTAINS filter despite its RegExp.prototype.source contract. Update
the mobilenext allocation flow around buildFilters to apply the pattern as a
RegExp client-side after Fleet returns candidates, or change the criteria
contract and callers to explicitly accept only plain substrings; preserve regex
semantics for anchors, character classes, and alternation.
- Around line 52-73: Update MobileNextAllocator.allocate to accept and use the
allocator contract’s takenDeviceIds and AbortSignal parameters, threading
takenDeviceIds into the buildFilters/allocation request so already-claimed
devices are excluded and propagating signal through
FleetApiClient.allocateDevice. If FleetApiClient lacks direct AbortSignal
support, add a wrapper while preserving cancellation on timeout or shutdown,
then keep the existing allocation bookkeeping and result mapping unchanged.

In `@packages/mobilewright/src/device-pool/allocator-factory.ts`:
- Around line 23-30: Update the MobileNextAllocator construction in the
driver-type factory to forward DriverConfigMobileNext.region, mapping it to the
allocator’s apiUrl option if required by its constructor. Preserve the existing
apiKey and allocationTimeout values so FleetApiClient uses the configured
endpoint instead of DEFAULT_API_URL.

---

Nitpick comments:
In `@packages/driver-mobilenext/src/fleet-api.ts`:
- Line 8: The module-level POLL_INTERVAL constant is not injectable, preventing
production tuning and fast polling tests. Update the fleet API configuration and
polling flow to accept a configurable poll interval alongside allocationTimeout
and fetchFn, use that value wherever polling delays are scheduled, and provide
the existing 5-second value as the default.

In `@packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts`:
- Around line 9-13: Remove the local VALID_DEVICE_TYPES and toDeviceType
definitions from the mobilenext allocator, and reuse the exported helper from
driver-mobilenext. Export toDeviceType from the driver-mobilenext package entry
point, then import it in mobilenext-allocator.ts while preserving its current
DeviceType conversion behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ec62f7e0-267d-4ee0-8868-5d5e9f3151ae

📥 Commits

Reviewing files that changed from the base of the PR and between 9628ea1 and 8b3e21c.

📒 Files selected for processing (7)
  • packages/driver-mobilenext/src/driver.ts
  • packages/driver-mobilenext/src/fleet-api.test.ts
  • packages/driver-mobilenext/src/fleet-api.ts
  • packages/driver-mobilenext/src/index.ts
  • packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts
  • packages/mobilewright/src/device-pool/allocator-factory.ts
  • packages/mobilewright/src/launchers.ts
💤 Files with no reviewable changes (1)
  • packages/mobilewright/src/launchers.ts

Comment thread packages/driver-mobilenext/src/fleet-api.ts Outdated
Comment on lines +15 to +18
function buildFilters(criteria: AllocationCriteria): DeviceFilter[] {
const filters: DeviceFilter[] = [
{ attribute: 'platform', operator: 'EQUALS', value: criteria.platform ?? 'ios' },
];

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Undocumented default: missing platform silently becomes 'ios'.

If criteria.platform is undefined, the filter defaults to 'ios' rather than omitting the platform constraint or supporting "any platform." If callers rely on an unset platform to mean "no preference," this narrows results to iOS only without any signal to the caller.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts`
around lines 15 - 18, Update buildFilters so an undefined criteria.platform does
not silently become an iOS constraint; omit the platform filter when no platform
is provided, while preserving the existing EQUALS filter for explicitly
specified platforms.

Comment on lines +15 to +23
function buildFilters(criteria: AllocationCriteria): DeviceFilter[] {
const filters: DeviceFilter[] = [
{ attribute: 'platform', operator: 'EQUALS', value: criteria.platform ?? 'ios' },
];
if (criteria.deviceNamePattern) {
filters.push({ attribute: 'name', operator: 'CONTAINS', value: criteria.deviceNamePattern });
}
return filters;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## files\n'
git ls-files | rg 'packages/mobilewright/src/device-pool/adapters/mobilenext-allocator\.ts|packages/mobilewright/src|AllocationCriteria|DeviceFilter'

printf '\n## outline mobilenext-allocator\n'
ast-grep outline packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts --view expanded

printf '\n## locate AllocationCriteria and DeviceFilter\n'
rg -n "interface AllocationCriteria|type AllocationCriteria|class AllocationCriteria|deviceNamePattern|interface DeviceFilter|type DeviceFilter|operator: 'CONTAINS'|EQUALS" packages/mobilewright/src -S

printf '\n## read relevant file slices\n'
for f in \
  packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts \
  packages/mobilewright/src/**/allocation* \
  packages/mobilewright/src/**/device*filter* \
  packages/mobilewright/src/**/filter*; do
  [ -f "$f" ] && { echo "--- $f"; wc -l "$f"; }
done

# print the exact file if it exists
if [ -f packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts ]; then
  echo "--- packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts"
  cat -n packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts | sed -n '1,220p'
fi

# show matching lines from likely definition files
for f in $(rg -l "deviceNamePattern|DeviceFilter|AllocationCriteria" packages/mobilewright/src -S); do
  echo "--- $f"
  cat -n "$f" | sed -n '1,260p'
done

Repository: mobile-next/mobilewright

Length of output: 41324


deviceNamePattern needs regex handling, not literal CONTAINS. packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts:15-21 forwards the serialized regex source straight to Fleet, but AllocationCriteria.deviceNamePattern is documented as RegExp.prototype.source. Patterns with anchors, classes, or alternation will be misfiltered; apply RegExp matching client-side or narrow the contract to plain substrings.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts`
around lines 15 - 23, The buildFilters flow currently sends deviceNamePattern as
a literal CONTAINS filter despite its RegExp.prototype.source contract. Update
the mobilenext allocation flow around buildFilters to apply the pattern as a
RegExp client-side after Fleet returns candidates, or change the criteria
contract and callers to explicitly accept only plain substrings; preserve regex
semantics for anchors, character classes, and alternation.

Comment on lines 52 to 73
async allocate(criteria: AllocationCriteria): Promise<AllocateResult> {
debug('allocating device (criteria=%o)', criteria);
const driver = new MobileNextDriver(this.driverOptions);
const session = await driver.connect({
platform: criteria.platform ?? 'ios',
deviceName: criteria.deviceNamePattern ? new RegExp(criteria.deviceNamePattern) : undefined,
deviceId: criteria.deviceId,
});
this.activeDrivers.set(session.deviceId, driver);
debug('allocated device %s (platform=%s)', session.deviceId, session.platform);
const info = driver.deviceInfo;
return { deviceId: session.deviceId, platform: session.platform, driver: 'mobilenext', model: info?.model, osVersion: info?.osVersion, type: info?.type };
const sessionId = await this.getSession();
const filters = buildFilters(criteria);
debug('allocating device (session=%s, filters=%o)', sessionId, filters);

const device = await this.client.allocateDevice(sessionId, filters);
const serial = device.info.serial;
if (!serial) {
throw new Error(`Device allocation ${device.id} became in_use without a serial`);
}
this.sessionBySerial.set(serial, sessionId);
debug('allocated device %s (allocation=%s, platform=%s)', serial, device.id, device.info.platform);

return {
deviceId: serial,
platform: device.info.platform === 'android' ? 'android' : ('ios' as Platform),
driver: 'mobilenext',
model: device.info.name,
osVersion: device.info.osVersion,
type: toDeviceType(device.info.type),
};
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate the allocator interface and mobilenext implementation
git ls-files | rg 'packages/mobilewright/src/.+allocator|packages/mobilewright/src/.+device-pool|packages/mobilewright/src/.+fleet' || true

echo '--- interface search ---'
rg -n "interface DeviceAllocator|type DeviceAllocator|allocate\\(criteria: AllocationCriteria" packages/mobilewright/src -S

echo '--- mobilenext allocator ---'
cat -n packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts

echo '--- related client methods ---'
rg -n "allocateDevice\\(|allocationTimeout|AbortSignal|takenDeviceIds|ReadonlySet<string>" packages/mobilewright/src -S

Repository: mobile-next/mobilewright

Length of output: 8452


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the device allocator contract and mobilenext adapter in smaller slices.
echo '--- files ---'
git ls-files packages/mobilewright/src | rg 'device-pool|fleet|allocator|allocation' || true

echo '--- interface definitions ---'
rg -n "interface DeviceAllocator|allocate\\(" packages/mobilewright/src -S

echo '--- mobilenext allocator ---'
sed -n '1,220p' packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts

echo '--- signal / takenDeviceIds propagation ---'
rg -n "takenDeviceIds|AbortSignal|signal\\?" packages/mobilewright/src/device-pool packages/mobilewright/src/fleet -S

Repository: mobile-next/mobilewright

Length of output: 12558


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,140p' packages/mobilewright/src/device-pool/application/ports.ts
sed -n '1,260p' packages/mobilewright/src/device-pool/application/device-pool.ts
sed -n '1,120p' packages/mobilewright/src/device-pool/adapters/mobilecli-allocator.ts

Repository: mobile-next/mobilewright

Length of output: 10777


Thread the allocator contract through MobileNext. DevicePool passes both takenDeviceIds and signal here, but this implementation drops them. In-flight allocations can’t be aborted on timeout/shutdown, and the pool can still race itself into handing out a device it already considers taken. If FleetApiClient can’t accept an AbortSignal directly, add a wrapper that does.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts`
around lines 52 - 73, Update MobileNextAllocator.allocate to accept and use the
allocator contract’s takenDeviceIds and AbortSignal parameters, threading
takenDeviceIds into the buildFilters/allocation request so already-claimed
devices are excluded and propagating signal through
FleetApiClient.allocateDevice. If FleetApiClient lacks direct AbortSignal
support, add a wrapper while preserving cancellation on timeout or shutdown,
then keep the existing allocation bookkeeping and result mapping unchanged.

Comment on lines 23 to 30
if (driverType === 'mobilenext' || driverType === 'mobile-use') {
const mobileNextConfig = config.driver as DriverConfigMobileNext;
const allocator = new MobileNextAllocator({
driverOptions: {
region: mobileNextConfig.region,
apiKey: mobileNextConfig.apiKey,
},
apiKey: mobileNextConfig.apiKey ?? '',
allocationTimeout: mobileNextConfig.allocationTimeout,
});
return { allocator };
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
ast-grep run --pattern 'interface DriverConfigMobileNext { $$$ }' --lang typescript packages/mobilewright/src
rg -n "region" packages/mobilewright/src/device-pool

Repository: mobile-next/mobilewright

Length of output: 961


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,200p' packages/mobilewright/src/device-pool/allocator-factory.ts
printf '\n---\n'
sed -n '1,220p' packages/mobilewright/src/device-pool/mobile-next-allocator.ts
printf '\n---\n'
rg -n "apiUrl|region" packages/mobilewright/src

Repository: mobile-next/mobilewright

Length of output: 1747


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Files under packages/mobilewright/src/device-pool:\n'
fd -a . packages/mobilewright/src/device-pool

printf '\n--- allocator/adapters outlines ---\n'
ast-grep outline packages/mobilewright/src/device-pool/adapters --view expanded || true

printf '\n--- search apiUrl/region ---\n'
rg -n "apiUrl|region" packages/mobilewright/src

Repository: mobile-next/mobilewright

Length of output: 5255


🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,140p' packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts
printf '\n---\n'
sed -n '1,120p' packages/mobilewright/src/launchers.ts
printf '\n---\n'
sed -n '1,140p' packages/mobilewright/src/config.ts

Repository: mobile-next/mobilewright

Length of output: 13309


🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "new MobileNextAllocator|apiUrl:.*region|DEFAULT_API_URL|region" packages/mobilewright/src packages/mobilewright/test packages/mobilewright

Repository: mobile-next/mobilewright

Length of output: 719


Forward region into MobileNextAllocator. DriverConfigMobileNext.region is still part of the config, but this path drops it and FleetApiClient will fall back to DEFAULT_API_URL. If the allocator should honor the same endpoint selection as MobileNextDriver, pass the config through here (or map it to apiUrl) before constructing the client.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/mobilewright/src/device-pool/allocator-factory.ts` around lines 23 -
30, Update the MobileNextAllocator construction in the driver-type factory to
forward DriverConfigMobileNext.region, mapping it to the allocator’s apiUrl
option if required by its constructor. Preserve the existing apiKey and
allocationTimeout values so FleetApiClient uses the configured endpoint instead
of DEFAULT_API_URL.

@gmegidish

Copy link
Copy Markdown
Member Author

@CodeRabbit review

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts (1)

93-95: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Preserve ownership when release fails.

The map entry is deleted before releaseDevice() completes. If that call rejects, the device remains allocated but later releases return immediately because the session mapping is gone, preventing retry and potentially leaking a fleet device. Delete only after success or restore the mapping on failure.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts`
around lines 93 - 95, Update the release flow around sessionBySerial and
releaseDevice so the device-to-session mapping remains available until the
release succeeds. Delete the mapping only after a successful
client.releaseDevice call, or restore it when that call fails, preserving later
retry behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts`:
- Around line 15-22: The buildFilters function currently ignores
criteria.deviceId, allowing allocation of a different device. Update
buildFilters to either add an exact-device filter using the
FleetApiClient-supported attribute or explicitly reject deviceId with a clear
error until exact-device filtering is supported; never silently proceed with
only platform/name filters.

---

Outside diff comments:
In `@packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts`:
- Around line 93-95: Update the release flow around sessionBySerial and
releaseDevice so the device-to-session mapping remains available until the
release succeeds. Delete the mapping only after a successful
client.releaseDevice call, or restore it when that call fails, preserving later
retry behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f54a04ca-e7a1-469d-92ac-90ff65444401

📥 Commits

Reviewing files that changed from the base of the PR and between 8b3e21c and cc28e47.

📒 Files selected for processing (4)
  • packages/driver-mobilenext/src/fleet-api.test.ts
  • packages/driver-mobilenext/src/fleet-api.ts
  • packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.test.ts
  • packages/mobilewright/src/device-pool/adapters/mobilenext-allocator.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/driver-mobilenext/src/fleet-api.test.ts
  • packages/driver-mobilenext/src/fleet-api.ts

@gmegidish gmegidish merged commit 3c7bbfc into main Jul 14, 2026
7 checks passed
@gmegidish gmegidish deleted the feat/driver-mobilenext-sessions-api branch July 14, 2026 18:05
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.

1 participant