Skip to content

feat: expose Limrun runtime - #1476

Merged
thymikee merged 2 commits into
mainfrom
agent/expose-limrun-runtime
Jul 28, 2026
Merged

feat: expose Limrun runtime#1476
thymikee merged 2 commits into
mainfrom
agent/expose-limrun-runtime

Conversation

@thymikee

@thymikee thymikee commented Jul 28, 2026

Copy link
Copy Markdown
Member

Summary

  • Export LimrunRuntime and LimrunRuntimeOptions from agent-device/limrun so first-party consumers such as the agent-device-cloud bridge can reuse the runtime without depending directly on @limrun/api.
  • Keep environment parsing in daemon composition and remove the redundant public version override.
  • Exercise ./limrun through the packed, installed-package smoke test so the subpath cannot silently disappear.
  • Document the motivating consumer and the constructor-based API.

Validation

  • pnpm exec vitest run --project unit-core src/__tests__/limrun-runtime.test.ts src/__tests__/provider-device-runtimes.test.ts src/__tests__/package-exports.test.ts
  • pnpm typecheck
  • pnpm build
  • pnpm test:integration:node
  • pnpm check (static/tooling, dependency graph, exports, build, and Fallow passed; the concurrent unit bundle hit unrelated host-load timeouts, and every affected file passed when rerun serially, with the final runner cancellation case passing alone in 733 ms)

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
JS raw 1.87 MB 1.87 MB +1.1 kB
JS gzip 599.0 kB 601.5 kB +2.5 kB
npm tarball 714.8 kB 719.0 kB +4.2 kB
npm unpacked 2.50 MB 2.52 MB +17.2 kB

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 28.0 ms 27.3 ms -0.7 ms
CLI --help 57.8 ms 57.3 ms -0.5 ms

Top changed chunks:

Chunk Raw diff Gzip diff
dist/src/device.js +57.3 kB +17.0 kB
dist/src/internal/daemon.js -57.3 kB -16.5 kB
dist/src/runner-disposal.js -27.1 kB -8.0 kB
dist/src/physical-device-control.js +22.6 kB +7.4 kB
dist/src/recording-export-quality.js -6.7 kB -2.1 kB

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-07-28 18:05 UTC

@thymikee

Copy link
Copy Markdown
Member Author

Code quality review

Small diff, but it's a published package boundary change, and the shape that's shipping isn't a usable API. Requesting changes on 1 and 2.


1. The ./limrun subpath exports a value whose type nobody can name (blocker)

I built the branch and looked at what actually lands in the tarball:

dist/src/limrun.d.ts   14,260 bytes
dist/src/limrun.js         74 bytes

limrun.d.ts ends with exactly one export:

declare function createLimrunRuntimeFromEnv(env: NodeJS.ProcessEnv): LimrunRuntime | undefined;
declare class LimrunRuntime implements ProviderDeviceRuntime { ... }
export { createLimrunRuntimeFromEnv };

LimrunRuntime is declared and not exported. Neither is ProviderDeviceRuntime, DeviceLease, DeviceInfo, or Interactor — I checked every sdk entry (src/sdk/*.ts); none of them are on agent-device/contracts or the root entry either. So for the use case the docs now advertise — "bridges that host the provider runtime themselves" — a consumer can call the factory, and then:

  • can't type a field or param that holds the runtime (short of NonNullable<ReturnType<typeof createLimrunRuntimeFromEnv>>)
  • can't construct a DeviceLease to call leaseLifecycle.allocate
  • can't type the Interactor that getInteractor returns
  • can statically do exactly one useful thing: await runtime.shutdown()

The docs list makes the gap visible on its own: agent-device/limrun is the only public subpath in client-api.md with no types: line, while install-source, android-adb, selectors, and finders all have one.

The other half of the problem is what the 14 kB is. Because the return type is structurally reachable, the d.ts inlines the whole internal transitive closure as non-exported declarations — Interactor in full, RunnerCommand (every runner command name), RunnerSequenceStep, AppleRunnerProvider, BackendSnapshotResult, Deadline, ProviderDeviceRuntime. That's the internal architecture published as types, none of it callable, all of it now shipping to consumers.

Two coherent options; the PR is between them:

  • Publish the contract deliberately — export LimrunRuntime and LimrunRuntimeOptions from src/sdk/limrun.ts, and lift ProviderDeviceRuntime / DeviceLease / DeviceInfo / Interactor into agent-device/contracts where the public types already live. That's a real API-surface commitment (those types are how the daemon talks to providers, and they'd become semver-relevant), so it deserves a note in the PR and probably an ADR line.
  • Don't publish the subpath. If the bridge is first-party, an internal import is fine and cheaper.

Shipping the value without the type is the one option that doesn't work.

2. Code judo: the function being published is the one that shouldn't exist

createLimrunRuntimeFromEnv returns LimrunRuntime | undefined, and the undefined means only "LIMRUN_API_KEY isn't set". Its sole internal caller already knows that:

// src/provider-device-runtimes.ts
if (!env.LIMRUN_API_KEY?.trim()) return runtimes;              // check 1
const { createLimrunRuntimeFromEnv } = await import(...);
const limrunRuntime = createLimrunRuntimeFromEnv(env);         // check 2, inside
return limrunRuntime ? [...runtimes, limrunRuntime] : runtimes; // check 3

Three evaluations of one condition, and an optional return type that exists only to carry it. And runtime.ts:104 is options.version ?? readVersion() — so the factory's version: readVersion() is a no-op duplicate of the constructor default, and LimrunRuntimeOptions.version has exactly one producer supplying exactly the default. Strip that out and the factory is only env plumbing.

Deleting it collapses all three:

const apiKey = env.LIMRUN_API_KEY?.trim();
if (!apiKey) return runtimes;
const { LimrunRuntime } = await import('./providers/limrun/runtime.ts');
return [...runtimes, new LimrunRuntime({ apiKey, region: env.LIMRUN_REGION?.trim() || undefined })];

One condition, no optional return, and the public export becomes a class with a nameable type and an explicit options contract — which also resolves finding 1's ergonomics for free. Env-var reading is daemon wiring; it belongs at the wiring site, not on a published API. Publishing ...FromEnv additionally makes LIMRUN_API_KEY / LIMRUN_REGION a published contract, and a bridge hosting the runtime itself is exactly the caller most likely to source config from somewhere other than process.env.

3. The new test doesn't guard what the description says it guards

limrun-public.test.ts imports '../sdk/limrun.ts' — the source barrel, via a relative path. It never touches the published subpath, so it can't stop the subpath "disappearing unnoticed". The './limrun' line you added to package-exports.test.ts is the guard that does that, and it already checks export target ⇄ tsdown entry ⇄ source exists ⇄ module has named exports.

What's actually left in the new file is first-ever behavioral coverage of the factory (there's none in limrun-runtime.test.ts today) — good, but it belongs there next to the rest of the runtime's tests, not in a separate file framed as a packaging test.

If the real goal is "the named exports of a public subpath can't vanish silently", that's worth doing properly: package-exports.test.ts already imports every supported subpath's module in its last test, so give supportedSubpaths an expected export-name set per entry and assert against it. That guards all twelve subpaths instead of one, and pins the client-api.md list to reality. One hand-picked entry point is the version of this test that goes stale.

4. The doc edit reverses a stated policy without saying so

device-clouds.md previously read: "The JavaScript client does not publish provider SDK subpaths. Use the normal typed client methods; provider implementation details stay internal." That line arrived with the provider runtime itself (#1278) and this PR deletes it in a paragraph rewrite. Reversing it may well be right, but it's a package-boundary decision, and the PR doesn't name the bridge that needs this or reference an issue/ADR. Worth stating the motivating consumer explicitly — it's also what decides between the two options in finding 1.


Verified clean

  • Layering is legal. sdk is rank 4, providers is rank 2, so src/sdk/limrun.ts → src/providers/limrun/runtime.ts runs down the spine. No R1–R3 violation.
  • No startup regression. The daemon's dynamic import of the limrun runtime survives the new static entry — dist/src/internal/daemon.js still contains import(\../runtime2.js`), and runtime2.js(which pulls@limrun/api) is referenced only by that dynamic import and by the new limrun.jsentry. The deliberate deferral inprovider-device-runtimes.ts` is intact.
  • No file crosses the 1k-line threshold; no new branching in existing flows.

@thymikee

Copy link
Copy Markdown
Member Author

./limrun is missing from the packed/install consumer smoke table, so Integration Tests fails and the source-relative unit test does not validate the published subpath. Add a deterministic installed-package import/exercise for agent-device/limrun (an empty env is sufficient), update the expected smoke results, and rerun the integration lane on this exact head.

@thymikee

Copy link
Copy Markdown
Member Author

Re-reviewed exact head 2449efc: clean and code-review ready. The installed tarball consumer now imports and exercises agent-device/limrun, and export enumeration makes the test fail if the shipped path disappears. The source-only duplicate was removed, and the documented public API matches the package export. No checks are currently failing; remaining checks are still in progress.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Jul 28, 2026
@thymikee
thymikee merged commit ce7b7a9 into main Jul 28, 2026
30 checks passed
@thymikee
thymikee deleted the agent/expose-limrun-runtime branch July 28, 2026 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant