Repository navigation
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Size Report
Startup median (7 runs, lower is better):
|
|
The code looks right to me at a68a590, but I cannot approve it yet because there is no device run for the changed exports. All 16 checks pass on a68a590. The Apple attempt 1 stalled on a WebView fixture load this PR does not touch, and attempt 2 passed. There are no conflicts. Ten device-facing public exports in src/sdk/android-adb.ts change from bare re-exports to scoped wrappers: runAdbShell, runAdbExecOut, listAndroidAppsWithAdb, forceStop, openApp, logcat, keyboard, clipboard, and createAndroidPortReverseManager, which now returns a Proxy. The PR body reports no device run, and under docs/agents/pull-requests.md:21-22 fixture tests do not replace live evidence for a changed device route. A regression in the built SDK entry (Proxy identity, overload dispatch, d.ts) would reach consumers without any run of these exports. Please run the built Not blocking, and you can take or leave these. The reverse managers are wrapped in a Proxy (adb-port-reverse.ts:37), which breaks the managed-identity check, so the PR adds a WeakMap registry and a new public export, scopeAndroidPortReverseManager. Nothing on the reverse path reads the command host, because the executor that runs adb already owns it (Limrun goes through the scoped host.runAdb), so you can stop wrapping reverse managers and delete the registry, the export and the reverse-scope test, as #3833 does. The registry also keeps the first scoped wrapper and ignores the scope passed on later calls. The fake AndroidAdbProvider.exec in android-adb.test.ts:330 and the Limrun reverse half of limrun-runtime-dependencies.test.ts:633-640 call the internal commandHostEnvironment(), which no public SDK consumer can write, so they can go with it while the zip half stays. Separately, resolveAaptPath in manifest.ts:364 already swallows every lookup failure, including host_environment_unscoped, and caches null for the whole process regardless of SDK root, so the next unscoped caller can poison it again; it should rethrow or skip caching when the SDK-root read fails and cache only a real filesystem miss, and a follow-up is fine. Could #3833's shape be enough here? That is an async-only withSdkHost/captureSdkHost in one root module, applied only where a host reader actually runs (artifacts resolve, Limrun zip and plist, Metro, remote-config env, runAndroidShell and runAndroidExecOut), with no synchronous overload, no exec-types extraction, no reverse wrapping and no blanket wrapping of executors that already own the host. That is about 70 production lines instead of +308/-188. The two PRs fix the same regression with the same R86 entry and policy test, so only one should land. Which of #3833 and #3835 will it be? Before merge, please pick one of the two PRs. If this one stays, drop the reverse-manager scoping and registry, and attach the built-entry emulator run above. For transparency, I read the base route but did not run artifacts.test.ts, android-adb.test.ts or limrun-runtime-dependencies.test.ts on base or head. I did not diff the emitted .d.ts, so the 0.21.x signature claim is unverified. I also did not confirm that the daemon passes the same HostEnvironment object to builtinProviderCandidates and to request-router; if it does not, the newly scoped Limrun callbacks drop requestEnv on the daemon route, as runAdb already did on main. I did not check which of the other wrapped exports (keyboard, clipboard, logcat, app control) read the command host, so those wraps may be purely defensive. |
|
Closing in favour of #3833, which fixes the same SDK command-host regression with less code (about 70 production lines, compared with +308/−188 here). #3833 also comes with built-entry emulator evidence and doesn't wrap the reverse managers in a Proxy or add the scopeAndroidPortReverseManager export. The maintainer has marked it ready at 57dfc78. The aapt cache note (don't cache a host refusal) applies to #3833 or a follow-up. |
|
1 participant
Summary
Restore aapt fallback for unscoped
resolveAndroidArchivePackageNamecalls and prevent the first call from poisoning later scoped lookups.Use one process-capture helper,
src/sdk/command-host.ts, for Android ADB exports, artifacts, Limrun construction/adapters, Metro, and remote config. Preserve 0.21.x SDK signatures. R86 declares that helper instead of per-module captures, with no baseline or allowlist.17 files changed. Supporting scope includes synchronous command-host callbacks, a type-only extraction from the oversized executor, and platform-owned canonical reverse-manager registration so scoped managers can be reused safely. Download uses fetch/filesystem APIs; zip and deferred reverse operations are scoped.
Part of #3747. Follow-up to #3753 (review on #3828).
Validation
Head:
a68a59017ff247931fac66049273ca655a98e0dd.pnpm install --frozen-lockfile && pnpm build: passed.pnpm exec vitest run --project unit-core src/sdk/artifacts.test.ts: 2 failed on unchanged main507ef2376621d78aa76cf531bedc5abdf30e7a5a; 2 passed on head. Both base failures returned undefined instead of the aapt package.pnpm check:affected --run --head a68a59017ff247931fac66049273ca655a98e0dd: all runnable checks passed; receiptrun-JlCjpd,exactCommit: true, 11,048 related tests passed.CI still owns Coverage, Integration Tests, compatibility, and native/device lanes. No manual device or live Limrun-provider verification; parked full replay lanes were not run.
CI is green on this head, including Coverage, Integration Tests, and all platform smoke jobs. Apple attempt 1 stalled on “Loading Fixture…”; attempt 2 passed iOS and macOS. The first failure was a WebView fixture-load timeout, not a Swift assertion.