Skip to content

feat(limrun): sync local iOS and Android app installs - #4225

Merged
thymikee merged 5 commits into
callstack:mainfrom
muvaf:codex/limrun-sync-install
Nov 6, 2026
Merged

thymikee merged 5 commits into
callstack:mainfrom
muvaf:codex/limrun-sync-install

Conversation

@muvaf

@muvaf muvaf commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Use Limrun app sync for local iOS and Android install/reinstall, including changed binaries. The package bundles the delta encoder and license notices. A session-owned basis cache survives artifact materialization, and SDK operations retain their source until completion. Install-only sync leaves apps stopped; asset URL installation remains supported.

This 38-file change uses released SDK 0.69.1 and follows the merged native runner replacement. Scope also includes the Apple deadline-test fixture added after CI exposed a race, now with an independent bounded wait and cleanup. CLI syntax is unchanged: agent-device install ./MyApp.app or agent-device install ./app.apk in a Limrun session.

Validation

Tested d2c9b9fc16e221ecd68e0bc7d3358a0a2c57a7dc: pnpm check:affected --run --head HEAD passes with exactCommit: true, including 17,400 unit tests, provider integration and compatibility checks. Review evidence includes independent review, 55 focused tests, 13 cancellation integration cases, and isolated checks proving the CRLF, complete-notice, queued-basis and bounded-wait coverage.

Earlier live evidence remains attributed to the pre-rebase source. It verifies changed-binary installs on both platforms, stopped state, installed contents, and 5.7 KB iOS/1.6 KB Android deltas. This follow-up changes tests and wording, with no new live-device claim. GitHub-owned checks remain CI obligations.


Autofix with [code]smith
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.

View guided diff

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 120 files

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

Comment thread packages/provider-limrun/src/android.ts Resolved
Comment thread packages/provider-limrun/src/ios.ts Resolved
Comment thread packages/provider-limrun/src/ios.ts Resolved
@thymikee

thymikee commented Nov 4, 2026

Copy link
Copy Markdown
Member

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 @limrun/xdelta3-wasm to a stub that throws (tsdown.config.ts:151). That alias assumed agent-device never calls client.syncApp, but every local install now does (ios.ts:97, android.ts:125). In SDK 0.69.1, syncApp loads that package for every changed file of 64 KB or more that has a basis from an earlier sync, and on Android the basis can also be seeded from the instance. So from the shipped CLI and daemon, the first install of a path works, but the next install of a rebuilt app throws the stub error. That is the normal dev loop. Android can fail on the first install too. CI runs from source with the real package, so green checks cannot show this. The rule: every SDK path the bundle reaches must resolve its dependencies in dist. Could you add @limrun/xdelta3-wasm to deps.onlyBundle, delete the alias and src/vendor/limrun-delta-sync-omitted.ts, and check that the #4070 third-party license notice covers it? I inferred this from the config and the SDK source and did not build or run dist, so please correct me if rolldown resolves it differently.

Your live run uses the built dist client and covers install, stopped state, launch and reinstall on both platforms. It installs the same binary each time, so it never reaches the delta path above. Please add one step to that script on iOS and Android: change the binary, rebuild, and install again. It must succeed, with no "omits @limrun/xdelta3-wasm" error.

Not blocking, take or leave: the versioned help at cli-help.ts:631 still says install needs LIMRUN_API_KEY while limrun.md:50 now says the opposite, so please align both and keep the Android ADB URL requirement accurate; syncApp throws plain errors that reach the daemon untyped, so one provider helper per platform could map them to AppError('COMMAND_FAILED', 'Limrun app sync failed', { platform, installablePath, cause }); runtime-dependencies.fixtures.ts:58 has a leftover blank line, and several tests still stub assets.getOrUpload, which no install path reads now; and the SDK leaves sync basis directories under the temp dir for each artifact path, so if the materialized path differs per run, each install may leave a full app copy behind (pointing the SDK basis directory option at the daemon state dir with bounded retention would fix it, or please confirm the path is stable per app).

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 syncApp do not apply: the caller deletes the artifact when the call returns, so returning early on abort would race that cleanup, and deployApp still observes cancellation.

@muvaf
muvaf force-pushed the codex/limrun-sync-install branch from a95049b to a6a2a7c Compare November 5, 2026 10:35
@muvaf

muvaf commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Confirmed the bundle finding on both platforms at a95049b9a: changed-binary installs fail on the throwing delta encoder stub. The final branch bundles the real encoder, removes the alias/stub, and retains its pinned copyright and full Apache-2.0 license in the generated notices.

The expanded live script runs the built dist client, installs one build, installs a rebuilt binary at the same path, then reinstalls the first build. Both platforms pass stopped-state and installed-content checks. Actual uploads contain 5,698/5,665-byte iOS deltas and 1,632/1,629-byte Android deltas. Base/head logs, wire records, build logs and receipts are published with their exact source SHAs.

The optional fixes are included too: aligned help and Android ADB credentials, typed SDK errors, obsolete fixture cleanup, and one serialized session-owned basis cache with pruning and disposal. In-flight SDK work retains its source until completion; canceled queued work never starts.

The full affected gate passes at 8f01e5cd7. Final head a6a2a7c51 adds only two public-API test mocks, with a passed incremental affected receipt against that parent. Both receipts have exactCommit: true. The branch includes updated #4224; the fork PR keeps the sync-only diff visible. Merge #4224 first, then this branch can be rebased to main.

@muvaf

muvaf commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

The first push exposed a CI timing race in the existing Apple launch-confirmation test: all five kernel-errors shards failed their initial run because the test exhausted 1,000 event-loop turns before the lazy lookup started. bafbb7556 replaces that polling limit with an explicit second-command barrier and preserves the deadline assertions. No timeout or test selection was loosened.

The full selected mutation shard passed locally before and after the barrier change, with 72 killed and 4 surviving mutants. The Linux failure was not reproduced on this Mac. Final bafbb7556 also passes the affected gate against 8f01e5cd7, including the full unit suite, with exactCommit: true. CI failure log, mutation runs, final fixture adjustment and receipt are published. The new CI run remains authoritative.

All 24 inline threads on this PR and 25 on #4224 have replies and are resolved. #4224's GitHub checks are green; this sync PR still follows it.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 1 file (changes from recent commits).

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

@muvaf
muvaf marked this pull request as draft November 5, 2026 11:11
@thymikee

thymikee commented Nov 5, 2026

Copy link
Copy Markdown
Member

At bafbb75, the code looks good, and the issues from the earlier review are fixed: the real delta encoder is bundled with its notices, rebuilt-app installs sync on both platforms, and the basis store is session-owned. The sync evidence (a built dist client, a changed binary at the same path, and the byte deltas on iOS and Android) is accepted from your report; I could not open the linked receipts.

The failing Mutants (kernel-errors-4) shard broke in its dry run on a daemon-client startup-race test, and this diff changes nothing under daemon-client, daemon-contracts or daemon, so it looks unrelated; please re-run that shard. The "Affected decision-kernel mutants" check fails only because shard 4 has no report. There are no conflicts. This PR stacks on #4224, which needs to land first, then a rebase.

Not blocking, take or leave:

  1. bundled-license-notices.ts:145 adds a second per-package notice branch beside jpeg-js; one table keyed by package (replace or append) read by one helper would keep it from growing.
  2. Constructing LimrunSyncCache when the platform session is created (ios.ts:87, android.ts:124) would make the field required and drop the ?. at lease-release.ts:310.
  3. In sync-cache.ts:30, keeping the limrun_app_sync_failed mapping around sync() only would let a prune error after a successful install drop the basis directory and return the sync result, instead of failing the install.
  4. No test reaches session.syncCache?.dispose() through releaseLocalResources; one install plus lease release in an existing test, asserting the basis directory is gone, would cover it.

Open threads: the P2 on launch-confirmation.test.ts:304 (bounded fallback in the race) still stands. The earlier threads on this PR's own changes are fixed and can be resolved; the ones on runner code belong to #4224's scope, and the three on cancellation during sync are deliberate, since the SDK keeps reading the artifact until it settles and the outer deploy still observes cancellation.

@thymikee

thymikee commented Nov 5, 2026

Copy link
Copy Markdown
Member

#4224 has merged, so this now conflicts with main. Please rebase onto main; the review at bafbb75 (#4225 (comment)) still stands, and review will continue on the rebased head.

@muvaf
muvaf force-pushed the codex/limrun-sync-install branch from bafbb75 to 12c2b67 Compare November 5, 2026 20:10
@muvaf

muvaf commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current main at e6ac165961def2f7b80336d2c04586a3ace32ce0; the new head is 12c2b679c3443ba8e7c6def74a47843bdb385cf3. The PR now contains only the 38-file sync layer. All five commits are patch-equivalent, with no manual conflict resolutions or new behavior changes.

pnpm check:affected --run --head HEAD passes with exactCommit: true. Independent review found no rebase regressions, including in the updated cancellation interfaces. Range-diff and validation evidence.

This update is limited to the requested rebase. The existing bounded-wait review finding and optional cleanup suggestions are unchanged. The PR stays draft; fresh CI results will replace the old-head results, and earlier live evidence remains attributed to its original source.

@muvaf
muvaf marked this pull request as ready for review November 5, 2026 20:11

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All reported issues were addressed across 38 files

Reply with feedback, questions, or to request a fix.

View guided diff | Re-trigger cubic

Comment thread src/commands/schema/cli-help.ts Resolved
@thymikee

thymikee commented Nov 5, 2026

Copy link
Copy Markdown
Member

At 12c2b67, the rebase is clean: the code looks good, the conflict from bafbb75 is gone, and all 25 checks pass. The rebase is patch-equivalent, so the live iOS and Android sync evidence accepted at bafbb75 still covers this head. I did not run tests locally or open the receipt links.

A few inline threads still apply, all test or wording issues that do not block merge: the license test portability, the truncated license body check, the weak queued-sync test, the SDK comment wording, and the launch-confirmation test race, which still holds even though that thread is resolved. The basis prune thread is unconfirmed, since @limrun/api 0.69.1 was not available to check, though your live reinstall delta suggests the basis survives pruning.

The cancellation-order thread can be resolved, since the deployment rejects only after the pending sync settles, matching the daemon's finally, and so can the help-text sentence thread.

@muvaf
muvaf force-pushed the codex/limrun-sync-install branch from 12c2b67 to d2c9b9f Compare November 5, 2026 20:59
@muvaf

muvaf commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the latest review in d2c9b9fc16e221ecd68e0bc7d3358a0a2c57a7dc, keeping five commits.

The tests now assert complete normalized xdelta notices, cover LF/CRLF input, and read the queued APK basis before teardown. The cancellation comment and apps help wording are corrected. I also fixed the earlier bounded-wait finding, even though the bot had marked it resolved: its real-I/O guard is independent of the fake clock, and both the guard and owned lookup are cleaned up.

Isolated checks reproduce the CRLF assertion failure and show that truncated notices and premature removal of the APK basis pass the old tests but fail the stronger tests. All 55 focused cases and all 13 unchanged cancellation integration cases pass. Independent review found no remaining findings after the lookup-cleanup refinement.

The two production reports do not apply to this code: materialized install cleanup waits for sync settlement, and pinned SDK 0.69.1 writes source-relative basis files without a separate metadata file in that directory. I replied with the call paths and evidence rather than adding speculative behavior.

pnpm check:affected --run --head HEAD passed with exactCommit: true, including 17,400 unit tests, provider integration and compatibility checks.

Regression/mutation logs, independent-review disposition, and exact-commit validation. Runtime installation logic is unchanged. Earlier live evidence remains attributed to its original source; fresh GitHub CI checks remain authoritative.

@thymikee

thymikee commented Nov 5, 2026

Copy link
Copy Markdown
Member

At d2c9b9f, the code looks good. The test and wording threads from the earlier review are fixed, and the launch-confirmation.test.ts guard now uses a real delay and aborts in finally, so the remaining inline thread can be resolved. All 22 checks pass and there are no conflicts; I read the delta and did not run the tests locally.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Nov 5, 2026
@thymikee
thymikee merged commit a9e286e into callstack:main Nov 6, 2026
22 checks passed
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.

2 participants