Repository navigation
fix(limrun): keep the owner's reverse mappings on attached Android instances - #3204
Conversation
createExecAndroidPortReverseProvider and createAndroidPortReverseManager
(executor form) take { noRebind }. The provider then runs
adb reverse --no-rebind for an endpoint it did not bind, still rebinds
its own mappings, and fails a refusal with COMMAND_FAILED and
details.reason 'android_port_reverse_rebind_refused'. The refusal is
classified from adb reverse --list, not from adb's stderr. The localhost
URL auto-reverse keeps that reason on the error it rethrows.
…stances createLimrunAndroidSession asks createPortReverse for no-rebind mode when ownership is 'attached'. configurePortReverse and the localhost URL auto-reverse can then no longer replace an owner mapping such as tcp:8081, so teardown cannot remove it. Created instances keep plain adb reverse.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 10 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Size Report
Startup median (7 runs, lower is better):
|
|
No-rebind mode now passes --no-rebind on every ensure, so a stale record of a mapping this provider bound can no longer turn into a plain adb reverse over another client's mapping. The manager serializes ensures per device endpoint, so a concurrent duplicate sees the first mapping instead of a refusal, and two owners can no longer both pass its ownership check. Docs state when the typed reason is available.
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Name adb reverse --list as the condition in the noRebind JSDoc, and scope the Limrun docs guarantee to attached Android instances.
|
Code looks right at e4bba1f, but the new adb reverse behavior has no live evidence yet. Smoke Tests was still running when I checked, so CI is not final. There are no conflicts. The change adds Not blocking: the localhost auto-reverse in app-lifecycle.ts calls ensure without an ownerId, so on an attached instance the disconnect carve-out never removes that mapping and agent-device may leave its own Is there a smaller design than a The five cubic-dev-ai threads (one P2, four P3) are fixed at this head and can be resolved: no-rebind decision, limrun.md typed reason, per-endpoint serialization, JSDoc --list condition, attached-instance wording. I read the diff and the pre-change code only. I did not run tests, and I could not check real adbd or tunnel behavior. Before merge, I need the live run above: |
The localhost URL auto-reverse now ensures its mapping with the stable owner 'localhost-url'. Teardown of an attached Limrun instance removes only owned mappings, so before this change the mapping stayed on the owner's device after disconnect. A live run showed that adbd keeps it.
|
I ran the live check on a real Limrun Android instance, and it found the leftover from your non-blocking note. I fixed that in 3569327. Setup. I created the instance with At e4bba1f:
Fix (3569327). The localhost auto-reverse now ensures its mapping with the owner At 3569327, same instance and steps: the refusal had the same reason,
🤖 Addressed by Claude Code |
|
The Limrun change at 3569327 now keeps the owner's reverse mappings intact on attached Android instances, and I found nothing to fix. Since the earlier review at e4bba1f, the requested live evidence is posted. It shows the refusal JSON, the tcp:8097 and tcp:8099 successes, and the owner's CI is green: 21 checks, none failing. The unit and smoke lanes cover the Android localhost-reverse route and the Limrun runtime tests that this change touches. There are no conflicts. The five cubic-dev-ai threads are fixed at this head, so please resolve them: the Nothing else blocks this. It is ready for a human review. |
thymikee
added
the
ready-for-human
1 participant
Summary
Follow-up to the
--no-rebindreview note on #3173.On an attached Limrun Android instance, agent-device no longer replaces a reverse mapping it did not create. Before,
configurePortReverseand the localhost URL auto-reverse ran plainadb reverse. They could take over the owner'stcp:8081, and teardown then removed it.createExecAndroidPortReverseProvider(adb, { noRebind })andcreateAndroidPortReverseManager(executor, { noRebind })runadb reverse --no-rebindon every ensure, so no existing device mapping is replaced. The manager serializes ensures per endpoint.adb reverse --listshows the mapping, a refusal fails withCOMMAND_FAILEDanddetails.reason: 'android_port_reverse_rebind_refused'. adb's stderr is never parsed.createLimrunAndroidSessionasks for no-rebind only whenownership === 'attached'.localhost-url, so attached teardown removes it.10 files, including
client-api.mdandlimrun.md.Validation
356932729c:pnpm check:affected --runpasses (3111 related tests).--no-rebind, restoring the own-mapping exemption, making the lock a no-op, the refusal branch, reason propagation, SDK option forwarding, forcingnoRebind, and dropping thelocalhost-urlowner.356932729con a Limrun Android instance. The owner mappedtcp:8081from its own adb server. agent-device was attached with onlyLIM_ANDROID_INSTANCE_*.open exp://127.0.0.1:8081was refused with the typed reason.tcp:8097andtcp:8099succeeded. Afterdisconnect, the owner listed onlyhost-19 tcp:8081 tcp:8081. Details are in the PR comment.